Skip to content

F5: prepare rebases and run head-bound checks - #149

Draft
mchwang wants to merge 37 commits into
mainfrom
codex/f5-rebase-admission
Draft

mchwang wants to merge 37 commits into
mainfrom
codex/f5-rebase-admission

Conversation

@mchwang

@mchwang mchwang commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add production prepare-merge admission that refreshes the task PR's exact base/head, fetches both commits, and admits the trusted F3/F4 rebase path
  • apply one-to-one rewrite mappings to the durable ledger, rebuild the review snapshot, preserve approvals whose attributed fingerprints are unchanged, and require review when attribution or approval evidence becomes stale
  • run every allowlisted plan-item cmd: acceptance check in the credential-free, no-egress runner profile and persist passing evidence against the exact reviewed head and command digest
  • bind guarded-merge readiness to the current task/review/snapshot generation and the latest successful preparation; preserve cancellation, shutdown, retry, and subprocess ownership through settlement
  • seed and ref-anchor a fresh runner-owned repository before making it authoritative for review reads, including the all-no-op execution path

This is F5 of #22. F6 remains: publish a rewritten head, wait for fresh required checks, rerun already-fixed detection, and hand the exact pair to guarded merge.

Current validation

Exact pair: base 5ecf92f6960c17c40aa122c38bcf2f7b1c6106b5, head e713ed7ce1aef02f99c98c33e9fa52b8032a6069.

  • current exact-head multibyte-diagnostic regression: failed before with a replacement character and a published byte count above the configured limit, then passed by UTF-8-bounding the retained prefix before appending the truncation notice

  • current exact-head affected suites after integrating current main: 8 files, 357 passed in 901.04 seconds; npm run typecheck and git diff --check passed

  • current exact-head independent full-diff review: clean on 5ecf92f..e713ed7; 233 focused tests and type checking passed in the review

  • current exact-head CI: push and pull-request runs passed; each ran 74 non-Docker files with 1,976 tests, 137 browser tests, and type checking

  • current exact-head remote full/real-Docker gate: 81 files, 2,230 passed, 1 skipped in 1,128.53 seconds; workflow typecheck also passed

  • prior head 3adc132 exact-head CI: push and pull-request runs passed; each ran 74 non-Docker files with 1,976 tests, 136 browser tests, and type checking

  • prior head 3adc132 remote full/real-Docker gate: 81 files, 2,229 passed, 1 skipped in 1,180.32 seconds; workflow typecheck also passed

  • current exact-head cleanup-failure regression: failed before because a retained rebase marker made its actionable cleanup failure immediately stale, then passed with the failure bound before the aggregate error is thrown

  • current exact-head lifecycle regressions: failed before when unresolved cleanup reached review/remote work, a failed rebase adopted a concurrent review edit, and an already-aborted shutdown still loaded review; all pass after fail-closed admission and original-review failure binding

  • current exact-head affected suites: 6 files, 294 passed serially; the full pre-merge suite passed 37 tests; npm run typecheck and git diff --check passed

  • current exact-head independent full-diff review: clean on 89707f8..3adc132; 233 focused tests and type checking passed in the review

  • prior exact-head core affected suites: 15 files, 660 passed serially; npm run typecheck and git diff --check origin/main...HEAD passed

  • current exact-head startup snapshot-anchor regression: failed before because persisted base/head objects had no retaining refs, then passed with both commits anchored before runnerRepository becomes authoritative

  • current exact-head cleanup-only budget regression: failed before with only 1 ms left for Git after process settlement, then passed with a 30-second minimum covering 17 seconds of Git work plus 13 seconds of settlement

  • prior head a857be3 remote full/real-Docker gate: 81 files, 2,225 passed, 1 skipped in 1,162.60 seconds; workflow typecheck also passed

  • current exact-head local Docker attempt: infrastructure-blocked by the saturated shared daemon and interrupted without a terminal result; only containers created by that attempt were removed, and the remote exact-head Docker gate supplies the clean full-suite evidence

  • exact-head affected suites: 14 files, 641 passed in 183.02 seconds

  • exact-head remote-commit retention regression: failed before because garbage collection pruned a fetched collaborator-only head, then passed with every validated remote base/head retained behind a runner-owned ref

  • exact-head unsettled-rebase failure regression: failed before because the retained recovery marker's own state transition made its current failure appear stale, then passed with the failure rebound to the latest durable task/review/snapshot state

  • exact-head command-policy binding regression: failed before by accepting preparation after the configured allowlist changed, then passed by binding readiness to the current policy digest at status and transactional admission

  • exact-head rebase cleanup-reserve regression: failed before because live work received the whole remaining operation budget, then passed with the minimum follow-up abort budget reserved before rebase start

  • exact-head command-output regressions: failed before because oversized or invalid-UTF-8 diagnostics changed an exit-0 command check into an output/capture failure, then passed with bounded lossy diagnostics that preserve exit-status semantics

  • exact-head partial-check result regression: failed before because a later preparation failure erased the already completed P1 check ID, then passed with completed IDs carried into the durable failure result

  • exact-head affected suites: 15 files, 660 passed in 595.86 seconds

  • after integrating current main (c822d36), exact-head affected suites: 15 files, 660 passed in 183.05 seconds; type checking and git diff --check passed

  • exact-head focused Docker regressions: 2 passed; six load-sensitive supervisor cases that failed in the saturated broad run passed in an isolated rerun

  • exact-head shared review-deadline regression: failed before because history linking received a fresh 30-second budget, then passed with one remaining deadline threaded through history and linking

  • exact-head command-allowlist regression: failed before because check preparation invoked a full plan-context read, then passed with a copied direct allowlist and no synchronous Git read

  • exact-head pre-merge suite: 32 passed in 90.88 seconds

  • exact-head merge admission suite: 56 passed in 3.52 seconds

  • exact-head queue-boundary PR-refresh regression: failed before by merging after the base changed during the watermark read, then passed by requiring a fresh matching PR/mode/readiness inspection

  • exact-head rebase-budget regression: failed before with missing live-run and cleanup budgets, then passed with both budgets bounded by the preparation deadline

  • prior head f94501b affected suites: 12 files, 550 passed in 200.97 seconds; merge and runner-start suites 243 passed in 69.85 seconds; runner-start 53 passed in 70.78 seconds after its production-faithful fixture fix

  • prior head 11c3853 affected suites: 10 files, 359 passed in 202.34 seconds; pre-merge suite 31 passed in 102.21 seconds

  • prior head c0262e6 affected suites: 10 files, 359 passed in 233.88 seconds; pre-merge suite 31 passed in 94.09 seconds

  • prior head d212471 affected suites: 10 files, 462 passed in 231.53 seconds

  • prior head f4ab96b affected suites: 10 files, 454 passed in 203.21 seconds

  • prior exact-pair independent full-diff review validation on f4ab96b: 6 files, 274 passed in 213.11 seconds

  • prior-head remote real-Docker gate on f4ab96b: passed in 17 minutes 1 second (run 37810803423); workflow typecheck and full npm test passed

  • prior candidate fb2187d affected suites: 9 files, 318 passed in 170.82 seconds

  • prior candidate cee9e01 affected suites: 9 files, 317 passed in 226.80 seconds; touched suites merge-task-pr.test.ts and store.test.ts, 99 passed

  • npm run typecheck: passed on the current head

  • git diff --check: passed on the current head

  • affected-suite attempt on intermediate head 3d6f0ff: interrupted after ten minutes without a terminal aggregate while shared Docker load rose from 456 to 478 running containers; this infrastructure-stalled run is not counted as passed and remote CI remains pending

  • prior-head affected suite on 49beda1: 15 files, 476 passed

  • isolated diagnostics after the load-stressed full attempt: history.test.ts 47/47, review.test.ts 20/20, pre-merge.test.ts 27/27

  • prior-head remote full suite and real-Docker gate on 49beda1: 81 files, 2,201 passed, 1 skipped, in 1,169.30 seconds; workflow typecheck also passed

  • local full-suite diagnostic: the exact-head run encountered 11 slow Git-heavy failures while Docker ownership probes took tens of seconds, then remained in Docker resource transitions for about an hour. It was interrupted without a terminal aggregate. All 11 affected cases passed in the isolated file reruns above. At diagnosis time the host had 438 running containers and 513 total. This was classified as host infrastructure load, not substituted for the green remote full-suite gate.

  • historical only, before integrating current main: head a0acf18f7654d233ad5e1054e0e2e163eda9d64e passed 81 files, 2,190 tests, with 2 skipped, in 2,058.60 seconds. This is not evidence for the current pair.

