feat(provision): apply/undo orchestration + run records (U10 U5) - #52
Merged
Merged
Conversation
U3 (render/diff, #47) + U4 (target registry + Cursor scanner, #46) merged via the inverted Codex-builds/Claude-reviews dogfood loop (decisions #55-#56). START-HERE ▶ NEXT → /unit-loop U5; PRODUCT 2/7 → 4/7; DECISIONS #55 (inverted loop, proven) + #56 (codex-gate doesn't fit worktree/inverted PRs — dogfood finding, fix deferred). Post-merge main green: 580 tests, tsc clean.
The Act half of the provisioning engine: ordered abort-all batch apply with LIFO rollback, journal-derived run records, and batch undo. apply.ts composes the pure U3 render+diff, the U4 target registry, and the U14 config-write engine behind a preflight-then-write discipline — an incompatible/unresolvable/ duplicate-destination target aborts before any file is written; a genuine mid-write fault rolls this batch's earlier writes back newest-first; an applied-but-unjournaled write is annotated un-undoable and survives honestly in the outcome. runs.ts derives batches from the undo journal (no second record file, fresh-process recoverable) and reverses them LIFO with per-file superseded/failed classification. Why LIFO everywhere: a blueprint may write one destination twice (two roles config-merging into a shared .cursor/mcp.json), forming a postHash chain that must reverse newest-first. Why the duplicate-destination guard: 2+ non-mergeable (whole-file) writers to one destination is a silent last-writer-wins blueprint — refused loudly at preflight; all-config-merge collisions still compose. 602 tests green, tsc clean.
… (U10 U5) Codex adversarial gate folds: - #43 effective-form secret scan (its promotion trigger — "before U5/apply ships" — fired here): apply now scans the DECODED form of every parser-consumed (json/toml) write, so a value secret hidden behind a JSON \uXXXX escape that the raw front-gate misses is refused before any write. Scans only the patch we contribute, never foreign disk content. Fails closed on unparseable sources. - duplicate-destination guard now groups ALL rows (incl. current-noop), and rejects conflicting config-merge patches (overlapping leaf, different value) — closing the oscillation; disjoint same-dest config-merge still composes. - transform-vs-surface-shape guard: a whole-file copy/compose/scaffold aimed at a merge-shaped surface (.cursor/mcp.json etc.) is refused before it can clobber the user's foreign config. Deferred (docstring-narrowed, honest): default undo targets the newest JOURNALED batch, not the newest SUCCESSFUL one — a failed+rolled-back batch is the default undo target and reports an honest no-op; durable fix lands with U6's CLI undo. 611 tests green, tsc clean.
… (U10 U5) Codex adversarial gate round 2 folds: - Blueprint-validity checks (shape guard + secret scan) now run for EVERY resolving row BEFORE the skip-continue — validity is a property of the blueprint, not of current disk. A whole-file copy to a merge surface, or a copy of a secret-bearing source, that currently no-ops is no longer accepted just because the target already matches. - The secret scan now RAW-scans whole-file payloads (closes the text-format gap: CLAUDE.md/AGENTS.md/role .md files) in addition to the decoded parser-format scan — apply is egress defense on the bytes it writes. (Keyword-only secrets in a config-merge source stay the verb front-gate's job — named in-comment.) - undo() returns a "reversed"|"noop" disposition; runs.ts routes an idempotent no-op to a new alreadyReversed bucket instead of falsely claiming reversed — making the deferred failed-batch undo an honest no-op in fact. - Supersession is now decided by JOURNAL ORDERING (a later, different batch that wrote the same path), not a live byte-hash — closing an A->B->A content-cycle clobber where the old hash-check would delete/restore over a newer batch. Cross-batch only, so the same-batch LIFO chain still reverses intact. Deferred to #50/U6 (durable batch terminal-state): the fail-OPEN precision that would let an old-batch undo proceed once a later batch is proven itself reversed. 616 tests green, tsc clean.
…ssion (U10 U5) Codex gate round 3 — surface-reducing folds (the last fold-and-rerun cycle): - Reject a non-provisionable-harness or unresolvable-destination row for EVERY row, including a noop/scaffold-skip whose bytes match disk — completing round 2's "validity is a property of the blueprint, not of current disk" class-fix (it previously still leaked for harness/resolve). - Journal-ordering supersession now keys on the posix-normalized absolute targetPath ALONE, dropping the redundant unnormalized projectRoot from the key, so equivalent root spellings (/p, /p/, /p/x/..) collapse and a later batch under a different spelling still supersedes — closing an alias-bypass ABA clobber. Deferred to #50/U6 (exotic-state, per plan's write-path gate budget), narrowed honestly in-code: symlink-root aliases (need realpath canonicalization at the registry ingress); an unjournaled write buried by a later same-path config-merge in one batch (needs path-poisoning); undo's created-file existsSync mapping EACCES to alreadyReversed (needs guarded lstat in the shared U14 primitive). 619 tests green, tsc clean.
…(U10 U5) Codex gate round 4: undo batch SELECTION matched the stored projectRoot by exact string, so applying under /p/ then undoing under /p missed the batch — default undo could reverse an OLDER batch and leave the latest applied. Canonicalize the root (posix.normalize + strip trailing slash, which normalize keeps) once in a shared helper, applied at apply ingress (canonical storage) and on both sides of every batch-selection comparison (covers legacy journals + spelling mismatch). Closes the ordinary lexical case; symlink-root realpath stays deferred (#51). Deferred (unchanged): R4 decoded machine-path scan is best-effort per spec and needs crafted encoded input — the R4 twin of the #43 secret gap, filed context in #51's neighborhood. 620 tests green, tsc clean.
… (U10 U5 learning) The U5 gate caught issue #43 (effective-form secret scan, trigger 'before U5/apply ships') that the brief + ce-work + 9-persona review + simplify all missed — because they reason from the plan and code, not from a separate deferred issue. Convention: at unit-loop entry, grep open deferred issues for promotion triggers naming this unit and fold them into the ce-work brief.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds all-or-nothing provisioning apply orchestration, explicit undo dispositions, journal-based batch discovery and undo, supersession protection, extensive integration tests, and updated U5 execution and project-status documentation. ChangesProvisioning apply and undo
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Apply
participant Engine
participant Journal
participant BatchUndo
Caller->>Apply: apply blueprint
Apply->>Engine: write or merge validated rows
Engine->>Journal: record mutations
Apply-->>Caller: ApplyOutcome
Caller->>BatchUndo: undoBatch project
BatchUndo->>Journal: read and group entries
BatchUndo->>Journal: classify supersession
BatchUndo-->>Caller: UndoOutcome
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements the “Act” half of the U10 provisioning engine by adding an abort-all apply orchestrator with LIFO rollback, plus journal-derived run/batch records and batch undo. It composes the existing render+diff pipeline, target registry compatibility checks, and the config-write engine into a single write path with deterministic recovery behavior.
Changes:
- Add
apply()orchestration with preflight-then-write, secret egress scanning (including effective/decoded-form scanning for config formats), and LIFO rollback with unjournaled-write honesty. - Add journal-derived run records (
readBatches,newestBatchForProject) andundoBatch()with cross-batch supersession protection and per-file outcome classification. - Expand undo semantics to return an explicit disposition (
"reversed"vs"noop") and add comprehensive apply/undo tests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/provision-apply.test.ts | New end-to-end tests covering apply/rollback/undo behavior, journal-derived runs, and secret egress defenses. |
| src/provision/apply.ts | New apply orchestrator with preflight validation, secret scanning, write pass, and LIFO rollback behavior. |
| src/provision/runs.ts | New run/batch reader and batch undo implementation derived purely from the undo journal. |
| src/provision/internal.ts | Adds project-root canonicalization and shared error formatting helpers used by provision modules. |
| src/configwrite/undo.ts | Changes undo() to return an explicit disposition to support honest no-op reporting. |
| src/configwrite/index.ts | Updates public exports to include UndoDisposition and hashContent. |
| docs/START-HERE.md | Updates the project status narrative to reflect U3+U4 shipping and U5 as next. |
| docs/solutions/conventions/check-promotion-triggered-deferred-issues-at-unit-entry.md | New workflow convention doc for promotion-triggered deferred issues. |
| docs/PRODUCT.md | Updates product status to reflect U10 progress (4/7 sub-units shipped; U5 next). |
| docs/DECISIONS.md | Adds decision ledger entries for U3+U4 shipping and related gating/worktree notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+30
to
+44
| function groupBatches(entries: readonly UndoEntry[]): ProvisionBatch[] { | ||
| const order: string[] = []; | ||
| const byId = new Map<string, ProvisionBatch>(); | ||
| for (const entry of entries) { | ||
| if (!entry.batchId || !entry.projectRoot) continue; | ||
| let batch = byId.get(entry.batchId); | ||
| if (!batch) { | ||
| batch = { batchId: entry.batchId, projectRoot: entry.projectRoot, entries: [] }; | ||
| byId.set(entry.batchId, batch); | ||
| order.push(entry.batchId); | ||
| } | ||
| batch.entries.push(entry); | ||
| } | ||
| return order.map((id) => byId.get(id)!); | ||
| } |
…, PR #52) groupBatches keyed only on batchId, but the module treats the journal as untrusted (validated on read for hand-edits/corruption). A corrupted journal reusing a batchId under a different projectRoot could let undoBatch reverse another project's paths. Drop any entry whose canonicalized root doesn't match the batch's established root — a legit batch's entries all share the ingress-canonicalized root, so this only excludes corrupt rows. 621 tests green, tsc clean.
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.
U10 U5 — Apply/undo orchestration + run records
The Act half of the provisioning engine (plan,
### U5). Composes the pure U3 render+diff, the U4 target registry, and the U14 config-write engine into an abort-all batch apply with LIFO rollback, plus journal-derived run records and batch undo.src/provision/apply.ts— preflight-then-write: an incompatible / unresolvable / duplicate-destination / secret-bearing / merge-surface-clobbering row aborts before any file is written; a genuine mid-write fault rolls this batch's earlier writes back newest-first; an applied-but-unjournaled write is annotated un-undoable and survives honestly in the outcome.src/provision/runs.ts— batches derived from the undo journal (no second record file, fresh-process recoverable); LIFO reversal with per-filereversed/alreadyReversed/superseded/failedclassification; journal-ordering supersession (byte-independent, ABA-proof).Review + gate journey
Full loop: ce-work (opus) → ce-simplify (3 reviewers, 7 fixes) → ce-code-review (9-persona roster + security + reliability + adversarial) → Codex adversarial gate — 4 fold rounds. The gate earned its keep: round 1 caught a real security gap — issue #43's effective-form secret scan, whose promotion trigger was literally "before U5/apply ships" (U5/apply is the first code that writes parser-consumed formats to disk), which the brief + ce-work + all 9 personas + simplify had missed. Later rounds folded whole-file clobber guards, disk-independent blueprint validity, an A→B→A undo-clobber fix (journal-ordering supersession), and project-root canonicalization for undo batch selection. Learning compounded →
docs/solutions/conventions/check-promotion-triggered-deferred-issues-at-unit-entry.md.Per the plan's write-path gate budget ("budget for 2+ rounds and use scoped defers with triggers rather than a third patch"), the gate's remaining findings are exotic-state-conditional, unreachable in normal single-user use, and deferred (each narrowed honestly in-code, filed with a promotion trigger). Codex confirmed the final production code has no newly established defect beyond these:
existsSyncmapping EACCES toalreadyReversed; the R4 decoded machine-path scan (best-effort per the settled Linear-time secret classifier (ReDoS: assignment + JWT patterns are O(n²) on pathological input) #42/Effective-form secret scan of config-format sources (copy + merge) before apply writes them #43 ruling; needs crafted encoded input); and the three unjournaled-write tests modelling outcome-shape rather than a genuinely-missing journal row.None is a data-loss or security path in ordinary operation; the durable fixes land where the model they touch is actually built (U6). Ship-then-defer was authorized (fold finding 1, defer the rest).
Summary by CodeRabbit