Skip to content

Revert "fix(writeback): guard onApplied on the write and settle post-spawn waits (#322)" — main is red - #352

Merged
khaliqgant merged 1 commit into
mainfrom
revert/322-broke-main
Aug 23, 2026
Merged

Revert "fix(writeback): guard onApplied on the write and settle post-spawn waits (#322)" — main is red#352
khaliqgant merged 1 commit into
mainfrom
revert/322-broke-main

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Aug 23, 2026

Copy link
Copy Markdown
Member

main is red and this reverts the merge that broke it — mine.

Evidence

CI on main, in order:

commit what CI
05552c93 #340 ai-review workflow success
b7c68462 #343 read-only CLI fix failure — but a different test (reaps a first dispatch whose spawn never returns (#303), the known #342 flake in factory.test.ts)
cb6044a1 #345 release 0.1.70 success
ddf6486d #322 failure

cb6044a1 green → ddf6486d red, with #322 as the only change between them. That is a clean before/after control.

The four failures are all in src/cli/fleet.test.ts, none of them the #342 flake:

FAIL  waits for its in-flight park confirmation ('acked')
      AssertionError: expected 3 to be +0
FAIL  does not attribute a third-party GitHub close from an in-flight 'explicit acknowledged' receipt
FAIL  does not attribute a third-party GitHub close from an in-flight 'legacy void' receipt
      AssertionError: expected [ 'issue-done', 'issue-done' ] to include 'live dispatch state changed'
FAIL  serializes terminal completion that starts after the ready read against the dispatch claim

Why it happened

#322 was green on its own head (88cfd410, all 5 jobs). It went red only once merged. main had moved underneath it — #343 landed at 18:50, #322 merged at 19:35, and #322's last CI ran against its own base. Two individually-green sides combined into a red result, which is the seam #346 independently identified: #329 moved durable ownership to dispatchLifecycleKey while #322's squash armed its post-spawn maps on issueKey.

This is my miss. I verified #322's own seams and its per-job green, but not against main's newer state, and I merged on a run whose base was already stale.

Why revert rather than roll forward

#346 does fix this and is green — but it is now +1039/-29 across 10 files with three live review threads, and it expanded rather than splitting as I ruled. Landing 1000 lines of unreviewed compensating-rollback logic to un-red main is the wrong trade when a revert restores green in one small, fully-reversible step.

A red main is not a cosmetic problem right now: #347 (reclaim inversion) and #349 (dry-run regression fix) are both showing package: failure with exactly these four tests, inherited from their base. Neither lane broke anything. Until main is green, no PR in this repo can demonstrate its own correctness, and every author pays to re-diagnose the same inherited red — which is how a repo learns to ignore failing checks.

Nothing is lost

#322's work returns via #346, whose job is now "re-land #322 correctly, split into PR A (key unification) and PR B (rollback, designed)". The split ruling stands and matters more after this, not less.

Also worth stating plainly: none of this is in production. v0.1.70 is cb6044a1, and ddf6486d is not an ancestor of it. The deployed container is 0.1.69 plus the read-only fix. This revert changes nothing about the live cutover or the current readinessReconcile stall (#351).

Merging on per-job green.


Summary by cubic

Reverts the recent writeback and post‑spawn coordination changes to restore a green main. This returns the orchestrator and GitHub writeback API to their prior behavior and removes the code paths that broke the build.

  • API: GithubWriteback.setStatus and closeIssue return Promise<void> again; remove GithubStatusWriteResult and GithubIssueCloseWriteResult and their exports from src/ports/writeback.ts, src/ports/index.ts, and src/index.ts.
  • Orchestrator: remove post‑spawn issue observation and claim‑fence logic, in‑flight issue‑writeback tracking, and shutdown/abandonment settlements; simplify issueWritebackConfirmedAtMs handling; revert dispatch/completion flow in FactoryLoop.
  • Writeback: remove audit‑based authorship receipts and GitHub events parsing; keep provider read‑backs to assert label/close outcomes; simplify src/writeback/github.ts.
  • Tests: drop deterministic post‑spawn/ownership tests and adjust writeback tests for the reverted API.
  • Rollout: no migration required; the reverted API was not released.

Written for commit 6bf6afe. Summary will update on new commits.

Review in cubic

@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

Requested for exact head 6bf6afea562e4787b6dc207fd19872e36fed41df.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 43 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5111f9b8-16fa-4628-9505-8e8b73d96794

📥 Commits

Reviewing files that changed from the base of the PR and between ddf6486 and 6bf6afe.

📒 Files selected for processing (8)
  • src/cli/fleet.test.ts
  • src/index.ts
  • src/orchestrator/factory.test.ts
  • src/orchestrator/factory.ts
  • src/ports/index.ts
  • src/ports/writeback.ts
  • src/writeback/github.ts
  • src/writeback/writeback.test.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@khaliqgant
khaliqgant merged commit f06267c into main Aug 23, 2026
9 checks passed
@khaliqgant
khaliqgant deleted the revert/322-broke-main branch August 23, 2026 21:16
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.

1 participant