Required race regressions

  • moved base with unchanged attributed fingerprints preserves approval
  • changed attributed fingerprints stale approval and require review
  • a breaking post-rebase cmd: blocks readiness and produces no passing evidence
  • collaborator head movement refreshes review against the prior base, including simultaneous base/head movement and movement during final authorization
  • cancelled, timed-out, interrupted, and shutdown-stopped command checks never produce passing evidence and retain their original stop reason
  • retry, rebase cleanup, final authorization, stale generation, current transition, startup recovery, and fresh-runner/no-op paths are covered with controlled promises or subprocess ownership assertions
  • a PR base or merge-queue-mode change during final authorization is rejected by a post-authorization remote validation
  • a refused concurrent preparation does not shadow the admitted preparation that later records readiness
  • trust revoked during the final remote merge-status refresh is refused before durable admission
  • local issue trust revoked during the final pre-merge remote inspection is refused before readiness is persisted
  • merge-queue correlation captures its durable event boundary after final external authorization/status reads, and revalidates local authorization after that final await
  • the PR identity, base, head, merge-queue mode, and readiness are re-inspected after the queue event boundary before durable admission
  • live rebase work and follow-up cleanup share the preparation's remaining monotonic deadline instead of starting fresh timeout windows
  • synchronous review/history reloads retain the process stop/drain reserve inside the overall preparation deadline
  • history parsing and attribution linking share one monotonic review-load deadline
  • command-check admission reads its copied allowlist directly instead of rebuilding the Git-backed plan context
  • persisted preparation readiness is bound to the exact current command allowlist and fails closed for legacy or malformed bindings
  • live rebase admission reserves a separate post-failure abort budget before recording durable ownership

