Skip to content

[Workstream C] PR 1 of 2: Controller-mediated, head-bound PR review submission tool #3096

Closed

Description

@zaxbysauce

Pipeline-hardening issue. Generated from the 2026-10-04/05 PR-review session retrospective; claims validated by independent reviewer + critic lanes at tree 1400cc0, then re-pinned against origin/master 5692faa on 2026-10-05. One issue = one PR.

Summary

Every PR-workflow channel except GitHub publication is bound to the exact pr_head_sha. There is no controller tool that submits review results to the PR: the controller surface is submit-receipt + artifacts only, the child overlay is explorer-only, and in read-only gate modes the operator cannot post findings at all (observed: gh pr comment blocked by the PR_REVIEW allowlist; gh CLI absent from PATH on the source host). This PR adds the missing head-bound submission tool, additively.

Validation provenance

  • Round-1 reviewer + critic: F5 UPHELD MEDIUM — "every other channel is head-bound; GitHub-side publication escapes that provenance discipline"; additive, cannot corrupt an existing settlement path.
  • State-of-the-art grounding: canonical machine-review channel is the GitHub PR Review API with commit_id-pinned inline comments (docs.github.com/rest/pulls/reviews); reviewer-behavior rules (dedupe against prior comments, consolidate repeats, bounded comment count) per published agent-review practice.

Problem (claims to verify; pinned at 5692faa)

  1. The PR-workflow tool surface (src/tools/manifest.ts:200-230) contains no review/comment submission tool (controller surface: complete/abort/authorize/submit-receipt/write-artifact/write-trigger-eval/prepare-checkout).
  2. Head-binding discipline exists elsewhere (validated at 1400cc0: write-pr-review-artifact.ts:995-998 pr_head_sha schema — re-derive exact lines on master).
  3. The child capability map is submit-only (src/config/constants.ts:397, explorer-only at :410).
  4. Source-session observation: with the PR_REVIEW gate active, no operator-visible path could publish findings (gate allowlist excludes gh pr comment; gh absent → gh_evidence degraded).

Acceptance criteria

  • AC1: THE SYSTEM SHALL provide a controller tool that submits a PR review through the GitHub PR Review API, bound to the gate's exact pr_head_sha, with commit_id-pinned inline comments for findings carrying file:line evidence.
    • class: DISCRIMINATING
    • argv: bun test tests/unit/tools/pr-review-submission.test.ts
    • expect: RED at 5692faa (no such tool registered).
    • files: src/tools/pr-review-submission.ts (new), tests/unit/tools/pr-review-submission.test.ts (new)
  • AC2: Submission SHALL be authorization-gated — available only after complete_pr_workflow clears the gate (or under explicit operator authorization), never auto-posted from a discovery or validation lane.
    • class: DISCRIMINATING
  • AC3: The submission body SHALL be severity-grouped with locations, finding IDs, and coverage disclosure; rendering SHALL remain strictly separate from submission authorization (the renderer is a pure function over settled findings).
    • class: DISCRIMINATING
  • AC4: Reviewer-behavior constraints SHALL hold: dedupe against existing comments, consolidate repeated findings into one comment, and a bounded maximum comment count.
    • class: DISCRIMINATING
  • AC5: No existing settlement, gate, or artifact path SHALL change (additive registration only; tool-metadata/manifest/index barrel updated per invariant 11).
    • class: PRESERVING

Constraints and invariants

  • Additive only — this tool must not alter any settlement or gate semantics.
  • Never bypasses read-only gates: while PR_REVIEW is active the tool refuses absent the completion path.
  • Full tool-registration invariant set applies (TOOL_METADATA entry, TOOL_MANIFEST handler, barrel export, help surfaces, tests).

Suggested approach (non-binding)

Model the tool on write_pr_review_artifact's head-binding and gate-check pattern; renderer as a pure module so [Workstream C] PR 2/2 (and any future preview surface) can reuse it without authorization.

Scope

  • In scope: the submission tool, its renderer module, registration surfaces, tests.
  • Out of scope: post-abort export ([Workstream C] PR 2/2); renderer polish beyond AC3's grouping.

Files expected to change

  • src/tools/pr-review-submission.ts (new)
  • src/tools/tool-metadata.ts, src/tools/manifest.ts, src/tools/index.ts (registration)
  • tests/unit/tools/pr-review-submission.test.ts (new)

