feat(review): a reviewer's objection cannot be answered on the change it is about

[tpouyer]

A reviewer objects on a framework-opened pull request. There is no way to say "you are right, fix
it here". Every route produces a second pull request, or a person picks up the keyboard.

Found on #442, where two review lenses (intent and tests) independently flagged a negative
control that asserted a property of str.startswith rather than anything about the code. The
finding was correct and small. Addressing it took a person pushing a commit to the branch.

What each route actually does today

/implement again on the ticket is the designed path and it half-works. with_review
(platform/conversation.py:103) calls scm.changes_for(key), finds the open change requests opened
for that ticket, and folds the review remarks into the ticket the model reads —
implement.yml grants pull-requests: read for exactly this, and says so. The first run on #431
printed the tell:

review    none (no open change request for #431)

But the second attempt opens a new branch and a new pull request: the branch name carries the
run id, and nothing looks for an existing change request to push onto. You end with two, and close
one. That is coherent — an attempt is a proposal and a second attempt is a better-informed proposal
— but it is not what a reviewer means by "fix it".

/fix on the pull request's own comment thread does something else again, and not the thing
anybody wants:

• fix.yml passes ISSUE: ${{ github.event.issue.number }}, which on a pull-request comment is the
  pull request's number. GitHub draws both from one sequence, so ticket_for resolves it to the
  pull request and its body becomes the bug report.
• The model materialises a worktree of HEAD, so the code under review is not in front of it. On
  fix(config): one directory named .lockstep, and a constant that says what it actually protects #442 the file the lenses complained about does not exist on main at all.
• fix/propose opens a fresh run-scoped branch. A second pull request again.
• And DiagnoseThenFix requires a reproducer that is red against current code. "This test
  passes for a reason that proves nothing" is not expressible as a failing test, so the likely
  outcome is fix.not_reproduced before any of the above matters.

None of that is broken. It is three verbs doing what they say, and none of them is amend this.

The hard part, which is not the plumbing

Two constraints turned up while working out why this is not a small change. Both are real and both
should be settled before anybody builds it.

1. Provisioning over a model-authored branch tip breaks the ordering property. prepared runs
the repository's own Provision over HEAD — reviewed code — before the staged change lands, and
worktree.py's docstring is explicit that this ordering is the whole control (#422): provisioning
the staged tree "would be arbitrary code execution with a supply chain attached", because
package.json and package-lock.json are not tier-1 denied and npm ci runs whatever
postinstall scripts a manifest names. A branch tip that a model wrote is exactly the tree that
argument refuses. So "start from the pull request's head" cannot simply replace "start from HEAD".

2. The branch may have a person's commits on it. #442 has one: a human pushed 6474c22 after
the model's d966444. A second run writing to that branch is a model writing over a person's work,
and assert_run_scoped's protection is a prefix rule — it would permit it, because the branch is
run-scoped, just scoped to a different run. Whether one run may write to another run's branch is a
control question, not a convenience question.

Open questions, none of which this issue answers

• Does a second attempt push to the existing branch, or keep opening new pull requests and close
  the old ones? The second is what the framework does now and it is defensible; say which and why.
• If it pushes: what is the base for the change — the branch tip (see constraint 1) or HEAD with
  the branch's changes re-derived? A ChangeSet is whole-file contents, so "amend" is not a diff
  operation today.
• If the branch carries a person's commit, does the run refuse, rebase, or write over it? A refusal
  by name is probably right and is certainly the cheapest.
• Is this /implement learning to update, or a new verb — /revise <pr> — whose subject is a
  change request rather than a ticket? The second keeps three existing verbs honest about what they
  do and makes the new behaviour opt-in.
• Should a reviewer be able to scope it: "address the tests lens's finding" rather than "have
  another go"?

Acceptance

Deliberately thin, because the design decision above is the work:

• The decision is written down where somebody meets it — on the verb that gains the behaviour, or in
  docs/trampoline.md beside the other chat-ops triggers.
• Whatever runs it refuses by name where it must not proceed: a branch carrying commits it did not
  write, a base it may not provision over, a change request the actor gate does not cover.
• The ordering property in prepared is not weakened. If a run needs a branch tip, that is a
  separate argument made explicitly, not a parameter that quietly changes what gets provisioned.
• /fix on a pull request's comment thread either does something defensible or says what it is
  doing. Silently treating the pull request as a bug report is the least useful of the three
  behaviours and nothing tells the person who typed it.

Objectives

O12 — a second engineer is served, not obstructed. This is the row with the least under it, and
"a reviewer's objection is answered by the machine that wrote the change" is squarely what it is
about: today the objection is answered by whoever is willing to open an editor. O10 — the loop
closing on this repository is what surfaced it, on a pull request the framework wrote and its own
lenses reviewed.

---

Decided (2026-09-11), and the issue is narrower than it was written

Re-reading the code first changed what this is. Most of the path already works. A reviewer
commenting /implement on the pull request already reaches the framework, and already reaches the
right ticket:

• implement.yml fires on issue_comment, which a pull-request comment raises, and passes
  github.event.issue.number.
• ticket_for (platform/conversation.py:59) reads the change request's trailer block and resolves
  it back to the issue it was opened for — "feat(ledger): a run says which subject it measured, so two runs can be compared #219 is one or the other and never both".
• with_review then gathers what people said on that open change request into the ticket the model
  reads, which implement.py:70 already calls "what makes a second /implement a reply rather
  than a retry".

So the reviewer's objection already reaches the model. The only thing missing is that propose then
opens a second pull request instead of updating the one being reviewed.

No workflow file changes. Every option considered lands in src/, which matters: .github/ and
.lockstep/ are tier-1 in DENY_ALWAYS and a model cannot write either.

1. A second attempt force-pushes to the same branch

Same pull request, same URL, review threads where the reviewer left them.

The changeset is still built over HEAD. That is the point of choosing force-push over
committing on top: prepared runs Provision over reviewed code before the staged change lands,
and building over the branch tip instead would mean provisioning code a model wrote — which is
#422's ordering argument, and not a small one, since package.json and package-lock.json are not
tier-1 denied and npm ci runs whatever postinstall scripts a manifest names.

So this replaces the branch's contents; it does not append to them.

The cost, stated: branch_for embeds the run id and its docstring says that uniqueness "is the
design's whole concurrency story". Reusing a branch gives that up, and what serialises two attempts
on one ticket becomes implement.yml's concurrency: implement-<issue> with
cancel-in-progress: false — which queues rather than overlaps. That is a real narrowing of where
the guarantee lives and it should be written where branch_for argues for it.

2. A branch carrying a person's commits is refused by name

Both #442 and #450 had one — a person pushed a fix on top of what the model wrote. Overwriting that
silently is the failure that would end trust in this fastest, and assert_run_scoped would permit
it, because the prefix matches.

Detection uses machinery that exists. Every framework commit carries an In-Lockstep-Run
trailer (scm/base.py:715), and commits_between already parses trailers off a range. A commit in
the range without that trailer is a person's. No heuristic, no author-name matching.

The refusal happens before anything is written, names the commits it would have destroyed, and
says what to do: close the pull request, or drop the commits, if another attempt is wanted.

3. No scoping, for now

with_review gathers everything said on the change request and the model reads it. Scoping a retry
to one lens (/implement address the tests lens) is a second feature with its own parsing and its
own closed-set question — chatops.aspect_from is the precedent if it is ever wanted. Not in this.

Acceptance

• A /implement whose ticket already has an open change request opened by this framework pushes its
  new changeset to that change request's branch, and opens no second one. The pull request's URL,
  number and review threads survive.
• The changeset is built over HEAD, not over the branch tip. A test asserts that, because building
  over the tip is the obvious implementation and it is the one that breaks feat(sandbox): the repository's own install builds the environment its checks run in #422.
• A branch carrying any commit without an In-Lockstep-Run trailer is refused before the push,
  by name, naming the commits. A test drives a branch with a person's commit on it.
• The refusal says what to do about it, in the refusal — not in a document.
• Two open change requests for one ticket is not a case this invents behaviour for: it updates the
  one it resolved through, and says which.
• A ticket with no open change request behaves exactly as it does today, asserted rather than
  assumed.
• branch_for's docstring says where the concurrency guarantee now lives, since this narrows it.

Notes for whoever picks this up

Nothing here needs .github/ or .lockstep/. Both are tier-1 in DENY_ALWAYS and no grant
lifts either. The work is platform/scm/ (an update path beside open_change),
workflows/implement.py (choosing between them), and tests. Wanting to edit a workflow file is a
signal of being off track.

A new held gate row must be cited by an objective in the same change, or
test_gate_test_8_the_held_and_uncited_count_is_exact_in_both_directions fails naming the new id.
That is the ratchet working; cite it rather than widening anything. O12 is the likely one — this
is squarely about a second engineer's objection being answerable.

Assert the claim, not the recording. Several tests in this repository have been pinned to
incidental facts — a finding count, a token field, a line number — and gone red for reasons that had
nothing to do with what they protect. A negative control that cannot fail is worse than none.

[tpouyer]

/implement

[github-actions]

/implement opened #455 as ready for review. Nobody has read it yet.