Review ledger

All independent passes used /codex review at high effort and reviewed the full diff for the base/head pair recorded for that round. No finding was declined.

Reviewed head Findings Remediation / regression evidence
988137f preserve rebased heads; guard all versions; prompt cancellation; shutdown reason fixed in 9d7351b; added exact state/head and stop-reason races
9d7351b unsettled rebase cleanup; trust before delayed resources/conflict agent; overall deadline fixed in 5b1d4b3; added cleanup, authorization, and deadline regressions
5b1d4b3 retry authorization; partial allocations; deadline classification; final versions fixed in b11a8ee; added retry/allocation/version regressions
b11a8ee re-inspect PR after authorization; simultaneous base/head movement; stale cached readiness fixed in 715ab39; added final-inspection and moved-pair regressions
715ab39 latest failed check must invalidate an older pass; async cancellation must close the PR; cleanup must settle fixed in c016a54; added latest-attempt and cancellation regressions
c016a54 reserve command settlement; shutdown must settle conflict resolver fixed in d2c5065; added bounded settlement/shutdown regressions
d2c5065 merge could proceed before preparation finalization/cleanup; collaborator lineage through unpushed rewrite fixed in a258492; guarded readiness on terminal preparation and lineage
a258492 trust can be revoked after preparation; rewrite scan was unbounded fixed in c6fd5c3; added final authorization and bounded scan regressions
c6fd5c3 planted configs inherited commands; outer timeout did not stop checks; settlement could exceed deadline; transient settlement could stick fixed in 7b5c112; added policy, timeout, settlement, and retry regressions
7b5c112 synchronous reloads were outside the deadline; fallback snapshot read could fail before settlement fixed in 2c5c910; bounded reloads and made settlement independent; self-review also removed the pending-readiness exposure
2c5c910 malformed Unicode command arguments could normalize across policy/hash/container boundaries fixed in 88200c6; added policy, digest, parser, and direct-script regressions
88200c6 all-no-op runner PR could not prepare; legacy malformed persisted commands made review loading throw fixed in 9068862; added no-op production and legacy repair regressions
9068862 a fresh runner repository became authoritative before it held the initial snapshot fixed in a0acf18; failing-before/passing-after production startup regression
49beda1 outer pre-merge timeout was persisted as cancellation; non-ready historical preparation results never became stale fixed in 3d6f0ff; added durable timeout ownership plus running/preparation races and failed/review-required stale-history regressions
d062643 final authorization could race a remote PR target/mode change; a refused preparation could shadow admitted readiness fixed in cee9e01; added post-authorization target refresh and refused-action precedence regressions
cee9e01 trust could change during the last status await; synchronous review loads could consume the process-settlement reserve fixed in fb2187d; added final trust revalidation and reserved reload-budget regressions
fb2187d the authorization-only third status read also ran without an authorization hook, breaking the established two-read contract fixed in f4ab96b; restored the two-read no-authorization path and added merge.test.ts to the affected gate
Copilot on f4ab96b final authorization/mode ordering; inherited planted runner repository; stale failure binding; rewritten execution and continuation approvals; base-tree repository; two ineffective Unicode fixtures fixed in d212471; six failing-before/passing-after regressions plus real lone-surrogate coverage; no finding declined
independent on d212471 local issue trust could be revoked during the final pre-merge remote inspection after the asynchronous authorization refresh fixed in c0262e6; added a failing-before/passing-after revocation race and split external refresh from synchronous local validation
independent on c0262e6 a cleaned-up rebase failure was bound to the pre-rebase state version and appeared historical immediately fixed in 11c3853; added failing-before/passing-after coverage that the current actionable failure remains non-stale
independent on 11c3853 the merge-queue watermark preceded newly added asynchronous authorization/status reads, so another actor's queue lifecycle could be attributed to this attempt fixed in f94501b; added failing-before/passing-after final-boundary coverage and post-boundary local-authorization coverage; also repaired the production-faithful runner-start fixture exposed by exact-head CI
independent on f94501b the final queue watermark was not followed by PR/mode revalidation; rebase cleanup could start fresh timeout scopes outside the preparation deadline fixed in 3458a84; added failing-before/passing-after PR-refresh and live-run/cleanup budget regressions; no finding declined
independent on 3458a84 review linking started a fresh deadline after history reads; command checks rebuilt an unbounded Git-backed plan context only to obtain the allowlist fixed in f1eaea1; added failing-before/passing-after shared-deadline and direct-allowlist regressions; no finding declined
independent on f1eaea1 preparation readiness survived a changed command allowlist; live rebase work could consume the budget needed to abort and clear its marker fixed in 7780422; added failing-before/passing-after policy-binding and cleanup-reserve regressions; no finding declined
independent on 7780422 an unsettled rebase recovery marker advanced task state but left the newly current failure bound to the pre-rebase generation fixed in 1be0f9c; added failing-before/passing-after coverage that the retained recovery failure is current and non-stale; no finding declined
independent on 1be0f9c fetched collaborator-only PR commits were validated but left unreachable, so Git maintenance could prune SHAs still named by durable snapshots fixed in 81c9c1d; added failing-before/passing-after garbage-collection coverage and runner-owned retention refs; no finding declined
independent on 81c9c1d exit-0 command checks could fail only because diagnostics exceeded capture bounds or were invalid UTF-8; later preparation failures erased earlier completed check IDs fixed in 3f3fb43; added failing-before/passing-after bounded/lossy-diagnostic and durable partial-result regressions; no finding declined
Copilot on 1e6cb57 startup snapshot commits were pruneable; cleanup-only rebases left only 1 ms for Git work fixed in 629a933; added failing-before/passing-after startup ref-anchor and cleanup-window regressions; no finding declined
Copilot on a857be3 a live cleanup failure was thrown before rebinding, so its retained recovery marker made the actionable result stale fixed in 9a00e47; added a failing-before/passing-after abort-cleanup regression that retains the marker and keeps the failure current; no finding declined
independent on 9a00e47 preparation admitted unresolved runner cleanup; failed rebases adopted a concurrent review edit; already-aborted shutdown still loaded review fixed in 3adc132; added failing-before/passing-after unresolved-cleanup, original-review binding, and pre-load cancellation regressions; no finding declined
Copilot on 3adc132 summary-only concern: raw prefix truncation before the notice could split multibyte UTF-8, introduce U+FFFD, and exceed the byte cap reproduced failing before and fixed in d7109d8; added a production-container multibyte truncation regression asserting the intermediate decoded output and both byte limits; no finding declined
independent on e713ed7 no actionable defects clean full-diff pass on base 5ecf92f; 233 focused tests and type checking passed; no finding declined