Defect class and known siblings

  • Class: provenance gap at the GitHub boundary (head-bound discipline ends at the last controller surface).
  • Known siblings: none (net-new capability).
  • Guard that should have caught it: a provenance test asserting every remote-write surface is head-bound.

Dependencies

  • Depends on: none
  • Gated on: none
  • Parallel-safe (touches no hotspot files; registration-only overlap)

Tier and risk triggers

  • Tier: M
  • Risk triggers: remote write authorization; GitHub API contract

Verification

  • bun test tests/unit/tools/ tests/unit/config/
  • bun run check-tool-registration && bun run check:invariants

Definition of done

  • All ACs verified with evidence; issue-tracer full-resolution contract satisfied; PR body Closes #<n> with closure evidence.

Activity

  1. added
    severity:mediumMeaningful workflow friction or correctness risk
    type:featureNew capability or behavior change
    parallel-safeCan be implemented concurrently with sibling slots
    on Oct 5, 2026
  2. zaxbysauce commented on Oct 6, 2026

    @zaxbysauce
    CollaboratorAuthor

    issue-tracer pre-implementation checkpoint anchor (3096-pr-review-submission-tool, base f102d09, frozen 2026-10-06). Acceptance checks C1-C4 DISCRIMINATING (RED at base) + C5 PRESERVING (GREEN at base) authored by an independent check-author context.

    issue-tracer-checkpoint-v1 slug=3096-pr-review-submission-tool manifest=03595a393a11abd67c0425fe92283392ae4201b2 semantics=59a999d436faba52c9df2bb35996f1e2bc06eb5c tree=31ebf9e26c2fbabb30f5d20ca7652b90d725900d

    Reviewers: verify with repro-check.sh verify-anchor --slug 3096-pr-review-submission-tool --receipt ''.

  3. zaxbysauce commented on Oct 6, 2026

    @zaxbysauce
    CollaboratorAuthor

    PR #3118 delivers this issue: #3118

    What changed: new architect-only pr_review_submission controller tool + pure renderPrReviewSubmissionBody renderer module. Submits a settled PR-review run to GitHub through the PR Review API (bounded gh transport) with commit_id pinned to the run's exact pr_head_sha and file:line inline comments. Fail-closed authorization: refuses while a PR-workflow gate is active, on an abort at/after settlement, on receipt/record head mismatches, and without a post_critic settlement record; child lanes never see the tool. Additive only — no settlement, gate, or artifact path changes.

    How to use: after a PR-review run settles and complete_pr_workflow clears the gate, call pr_review_submission with { pr_number, repo (owner/name), run_id, pr_head_sha } from the architect session that ran the workflow.

    Migration: none. Full issue-tracer ladder (frozen checks C1-C5, published checkpoint anchor above, plan critic 3 rounds, implementation review 2 rounds, final critic — all APPROVE). Merging awaits explicit maintainer approval.

  4. zaxbysauce commented on Oct 6, 2026

    @zaxbysauce
    CollaboratorAuthor

    SUPERSEDES the earlier checkpoint anchor (comment 6011535643, manifest 03595a39...) for trace 3096-pr-review-submission-tool. Reason: sanctioned AMEND (CHECK_WRONG) of frozen check C4 during the swarm-pr-feedback round — the check pinned bare-substring dedupe that review findings PRR-003/PRR-007 verified as silently dropping findings (substring collisions) and gameable; C4 was re-frozen with anchored-marker dedupe semantics after a fresh RED(base f102d09)→GREEN replay. Acceptance-table semantics unchanged. New receipt:

    issue-tracer-checkpoint-v1 slug=3096-pr-review-submission-tool manifest=3832719bb2fe3f8757d2ffd9c63d755df5ae7bd8 semantics=59a999d436faba52c9df2bb35996f1e2bc06eb5c tree=31ebf9e26c2fbabb30f5d20ca7652b90d725900d

    Verify: repro-check.sh verify-anchor --slug 3096-pr-review-submission-tool --receipt ''.

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

    area:reviewarea:toolsArea: toolsparallel-safeCan be implemented concurrently with sibling slotsready:implementationScope is clear enough to implementseverity:mediumMeaningful workflow friction or correctness risktype:featureNew capability or behavior change

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions