Repository navigation
D: show a push left running by a crash is harmless; document it (#112) - #116
Merged
Merged
Conversation
TaskPublishing (runner/publishing.ts) publishes the task's PR with the real publisher: GhAlreadyFixedGateway, GhPullRequestGateway and a GitBranchPusher that pushes from the runner-owned repository to the configured repository on GH_HOST (never a configured URL), overwriting only ledger-owned commits. - Mode by status: a running task whose plan items have all run gets a ready PR; a task in needs human gets a draft PR with what sent it there. - When: after a start/resume run ends; when a refused start/resume moves the task to needs human; on the new `publish` runner action; once at startup, after the CLI re-verifies the lock, if a publish is owed. - Schema v10 `publish_outcomes`: the last outcome, shown as `publish` in GET /api/runner and replayed by the publish action. Tasks already running or in needs human at the upgrade are recorded as `not published`. - Shutdown: publishes are aborted in step 1 and awaited before the Store write gate closes. Start, resume and a second publish are refused while a publish runs. Demos never publish. - github.baseBranch is required with a runner block, checked before the lock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A crashed or killed codeboost can leave its git push running, and nothing records its process group. The push is leased to the branch value its own run read, so it lands only while the branch still holds that value: - before a newer publish reads the branch: read as codeboost's commit; - between that read and its push: the newer push is refused as stale, and the next publish moves on; - after the newer publish moved the branch: the leftover is refused. pushArguments is now the one place that builds the push, used by the pusher and by tests that replay a leftover push against a real bare remote in each order. Dropping the lease fails the two lease tests. The docs also cover a branch that returns to the read value (moved back by a person, or the same commit pushed again). No process group is recorded (decision on #112). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…env, CLI test - A draft PR's problems name the current attempt's safety finding before a spent budget, so a budget that ran out later cannot hide the violation. - The already-fixed and PR gateways take the setup's env, the same as the push, so GH_HOST and credentials agree; recorded errors are redacted with that env's token values (RunnerAssembly.env, TaskPublishing). - A CLI test shows the startup publish runs after the lock is verified. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…12-push-process-group
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The empty-lease create-delete-recreate ordering is undocumented and untested.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Documents and tests why crash-surviving pushes are lease-safe.
Changes:
- Centralizes push argument construction.
- Adds crash-ordering regression tests.
- Documents lease behavior and limitations.
| File | Description |
|---|---|
runner/branch-push.ts |
Extracts leased push arguments. |
test/runner-branch-push.test.ts |
Tests leftover push orderings. |
docs/implementation/pull-request-opening.md |
Documents crash-surviving pushes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… feat/103-publish-pr # Conflicts: # web/cli.ts
…/codeboost into feat/112-push-process-group
A run that read no branch pushes with an empty lease, which matches again once a newer publish's branch has been deleted (by a person, or by GitHub after the PR merged): the leftover recreates the branch at its own codeboost head, with no PR, overwriting nothing. Copilot review on PR #116. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…103) The publish action's ID lived only in memory, so a crash after its 'publishing' reply committed and before the publish recorded an outcome left the replay saying 'publishing' for ever. A reply that still says 'publishing' now counts as unsettled: any later outcome of the task settles it (one publish runs per task), and startup settles it as 'stopped' (interrupted) when no publish is owed. Copilot review on PR #110. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
…ver record at startup (#103) An automatic publish (after a run, at startup) had no durable in-flight marker: a crash after its PR opened and moved the task to in review, but before its outcome was written, left publish.last empty or stale for good, since startup then saw nothing owed and no unsettled action. Every publish now records 'publishing' before its first external call (and does not run if that write fails); startup publishes again when one is owed and otherwise settles a leftover 'publishing' record as 'stopped'. Copilot review on PR #110 (previously missed finding). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
…lock before the lock (#103) - A task whose earlier run owes a safety escalation or a scope pause (its write failed, or the process stopped after the item settled) is not finished work, but every item has run, so no later run would pay it and a ready PR would show unreviewed out-of-scope changes. Publishing refuses it (mode), and a run's end, startup and a refused publish action have the executor pay it first (ItemExecutor.owes/payOwed). - The CLI refuses a runner with no github block before taking the lock, so the missing block is reported as itself, not as a lock conflict. Copilot review on PR #110. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
- A ready publish needs every item of the current plan revision completed at that revision, checked in the Store's publish guard before every guarded write: a revision that adds an item during a publish's first GitHub lookups no longer gets a ready PR. - Only a publish action refused because of owed work pays it; a refusal for a stale view, a busy task or a replay starts nothing. - A task recorded as 'not published' at the v10 upgrade stays unpublished until a person asks, whatever changes since. - A failure while paying owed work no longer stops startup from settling an in-flight record or a stuck reply. - A publish refused while a lost opening settles is retried at that opening's deadline (settleRemaining). - Startup settles a reply that is the only thing stuck with the outcome on record, leaving that record (and its version stamp) as it is. - The owed-work scan runs only for a status that could publish, not on every poll. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
- A publish that ends with an opening still in flight (its own reply was lost, not only a refusal while one settles) is retried at the opening's deadline; a list-lag OpeningUnsettled is retried after 30 s. - 'not published' (v10) holds only until a run starts after the upgrade. - Only the publish action's own owed-work refusal pays that work; a stale request or any replay applies nothing. - A draft waits only for an owed safety finding, not an owed scope pause (which a needs-human task cannot record). - startup() leaves a publish of this process alone; the stopped record it writes keeps the interrupted publish's draft flag; the retry decision cannot throw out of run(). - owes() reads only owed findings in memory and completed out-of-scope attempts (Store.scopeFindingHeads), not every attempt, on a poll. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
- Bound the automatic retry (AGENTS.md: back off recurring external polling): one timer per task; a lost opening is waited for only while its deadline is ahead, so one still stuck after it (two PRs carry its marker, the repository changed, GitHub keeps failing) no longer re-publishes every second; the list-lag retry runs once per chain. - A person's start or resume that moves an upgraded task (v10 'not published') ends that hold, so a refused resume that sends it to needs human gets its draft. - Owed work is paid only where a run would (running, queued) or a draft waits on it (needs human). - The 'not published' tests space their timestamps, which are compared. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
- Retry budget per chain (until a publish settles): at most one retry at a lost opening's deadline and one short retry, so a GitHub outage that fails every open no longer republishes every settle time for ever. - An 'opened' outcome that left the PR or the task where the publish did not mean them (GitHub showed another head, so the PR was drafted and the task stayed running; a draft change failed) is recorded with `reconcile`: still owed, and given the short retry. - A refusal just as an opening's deadline passes gets the short retry. - The short retry's delay is a test seam (RunnerAssembly.shortRetryMs); tests now cover the list-lag retry, the outage bound and reconciling. - Tests that proved less than they claimed: the owed-work failure test now reaches the failure; the CLI baseBranch half holds the lock; the CLI startup test stops only after the outcome is recorded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
- A run's end, a person's action or a restart starts a new retry chain with its own budget (AGENTS.md: reset on a lifecycle change or user action); a retry timer's own publish does not, so retries stay bounded. - No short retry while a lost opening's deadline is still ahead (it would only be refused again). - `reconcile` only while the task can still be published; the action reply carries it; the settleHead comment no longer overstates. - The owed-work path of a refused publish action carries the person's intent (personAsked), like start/resume. - publishOwed runs the lock check itself, first: no order of the CLI's lines can start a startup publish before the lock is verified. - Tests: the one-short-retry bound, the new chain on a person's action, and the lock check running first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
- A person's publish action drops a retry the old chain armed, so it cannot fire later on the new chain's budget (at most two automatic publishes per chain, not three). - Tests for two round-5 fixes that no test covered: no reconcile for a task that can no longer be published, and the action reply carrying reconcile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
- Tests for the owed-safety-finding gate: a ready publish is refused while a finding is owed, the action pays it and the draft follows; a needs-human task's owed finding is paid at startup before its draft; a replay of that refusal pays nothing. - The draft of a task whose owed finding was paid names that finding: the executor settles it in memory only, so TaskPublishing keeps the reason payOwed returned (it was lost before). - A run's end starting a new chain drops the old chain's armed retry, as a person's action does (#newChain): one timer per task, tested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A paid safety finding names a draft only while the task stays in needs human on the same current attempt; a later return has its own reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
A publish refused by a guard (a race) gets the one short retry; a draft names a finding whose save failed even after the budget ran out; a needs-human task pays only an owed finding, not a scope pause. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/codeboost into feat/112-push-process-group
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #112. Stacked on #110 (base
feat/103-publish-pr). Retarget it tomainonce #110 merges.What it does
#112 asked whether a
git pushleft running by a crashed or killed codeboost must be tracked, or shown to be harmless. Decision (user, 2026-10-03): show it is harmless and document it. No process group is recorded and there is no schema change.The push is leased to the branch value its own run read, so it lands only while the branch still holds that value. Three orderings against a newer publish:
BranchPushRefused) and nothing is overwritten. The next publish moves on.One exception, documented. The lease compares the branch's current value, not its history. So if the branch returns to exactly the value the run read (a person moves it back, or the same commit is pushed again), the leftover lands and puts back a codeboost commit. The open PR shows that commit until the next publish refreshes it.
Code change.
pushArguments(runner/branch-push.ts) is now the one place that builds the push. Both the pusher and the tests use it.Tests
test/runner-branch-push.test.tsreplays a leftover push against a real bare remote in each order, using the pusher's own arguments.[rejected] … (stale info)).Typecheck passes. The branch-push, publishing and publish files pass: 215 tests.
Review rounds
/code-review high/code-review high/code-review highReview-lesson audit. Both points fall under existing AGENTS.md rules:
No new rule is needed.
🤖 Generated with Claude Code