The independent pass on base 26d03e6 / head fb2187d completed with the finding above. It was fixed in f4ab96b. A fresh full-diff independent pass at high effort then reviewed base 26d03e6c0e5d50c136ffc3e5c0d74cd3af52303e / head f4ab96bc413e2ee8783ecac1e3c38ef65198f0e1, found no actionable correctness issues, and passed 274 focused tests across 6 files. Copilot was requested only after that pass and exact-head CI were green. Its seven findings were fixed in d212471. Subsequent independent full-diff passes found and drove fixes for the local-trust race, stale failure bindings, queue-boundary ordering, final PR refresh, shared deadlines, command-policy binding, cleanup reservation, unsettled recovery, remote-commit reachability, command-output semantics, and durable partial check results. The local pass on 3f3fb43 was clean before that head was pushed. After main advanced to c822d36, the integration conflict preserved both G2's planning-screen status and F5's behavior/scope boundary; a new independent full-diff pass on c822d36..1e6cb57 was clean before the integrated head was pushed.

Self-review reread the full diff before each independent request. The only additional actionable finding was the pending-readiness exposure at 7b5c112; it was fixed in 2c5c910 and included in the next full-diff pass. The independent full-diff pass on base c822d360d5098c9f5f36347e00e5949708a5bf96 / head 1e6cb579f8535b2541dd2c56dbf4e84edbb54448 found no actionable correctness issues and passed 144 focused tests plus type checking and diff validation. After Copilot's two findings were fixed and main advanced again, a fresh high-effort independent full-diff pass on base 89707f86de76ca71060ae0a07496faf084d46088 / head a857be3834c8aea8b13b6b5c4b88c66e20f29cef found no actionable correctness issues and passed type checking plus 206 focused lifecycle tests. Copilot then found the cleanup-failure binding issue on a857be3; it was fixed locally in 9a00e47. The next independent full-diff pass found the three lifecycle gaps recorded above, all fixed in 3adc132. A final high-effort independent full-diff pass on base 89707f86de76ca71060ae0a07496faf084d46088 / head 3adc13226f7f114446c35b562fdcbb14ab21434a found no actionable defects and passed 233 focused tests plus type checking. Copilot's summary-only multibyte concern on 3adc132 was reproduced and fixed in d7109d8. When main advanced to 5ecf92f, the integration preserved Lane G and F5 documentation. A new high-effort independent full-diff pass on base 5ecf92f6960c17c40aa122c38bcf2f7b1c6106b5 / head e713ed7ce1aef02f99c98c33e9fa52b8032a6069 found no actionable defects and passed 233 focused tests plus type checking.

