From a3b4fe943a2302a55580d519fa242598df53cdad Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Thu, 16 Jul 2026 11:53:46 -0400 Subject: [PATCH] docs: close lifecycle reliability plan --- docs/design-guidance.md | 11 ++- ...07-16-ratchet-cli-lifecycle-reliability.md | 2 +- ...et-cli-lifecycle-reliability.md.scope-lock | 1 - ...ratchet-cli-lifecycle-reliability-retro.md | 84 +++++++++++++++++++ 4 files changed, 94 insertions(+), 4 deletions(-) delete mode 100644 docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md.scope-lock create mode 100644 docs/retros/2026-07-16-ratchet-cli-lifecycle-reliability-retro.md diff --git a/docs/design-guidance.md b/docs/design-guidance.md index baccbc0..d433139 100644 --- a/docs/design-guidance.md +++ b/docs/design-guidance.md @@ -1,8 +1,10 @@ # Design Guidance **Status:** Active -**Last updated:** 2026-07-15 -**Source:** workspace guidance; `docs/retros/2026-07-15-provider-drain-managed-hooks-retro.md` +**Last updated:** 2026-07-16 +**Source:** workspace guidance; +`docs/retros/2026-07-15-provider-drain-managed-hooks-retro.md`; +`docs/retros/2026-07-16-ratchet-cli-lifecycle-reliability-retro.md` ## Product Direction @@ -36,6 +38,8 @@ - Run the exact merge-gating race/coverage selector locally when practical. Isolate heavyweight real-binary process smoke from race/coverage jobs so one test cannot consume the suite timeout. +- Bound cleanup and join paths independently. A timeout-triggered cancel or + process kill must not be followed by an unbounded `Wait` or channel receive. - Keep credentials, request payloads, command environments, and raw provider errors out of durable metadata, logs, snapshots, audits, and PR evidence. - Release only merge commits. Verify checksums, every declared platform @@ -55,6 +59,8 @@ - Exercise each runtime-integrated boundary through its real consumer: CLI or TUI to daemon, daemon to plugin/provider, and released archive or package to operating system. +- For asynchronous notifications, prove send completion and receiver handling + separately; use an explicit receiver barrier when claiming handler behavior. - Stateful flows prove results after restart or reload where feasible. Security boundaries include a negative authorization, trust, or mutation path. @@ -75,3 +81,4 @@ | Date | Source | Change | |---|---|---| | 2026-07-15 | `docs/retros/2026-07-15-provider-drain-managed-hooks-retro.md` | Established shared-contract, native-platform, settled-merge, exact-test, and release-runtime gates after repeated Windows and PR-monitoring misses. | +| 2026-07-16 | `docs/retros/2026-07-16-ratchet-cli-lifecycle-reliability-retro.md` | Added independent cleanup-join bounds and separate notification send/handler proof after ACP stress and PR review findings. | diff --git a/docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md b/docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md index 4c4faf2..c80b72a 100644 --- a/docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md +++ b/docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md @@ -30,7 +30,7 @@ | 1 | `fix: harden lifecycle cleanup and process smoke` | Task 1, Task 2, Task 3, Task 4, Task 5 | `fix/reliability-followups` | | 2 | `docs: close lifecycle reliability plan` | Task 6 | `docs/lifecycle-reliability-closeout` | -**Status:** Locked 2026-07-16T12:38:31Z +**Status:** Complete 2026-07-16T15:00:54Z ## Guidance Mapping diff --git a/docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md.scope-lock b/docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md.scope-lock deleted file mode 100644 index f75e7c2..0000000 --- a/docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md.scope-lock +++ /dev/null @@ -1 +0,0 @@ -0ffeb53f28790a0b6b87d941dcbe3d50a0a8c6daad67c810405c9b9219734a7f diff --git a/docs/retros/2026-07-16-ratchet-cli-lifecycle-reliability-retro.md b/docs/retros/2026-07-16-ratchet-cli-lifecycle-reliability-retro.md new file mode 100644 index 0000000..6200a00 --- /dev/null +++ b/docs/retros/2026-07-16-ratchet-cli-lifecycle-reliability-retro.md @@ -0,0 +1,84 @@ +# Retro: Ratchet CLI Lifecycle Reliability + +**PR:** #142 - fix: harden lifecycle cleanup and process smoke +**Merged:** 2026-07-16 +**Release:** `v0.30.36` +**Branch:** `fix/reliability-followups` +**Design:** `docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability-design.md` +**Plan:** `docs/plans/2026-07-16-ratchet-cli-lifecycle-reliability.md` +**Related ADRs:** None + +## Adversarial-review findings, scored + +| Phase | Finding | Severity | Outcome | +|---|---|---|---| +| design | D1: persistent cleanup failures could flood logs | Important | Resolved upfront: equivalent errors are suppressed for one minute and reset after recovery. | +| design | D2: moving real-start smoke did not replace its five-second bound | Important | Resolved upfront: the three lifecycle smokes use a shared 30-second transition bound and a five-minute CI command bound. | +| design | D3: Linux-only process proof conflicted with native-platform guidance | Important | Resolved upfront: the existing `windows-2025` job ran the same selector and passed. | +| design | D4: notification write and peer handling were conflated | Minor | Prescient: stress exposed the SDK's asynchronous handler goroutine; the design was backported and tests gained an explicit peer barrier. | +| plan | P1: canceled manager context could mask the intended query failure | Important | Resolved upfront: the closed-database test uses an unstarted manager and live context. | +| plan | P2: reverting a signature would prove compilation, not behavior | Important | Resolved upfront: revert proof preserved signatures and disabled close, query, and suppression behavior independently. | +| plan | P3: timeout cleanup could leak or block on child reap | Important | Prescient: it identified the unreaped-child risk and required kill-and-drain cleanup, but its remediation missed that the drain itself needed an independent bound; Copilot found the gap and `eb03f01` added a second bounded window. | +| plan | P4: a docs closeout PR creates a second patch release | Minor | Inconclusive: the overhead is real, but it preserves truthful post-merge evidence and the explicit release-per-merge policy. | + +## Gate misses + +| Issue | Gate that missed | Why it slipped | Fix idea | +|---|---|---|---| +| Copilot found an unbounded `<-child.done` after the process timeout and kill. | `adversarial-design-review` (plan), then `requesting-code-review` | P3 introduced kill-and-drain cleanup without bounding the drain; the downstream pre-PR review also missed that independent wait. | Require an independent bound around every timeout-triggered cleanup join or receive. | + +No PR or merge-commit CI failure occurred. The initial ACP exact-once stress +failure was caught locally before PR creation and corrected with a design +backport rather than a runtime sleep. + +## Missed skill activations + +The canonical `.claude/autodev-state/in-progress.jsonl` activation log is +unavailable. Committed artifacts and PR evidence prove outputs, not +hook-recorded activations. + +| Gate | Fired? | Notes | +|---|---|---| +| brainstorming | activation log unavailable | Design artifact exists. | +| adversarial-design-review (design) | activation log unavailable | Committed report contains D1-D4. | +| writing-plans | activation log unavailable | Locked six-task, two-PR plan exists. | +| adversarial-design-review (plan) | activation log unavailable | Committed report contains P1-P4. | +| alignment-check / scope-lock | activation log unavailable | Alignment artifact and verified lock exist. | +| subagent-driven-development | activation log unavailable | Task commits and verification evidence exist. | +| finishing-a-development-branch | activation log unavailable | PR #142 contains the required verification and scope evidence. | +| finishing Step 1e (doc-reconciliation) | yes | PR #142 records one corrected ACP scheduling assumption. | +| pr-monitoring | activation log unavailable | All checks settled green; one Copilot thread was fixed, replied to, resolved, and re-reviewed with no new comments. | +| post-merge-retrospective | activation log unavailable | This retro supplies the required output artifact. | + +## What worked + +- Behavioral revert proofs showed the cleanup tests detect dropped close, + query, and log-suppression behavior instead of only API drift. +- Repeated cancellation tests exposed the ACP SDK scheduling assumption before + PR creation; the correction preserved immediate teardown and exact receiver + proof without adding a sleep. +- Dedicated Linux and native Windows lifecycle selectors passed while + in-process race coverage retained the concurrency boundary. +- PR #142 and merge-commit `f1ff79a` CI settled green; release run + `29507752893` published `v0.30.36` with seven assets, six valid checksums, + matching Formula/Cask hashes, and a Homebrew-installed 22-provider runtime + proof. + +## What didn't + +- No retained independent reviewer output was available before PR creation; + the downstream Copilot review caught the missing post-kill wait bound. +- The pre-PR review path failed to carry P3 through to an independently bounded + drain even though it was the exact cleanup path under scrutiny. + +## Plugin-level follow-ups + +No plugin-level change is warranted from this single review miss. The post-kill +wait gap is captured as project guidance; another occurrence should promote it +into the shared code-review checklist. + +## Project guidance updates + +| Guidance file | Change | Reason | +|---|---|---| +| `docs/design-guidance.md` | updated | Future lifecycle designs must bound cleanup joins independently and distinguish notification send completion from receiver handling. |