-
Notifications
You must be signed in to change notification settings - Fork 1.1k
docs(devlog): record the Wave 5 WP5 triage outcome (#1849 split, #1049 assessed) #1943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
526aef8
2d3de03
66905cc
3dcac90
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
|
|
||
| ## #1849 — split, do not auto-close | ||
|
|
||
| v2.24.2 shipped #1877, which stops the service restart loop when the install is | ||
| v2.24.0 shipped #1877 (merge `01b212579`), which stops the service restart loop when the install is | ||
| missing. That is the mitigation half. The root defect is the non-atomic update | ||
| transaction: download → delete existing files → copy new files, which leaves a | ||
| file-less package skeleton when it fails midway. | ||
|
|
@@ -64,3 +64,76 @@ stops at `ambiguous` rather than guessing. | |
| #1849 stays OPEN as the transactional-updater issue with A recorded as shipped. | ||
| #1049 closes only on landed adoption + crash-recovery evidence; otherwise it is | ||
| reported as a real terminal outcome, not silently dropped. | ||
| ## Outcome (executed) | ||
|
|
||
| **#1849 — split, umbrella kept open.** The report carried two defects with different fixes | ||
| and different closure conditions: | ||
|
|
||
| | Half | Defect | Status | | ||
| |---|---|---| | ||
| | A | Service wrapper restart-loops against a missing install | Fixed by #1877, released in v2.24.0 (`git tag --contains 01b212579`) | | ||
| | B | `ocx update` deletes the working install with no rollback | **#1942**, open | | ||
|
|
||
| #1849 stays open as the umbrella so the original evidence — 107 directories / zero files, every | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Escape issue identifiers at the start of paragraphs.
Proposed Markdown fix-#1849 stays open as the umbrella
+\`#1849` stays open as the umbrella
-#1049 is unstarted because doing it correctly
+\`#1049` is unstarted because doing it correctlyAlso applies to: 137-137 🧰 Tools🪛 markdownlint-cli2 (0.23.2)[warning] 77-77: No space after hash on atx style heading (MD018, no-missing-space-atx) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
| launcher gone from `%APPDATA%\npm`, 1,009 restarts over 89 minutes — stays attached to the | ||
| story instead of being split away from it. #1942 carries the transactional shape, the ordering | ||
| constraint (never delete the existing install before the replacement is verified), and the | ||
| kill-at-every-boundary matrix. | ||
|
|
||
| **#1049 — assessed, not started, and the reason is in the issue.** The gap is exactly as | ||
| described: `codexWriteCoordinationEligibility` returns `legacy-uncoordinated` for routed homes without a | ||
| coordinator row, routing around a refusal that is itself correct. | ||
|
|
||
| What made me stop is the size of what is missing. `grep -r 'adoption-pending' src/` returns | ||
| **zero** matches against **37** in `devlog/_fin/260804_codex_write_substrate/`. The design is | ||
| complete and none of it is built: the complete-database temp publisher, atomic no-clobber | ||
| publication (exclusive hard link or rename-without-replace, with an ordinary replacing rename | ||
| forbidden), fsync before publication and parent-directory fsync after, `adoption-pending` | ||
| accepted as a ready state by ordinary openers, and the positive-authority gate. | ||
|
|
||
| This is crash-safety machinery for durable state in the user's Codex home. A subtly wrong | ||
| publication corrupts an install rather than failing a test, and the contract's crash boundary | ||
| is the kind of property that is implemented completely with its fixture matrix or not at all. | ||
| Landing a partial version inside a wave next to unrelated fixes would be the worst option | ||
| available. | ||
|
|
||
| Terminal outcome for #1049: **NEEDS_HUMAN on scheduling** — well-specified, unstarted, and | ||
| deserving its own work-phase. | ||
| ### Corrections from the WP5 audit | ||
|
|
||
| Three, all folded: | ||
|
|
||
| 1. **Wrong symbol name.** I cited `decideCoordination`; it greps to nothing. The real export | ||
| is `codexWriteCoordinationEligibility` (`src/codex/inject-coordination.ts`). The described | ||
| behaviour was right — only the name was wrong, which is the one error that costs the next | ||
| reader time in a note written to save them the rediscovery. | ||
| 2. **#1942 overstated the gap.** It said nothing verifies the post-install tree can boot. | ||
| Verification exists — `checkUpdatePackageIntegrity` fails closed before the proxy stops, | ||
| `isRealBunBinary` rejects the postinstall stub, and `awaitRestartedProxyHealthy` runs an | ||
| identity-checked probe. The accurate claim is narrower and worse: it is **detect-only and | ||
| runs after the old install is gone**, so a failed probe has nothing left to restore. | ||
| 3. **The #1049 gap is larger than recorded, which strengthens the defer.** The contract forbids | ||
| opening a missing final path with SQLite `create:true`; `src/codex/transition-state.ts` does | ||
| exactly that today. So the temp-publisher work rewrites the create path **every clean | ||
| install** uses — the blast radius is the whole installed base, not the legacy subset. | ||
|
|
||
| And one scoping correction worth carrying forward: the deferral was right, but the monolith | ||
| framing was not. A prerequisite subset stands alone — the complete-temp-database publisher plus | ||
| atomic no-clobber publication, **ordinary clean row only**, with no `adoption-pending`, no | ||
| authority gate, no callback plumbing. It closes nothing by itself, but it removes the visible | ||
| zero-byte window on first create for every user and splits the remaining work into two | ||
| reviewable phases instead of one. | ||
| ### Where this work now lives | ||
|
|
||
| | Tracker | Scope | State | | ||
| |---------|-------|-------| | ||
| | #1849 | Umbrella; holds the original evidence (107 dirs / 0 files, missing launchers, 1,009 restarts) | open | | ||
| | #1877 | Service wrapper stops restart-looping a missing install | merged `01b212579`, released v2.24.0 | | ||
| | #1942 | Transactional updater: stage → verify → snapshot → atomic switch → probe → restore | open, unstarted | | ||
| | #1049 | Coordinator adoption for pre-substrate homes | open, unstarted, needs its own phase | | ||
|
|
||
| The two open items are unstarted for different reasons, and the distinction matters when | ||
| someone picks them up. #1942 is unstarted because it is ordinary work nobody has done yet. | ||
| #1049 is unstarted because doing it correctly means rewriting the coordinator create path | ||
| that every clean install already uses — a decision about blast radius, not a backlog | ||
| position. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add blank lines before the new headings.
markdownlint-cli2reports MD022 because these headings follow the preceding paragraphs without a blank line. Insert one blank line before each heading.Proposed Markdown fix
reported as a real terminal outcome, not silently dropped. + ### Corrections from the WP5 auditApply the same spacing before
## Outcome (executed)and### Where this work now lives.Also applies to: 102-102, 126-126
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 67-67: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Source: Linters/SAST tools