Review-lesson audit

  • lifecycle ownership, cancellation, deadlines, shutdown, and race findings are covered by Async jobs and polling and Required race regressions in AGENTS.md
  • exact base/head, approval, validation, and re-review findings are covered by Review readiness, External status confirmation, and Stacked pull requests
  • command normalization, bounds, trust, and subprocess findings are covered by Guarded external actions and Agent-controlled content
  • fresh-runner seeding was a one-off production assembly ordering defect; it does not justify a new general rule
  • no review finding was declined, and no new AGENTS.md rule merely repeats existing guidance

Gate status

  • independent exact-head review: clean on base 5ecf92f6960c17c40aa122c38bcf2f7b1c6106b5 / head e713ed7ce1aef02f99c98c33e9fa52b8032a6069; no actionable findings, 233 focused tests and type checking passed in the review; author validation also passed 357 affected tests across 8 files, type checking, and diff validation
  • push CI and pull-request CI: green on e713ed7ce1aef02f99c98c33e9fa52b8032a6069 (6m14s and 5m50s); each passed 74 non-Docker files / 1,976 tests, 137 browser tests, and type checking
  • remote real-Docker: green on e713ed7ce1aef02f99c98c33e9fa52b8032a6069; 81 files, 2,230 passed, 1 skipped in 1,128.53 seconds (19m05s), with workflow typecheck passed
  • local full-suite rerun: infrastructure-stressed as recorded above; remote exact-head full suite supplies the required clean gate
  • Copilot review: first round on f4ab96b returned seven findings, fixed in d212471; second round on 1e6cb57 returned two findings, fixed in 629a933; third round on a857be3 returned one finding, fixed in 9a00e47; fourth round on 3adc132 returned one summary-only concern, fixed in d7109d8; the current integrated head is independently clean with green exact-head remote gates and zero review threads unresolved, so the next review may now be requested
  • merge: not authorized and not attempted

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Post-rebase repository and approval handling can block preparation, while the final authorization await can leave merge-mode validation stale.

7 open findings
What changed in this PR

Implements F5 of #22, connecting the existing rebase engine to production merge preparation and exact-head command checks. Publishing rewritten heads and the final merge handoff remain for F6.

Changes:

  • Adds guarded preparation, review refresh, and durable readiness evidence.
  • Runs allowlisted commands in credential-free, no-egress containers.
  • Extends lifecycle regressions and updates implementation documentation.
