Public: anyone can read this; the crew can change it.

A resolve run that pushed nothing is never shown as the crew's commit #2296

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

A resolve run that ends `ok` without moving the pull's head now records no commit, and the conflicts page reads "the crew did not push a resolution" instead of "committed as".

shipped production is serving it deploy b8ab032

opened this

A resolve run that ends ok without moving the pull's head now records no commit, and the conflicts page reads "the crew did not push a resolution" instead of "committed as".

bun test passes on test/proposal-run.test.ts, test/conflicts.test.ts and test/crew-proposal.test.ts. bun x tsc --noEmit could not run to a verdict here: it stops at "Cannot find type definition file for 'bun'" because @types/bun isn't installed in the sandbox, and it reported nothing else. The forge's check will give the real type verdict.

What changed

  • Start of a resolve run (src/crew/runner.ts): the run keeps the pull's head in run.base_oid, the column that already exists. It was only used by check runs, so no migration. No new column, and nothing else reads it for agent runs.
  • End of the run (src/crew/runner.ts): headLeftBy sets commitOid only if the head differs from that start.
  • proposalOf (src/crew/proposal.ts): a run that ended ok with no commit, on a pull that still conflicts, reads failed with that summary. Once nothing conflicts it reads none, as a failed run already did.
  • Tests:

- test/proposal-run.test.ts: an ok resolve run with no commit reads failed, and one that recorded the head reads landed. - test/crew-proposal.test.ts: with a fake agent that pushes nothing, the run records no commit and the conflicts page lacks "committed as" and shows the new summary. I put this one there because that file already has the crew harness; proposal-run.test.ts has none.

Left out

  • Older ok resolve runs that recorded a head without moving it keep their commit. Only new runs get the fix.

To try

  • Tap "Have the crew propose" on a conflicting pull with an agent that exits without pushing. The page shows "The last proposal did not land (ok): the crew did not push a resolution".

Closes #2276

commented

A resolve run that ends ok without moving the pull's head now records no commit, and the conflicts page reads "the crew did not push a resolution" instead of "committed as".

bun test passes on test/proposal-run.test.ts, test/conflicts.test.ts and test/crew-proposal.test.ts. bun x tsc --noEmit could not run to a verdict here: it stops at "Cannot find type definition file for 'bun'" because @types/bun isn't installed in the sandbox, and it reported nothing else. The forge's check will give the real type verdict.

What changed

  • Start of a resolve run (src/crew/runner.ts): the run keeps the pull's head in run.base_oid, the column that already exists. It was only used by check runs, so no migration. No new column, and nothing else reads it for agent runs.
  • End of the run (src/crew/runner.ts): headLeftBy sets commitOid only if the head differs from that start.
  • proposalOf (src/crew/proposal.ts): a run that ended ok with no commit, on a pull that still conflicts, reads failed with that summary. Once nothing conflicts it reads none, as a failed run already did.
  • Tests:

- test/proposal-run.test.ts: an ok resolve run with no commit reads failed, and one that recorded the head reads landed. - test/crew-proposal.test.ts: with a fake agent that pushes nothing, the run records no commit and the conflicts page lacks "committed as" and shows the new summary. I put this one there because that file already has the crew harness; proposal-run.test.ts has none.

Left out

  • Older ok resolve runs that recorded a head without moving it keep their commit. Only new runs get the fix.

To try

  • Tap "Have the crew propose" on a conflicting pull with an agent that exits without pushing. The page shows "The last proposal did not land (ok): the crew did not push a resolution".
Approved

The fix is correct and small. A resolve run now records the pull head it started from in the existing base_oid column (setRunBase already exists). It sets commitOid only if the head moved. proposalOf reads an ok run with no commit as failed while the pull still conflicts, and as none once it does not. Both tests would fail without the change. Nothing here touches input, auth or output escaping, and no dependency was added. I did not run the preview. Two things I would do next: (1) the page copy reads "did not land (ok)", where a person sees "ok" next to a failure; for this case drop the state or say "ended without pushing". (2) Move the setRunBase import to its alphabetical place in runner.ts, after overranBefore.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs