Repository navigation
W3-03c: crash/power-loss drill kit - #379
Conversation
Before/after snapshots of run, launch and delivery state from a copy of the database, a restart judge (reconciling, no redispatch, no silent change), PowerShell launcher and German steps. Test-First: scripts/lib/crash-drill.test.mjs::an open run that becomes reconciling passes Test-First: scripts/lib/crash-drill.test.mjs::an open run left open or completed silently fails Test-First: scripts/lib/crash-drill.test.mjs::a new run for a task with a reconciling run is a redispatch Test-First: scripts/lib/crash-drill.test.mjs::a delivery must not advance without a process and a drill needs an open run Test-First: scripts/lib/crash-drill.test.mjs::before and after bundles read a copy and pass for a reconciled run Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e808b074d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const state = snapshotState(live); | ||
| bundle.addFile('state.json', JSON.stringify({ transition, ...state }, null, 2)); | ||
| bundle.step('snapshot state after the kill (copy, read-only)', { detail: `transition ${transition}; ${state.runs.length} runs` }); |
There was a problem hiding this comment.
When the user supplies any of the six transition labels, runBefore merely writes that string beside a database-wide snapshot, while judgeRestart never uses it. Consequently, a run killed during intent but labeled checkpoint can pass once any open run becomes reconciling; claimed and completion can likewise be falsely represented by an unrelated open run. Bind the evidence to a specific run and verify the corresponding launch, delivery, or checkpoint state before accepting the label.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
| for (const s of ['', '-wal', '-shm']) if (existsSync(dbPath + s)) copyFileSync(dbPath + s, join(tmp, `projecta.db${s}`)); | ||
| const con = new DatabaseSync(join(tmp, 'projecta.db'), { readOnly: true }); |
There was a problem hiding this comment.
During the documented after phase ProjectA is running, so it can commit or checkpoint between these three independent copies of the main database, WAL, and shared-memory files. The resulting temporary set may combine different SQLite states, producing an error, stale rows, or a misleading restart verdict. Use a coherent SQLite online snapshot or quiesce the writer before using this data as runtime evidence.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
| export const TRANSITIONS = ['claimed', 'intent', 'process-starting', 'input-delivery', 'checkpoint', 'completion']; | ||
| const OPEN = ['intent', 'launched']; | ||
| const QUERIES = { | ||
| runs: 'SELECT id, task_id, status, claim_fence, process_id, terminal_detail FROM development_runs ORDER BY id', |
There was a problem hiding this comment.
When any historical blocked run has an agent-supplied failure reason containing a bare credential, this query exports it through terminal_detail into state.json and the after bundle. The shared redactor only recognizes named key/value or authorization-header forms, so a value such as a bare ghp_... token survives unchanged despite the documentation saying the bundle is redacted. Omit this arbitrary field or apply fail-closed secret sanitization before persisting it.
AGENTS.md reference: AGENTS.md:L44-L46
Useful? React with 👍 / 👎.
| if (processList) bundle.addFile('processes-after.txt', processList); | ||
| const problems = judgeRestart(before, after); | ||
| bundle.step('judge restart: reconciling, no redispatch, no silent change', { command: `transition ${before.transition}`, exitCode: problems.length ? 1 : 0, detail: problems.join('; ') || 'ok' }); |
There was a problem hiding this comment.
During both phases the launcher captures a process list, but runAfter only stores it and the verdict checks database run IDs rather than OS processes or launch identities. A provider child can therefore survive the ProjectA kill, or a process can restart under the existing run, while that run becomes reconciling; the drill still reports pass even though its central claim is that nothing restarted before process and database state agree. Compare the recorded run/process identity with both process snapshots and fail when the old process remains or a replacement appears.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
| const delivery = new Map(after.deliveries.map((d) => [d.run_id, d.state])); | ||
| for (const d of before.deliveries) if (d.state === 'started' && delivery.get(d.run_id) === 'enqueued') problems.push(`run ${d.run_id}: delivery went started -> enqueued without a live process`); |
There was a problem hiding this comment.
If a committed delivery row present before the restart is missing afterward, delivery.get(d.run_id) is undefined, so this condition does not report any problem; launch rows and checkpoint counts are not compared at all. The drill can therefore return pass after losing the durable launch, delivery, or checkpoint record that the selected crash transition was meant to exercise, provided the run itself becomes reconciling. Compare the target run's durable records across snapshots and reject deletions or regressions.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
|
Queued — the merge queue status continues in this comment ↓. |
Test-First: scripts/lib/crash-drill.test.mjs::a failed run cannot pass because it releases the redispatch block
Test-First: scripts/lib/crash-drill.test.mjs::a failed run cannot pass because it releases the redispatch block
|
@Mergifyio queue |
Merge Queue Status
This pull request spent 26 minutes 56 seconds in the queue, including 14 minutes 16 seconds running CI. Required conditions to merge
|
Dieses Paket liefert den Absturz-Drill für deinen Windows-PC: Du beendest ProjectA hart an einem kritischen Übergang, startest es neu, und das Skript prüft, ob der Lauf auf „reconciling" steht und nichts neu gestartet wurde. Deine Datenbank wird nur über eine Kopie gelesen, Tokens werden geschwärzt. Die Schritte stehen auf Deutsch in
docs/drills/crash-drill.md; ohne Continuous findet der Drill keinen offenen Lauf und zählt dann als „nicht abgedeckt".Report
What changed
scripts/lib/crash-drill.mjsfor state snapshots from a temporary database copy, restart judgement, and before/after evidence bundles.reconciling; terminalfailedno longer counts as safe because it releases the active-run redispatch barrier.Evidence
node --test scripts/lib/crash-drill.test.mjsbefore the review fix -> exit 1 (5 passed, the new failed-run regression test failed as expected).node --test scripts/lib/crash-drill.test.mjsat02b3e33-> exit 0 (6 passed).bash scripts/ci/red-first.shat02b3e33-> exit 0 (7 exact test specifications red/missing at base and green at head).bash scripts/ci/gates.sh lane prepushat02b3e33-> exit 0 (229 s; frontend 480/480, HQ 533/533, Rust 1769/1769 with 7 skipped).PA_PREPUSH=light git push -u origin claude/w3-03c-drill-crash-> exit 0 (branchpush lane green).git ls-remote origin refs/heads/claude/w3-03c-drill-crash-> exit 0, remote SHA02b3e338731e9edeaecac3a9a240c89a153ea98e.NICHT ABGEDECKT
#[cfg(windows)], the.ps1launcher is not run on Windows here); the Windows lane runs in the merge queue, and the drill itself is run by the user on the installed app.development_runs,development_launches,development_deliveries,development_capture_checkpoints) were checked against the store source, not an installed-app database; missing tables yield empty lists.Architecture drift
No new finding. The change stays in the existing Node drill/bundle pattern and introduces no frontend state, Tauri result/error, Store/lock, IPC, or Rust-to-frontend event pattern.
Review disposition
4e808b074dd67ddc9fa0016f40969ee8eeed2a4c; fix head and delta rechecked at02b3e338731e9edeaecac3a9a240c89a153ea98e.scripts/lib/crash-drill.mjs:38: the candidate accepted an interrupted run that becamefailedwith any detail. Countercheck againstdevelopment_runs_one_active_taskand startup reconciliation showed that onlyreconcilingretains the active-run barrier; the candidate could therefore report pass while a new run for the same task was allowed. Accepted with red-first test commit527a761and fix commit02b3e33.Open points