File Description
web/​server.ts Wires preparation actions and shutdown.
test/​store.test.ts Tests readiness persistence and recovery.
test/​runner-rebase-conflict.test.ts Tests conflict-launch authorization.
test/​runner-publishing.test.ts Tests cancellation and shutdown integration.
test/​runner-production.test.ts Tests startup seeding and retry guards.
test/​runner-coordinator.test.ts Tests allocation, authorization, and timeout ownership.
test/​runner-command-script.test.ts Tests malformed command rejection.
test/​runner-checks.test.ts Tests head-bound check evidence.
test/​runner-branch-push.test.ts Tests exact-commit fetching.
test/​review.test.ts Tests repository selection and legacy commands.
test/​pre-merge.test.ts Covers preparation outcomes and races.
test/​plant.test.ts Tests derived command-policy isolation.
test/​plan-v1.test.ts Adds Unicode parser regression.
test/​merge-task-pr.test.ts Tests preparation and authorization gates.
test/​agent-policy.test.ts Tests exact command authorization.
test/​agent-network.test.ts Verifies runner host allowlist.
test/​agent-container.test.ts Tests credential-free container execution.
scripts/​plant.ts Clears inherited command authorization.
runner/​store.ts Persists readiness and command evidence.
runner/​review.ts Displays check results and selects repository.
runner/​rebase-conflict.ts Reauthorizes conflict-agent launches.
runner/​production.ts Assembles preparation and command execution.
runner/​pre-merge.ts Coordinates refresh, rebase, and checks.
runner/​merge.ts Enforces preparation and authorization gates.
runner/​coordinator.ts Composes dependencies and preserves timeout ownership.
runner/​checks.ts Implements exact-head command attempts.
runner/​branch-push.ts Fetches commits without moving refs.
README.md Updates capabilities and remaining work.
docs/​implementation/​runner-lifecycle.md Documents preparation lifecycle.
docs/​designs/​codeboost-plan-indexed-review.md Updates feature status.
docs/​architecture.md Documents F5 architecture and boundaries.
core/​plan.ts Rejects malformed Unicode commands.
agents/​policy.ts Authorizes the fixed command dispatcher.
agents/​network/​proxy.mjs Supports an explicitly empty allowlist.
agents/​network/​network.ts Adds the no-egress runner profile.
agents/​contract.ts Adds the runner invocation vendor.
agents/​container/​run.ts Rejects runner credentials.
agents/​container/​profile.ts Constructs credential-free profiles.
agents/​container/​probe.sh Validates runner isolation.
agents/​container/​Dockerfile Installs the command dispatcher.
agents/​container/​command-check.mjs Executes bounded argv lists without a shell.
agents/​adapters/​runner.ts Launches governed command-check containers.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread runner/merge.ts Outdated
Comment thread scripts/plant.ts Outdated
Comment thread runner/pre-merge.ts Outdated
Comment thread runner/pre-merge.ts
Comment thread runner/review.ts
Comment thread test/plan-v1.test.ts Outdated
Comment thread test/review.test.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread runner/production.ts
Comment thread runner/rebase.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A failed rebase cleanup leaves the retained recovery failure incorrectly marked stale.

1 open finding
2 resolved since last review

🧠 Review effort: Balanced

Comment thread runner/pre-merge.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Multibyte diagnostic truncation can violate the configured output byte limits.

0 open findings

1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Prevent UTF-8 corruption when truncating diagnostic output

agents/​adapters/​supervisor.ts:809

Appending the truncation notice can cut finalStderr in the middle of a multibyte UTF-8 sequence. The later toString('utf8') replaces that fragment with U+FFFD, so published diagnostics can exceed stderrBytes/combinedBytes despite the bounded-output contract. Trim the retained prefix through boundedDiagnostic before concatenating the notice, and add a multibyte truncation regression.

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Preparation can overlook unresolved rebase ownership, and a cancellation race can leave command checks running.

2 open findings

🧠 Review effort: Balanced

Comment thread runner/pre-merge.ts
Comment on lines +260 to +263
const runnerStatus = this.runner.status(identity);
if (runnerStatus.unresolved || this.runner.unreleased)
throw new GuardRefusal('Runner cleanup is unresolved; restart and recover owned resources before preparing a merge.');
let view = load(), task = this.service.store.getTask(identity);
Comment thread runner/pre-merge.ts
else if ((signal.reason as { code?: unknown } | undefined)?.code === 'ETIMEDOUT') this.runner.timeout(identity, attempt.id);
else this.runner.stop(identity, attempt.id, 'cancelled');
};
signal.addEventListener('abort', stop, { once: true });
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants