Skip to content

Close the HITL deferral loop: reconcile shft/hitl review issues on PR merge/resolution #306

Description

@arndvs

Close the HITL deferral loop: reconcile shft/hitl review issues on PR merge/resolution

Problem

The engine's HITL deferral path (shft/engine/lib/defer-to-issue.ts) creates a GitHub issue carrying the shft and hitl labels whenever an auto-review/address-review round encounters a HITL-tier Copilot comment it won't auto-fix. The intent is a durable, human-reviewed record of a caught decision (the core thesis of ADR-006's "verifiable decision history").

That record is currently write-only and terminal:

  • The pipeline state machine (instructions/sandcastle-pipeline.instructions.md, rows 37–38) declares both labels as "review-metadata marker" with a — for every downstream transition. No workflow keys off the shft or hitl labels.

    | `shft`  | `defer-to-issue` (engine) | — (review-metadata marker) | — |
    | `hitl`  | `defer-to-issue` (engine) | — (review-metadata marker) | — |
    

    Every other pipeline label has at least one successor transition; these have none.

  • There is no automatic close path. A deferral issue is only ever closed by a human manually triaging it. There is no expiration and no close-on-thread-resolution / close-on-PR-merge behavior.

  • The back-reference is one-way: the issue ↔ PR thread link is written at creation time (Deferred to #N reply on the PR), but nothing records when/if the underlying comment is later addressed, the thread is resolved, or the PR merges. A resolved-at-the-source HITL item stays open forever in the issue backlog.

  • The dedup guard (findExistingIssue) matches only on exact, identical, still-open issue titles. The near-identical recurrence of a comment on the same code path after a re-review (e.g., wording shifts, or the original thread resolved but the issue not closed) bypasses dedup and creates a second, slightly-different open issue.

Impact:

  1. Unbounded, decaying backlog. HITL deferral issues have a positive creation rate (any HITL comment on any auto-reviewed PR) and a near-zero automatic close rate. Over a repo's lifetime the shft/hitl backlog accumulates review items that have meanwhile been handled at the source, making it progressively harder to tell a genuinely pending human decision from a resolved-but-never-closed one. The label ceases to mean "needs human action."

  2. ADR-006 blind spot persists on the consumer side. The Decision Ledger's foundational query — "every decision, human and autonomous, on PR #N, and whether it survived review" (ADR-006, Gap 1) — cannot account for HITL deferrals because there is no terminal state and no resolution datapoint. The creation of a deferral is recorded; the outcome is not. Issue Persist safety-hook denials as HUD events to build the blocked-mistake audit trail (ADR-006 gap #2) #295 fixes this for safety hooks (the decision.blocked emission into HUD), but its scope stops at HUD events — it does not give deferral issues a lifecycle. This proposal is the consumer-side mirror that Persist safety-hook denials as HUD events to build the blocked-mistake audit trail (ADR-006 gap #2) #295 does not cover.

  3. Decision sustainment is unreachable. Without a defined "resolved" state, the "% survives review unchanged" metric (ADR-006, Slice B) has no signal for whether a deferred comment was acted on, disputed, or still unresolved.

Proposal

Introduce a HITL deferral reconciliation path that gives shft/hitl issues a bounded lifecycle, while staying strictly observational — it closes records, never auto-acts on the underlying comment. The human judgment the HITL label exists to force is untouched.

Scope

1. A single-source reconciliation rule documented in the pipeline contract.
Update instructions/sandcastle-pipeline.instructions.md so the two metadata labels have a defined terminal transition:

  • shft/hitl → closed when the backing review thread is resolved and/or the PR that raised the comment is merged or closed.

This encodes the invariant: a HITL deferral issue is a pending-decision record, not a permanent artifact — its lifetime is bounded by the PR/thread that produced it.

2. The merge/close hook (bin/ or hooks/).
Add a reconciliation step that runs when a PR closes (pull_request.closed) or when a review thread resolves. It:

  • Lists open issues labelled shft+hitl whose body references the closing PR (PR: #N — already present in the generated body, defer-to-issue.ts).
  • Closes each with a comment noting the PR merged/closed and the date. No label mutation, no comment edits.

Wire it as a small GitHub Actions workflow (alongside agent-*.yml) triggered on pull_request: closed, matching the existing AGENT_PAT label-chaining convention so it works across repos. Because this runs on PR-close only, it addresses the dominant leak (PRs merge; their associated HITL issues linger).

3. Thread-resolution reconciliation (engine-side, optional but recommended).
Extend the existing resolveThread/address-review close-out path: when a deferred thread is later resolved (a human or a later round addresses the comment), mark the corresponding shft/hitl issue resolved with a status comment. This is the highest-signal terminal state for ADR-006.

4. Deliberate, documented non-goals (kept for the human-in-the-loop boundary):

  • Reconciliation never alters the content of a deferral issue or the underlying PR comment.
  • Reconciliation never re-routes a HITL item into the autonomous pipeline (no agent:* transition from hitl). A human must still act on the comment.
  • HITL verdict (accepted/disputed) is left to the human + the decision ledger Slice B — this proposal only provides the bounded terminal state those systems need.

Success criteria

  • A PR merge closes every open shft/hitl issue whose body references that PR (#N).
  • A resolved review thread closes the matching deferral issue (engine-side).
  • instructions/sandcastle-pipeline.instructions.md documents the shft/hitl → closed transition; the — cells for these two rows are gone.
  • Dedup (findExistingIssue) is verified to not re-open or duplicate after reconciliation closes the original.
  • Tests cover: close-on-PR-merge, close-on-thread-resolve, no-op when PR has no open deferral issues, and no content/label mutation on close.
  • ADR-006's "every decision … on PR #N" query can now list deferral outcomes (open = pending, closed-with-reason = resolved).

Out of scope (anti-targets)

References

  • shft/engine/lib/defer-to-issue.ts — issue creation, dedup, useReviewLabels guard
  • shft/engine/lib/resolve-threads.ts, workflows/address-review.ts — thread-resolution path
  • instructions/sandcastle-pipeline.instructions.md — label state machine (rows 37–38 are the gap)
  • shft/engine/lib/pipeline-states.ts — canonical label catalogue (shft/hitl entries)
  • ADR-006 docs/adr/ADR-006-decision-ledger.md — Gap 1 metric; Persist safety-hook denials as HUD events to build the blocked-mistake audit trail (ADR-006 gap #2) #295 is the safety-hook half of the loop

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    source:architecture-reviewPRDs proposed by the automated architecture-review workflow

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions