d2e0ddcd
Close the lease branch follow-up actions
a73x 2026-09-06 09:50
Commit message
CLAUDE.md
| Old | New | ||
|---|---|---|---|
| @@ -79,10 +79,10 @@ Each of these exists because of a specific failure; the date is when it bit. | |||
| 79 | 79 | ||
| 80 | - **Never touch the index or HEAD to answer a question.** No | 80 | - **Never touch the index or HEAD to answer a question.** No |
| 81 | `stash`/`switch`/`reset` to check whether a failure pre-dates your work — use | 81 | `stash`/`switch`/`reset` to check whether a failure pre-dates your work — use |
| 82 | `git worktree add --detach /tmp/base-check <ref>`. Under Delta's replicating | 82 | `make base-check REF=<ref>`, which wraps a detached worktree and cleans it up. |
| 83 | VFS a broken `&&` chain left the tree clean and the work in `stash@{0}`, | 83 | Under Delta's replicating VFS a broken stash-and-switch chain left the tree |
| 84 | twice in one session, the second time two hours after writing the lesson down | 84 | clean and the work in `stash@{0}`, twice in one session, the second time two |
| 85 | (2026-09-06). | 85 | hours after writing the lesson down (2026-09-06). |
| 86 | - **Branch before the first commit of a slice.** Delta's Changes view diffs the | 86 | - **Branch before the first commit of a slice.** Delta's Changes view diffs the |
| 87 | thread's branch against its base, so committing onto `main` renders an empty | 87 | thread's branch against its base, so committing onto `main` renders an empty |
| 88 | diff while the work piles up invisibly (2026-09-06, five commits). | 88 | diff while the work piles up invisibly (2026-09-06, five commits). |
| @@ -103,6 +103,10 @@ Each of these exists because of a specific failure; the date is when it bit. | |||
| 103 | - **Ask the outside about the inside.** A component reporting on itself cannot | 103 | - **Ask the outside about the inside.** A component reporting on itself cannot |
| 104 | catch itself being wrong: keys come from `ssh-keygen`, not our own encoder; | 104 | catch itself being wrong: keys come from `ssh-keygen`, not our own encoder; |
| 105 | interop is asserted through the real `ssh` client and real `git`. | 105 | interop is asserted through the real `ssh` client and real `git`. |
| 106 | - **Test ids as users receive them.** At least one side of an id-taking command | ||
| 107 | pair uses the abbreviation printed by the CLI. Full-id-only lease tests | ||
| 108 | protected a release that reported success without freeing anything | ||
| 109 | (2026-09-06). | ||
| 106 | - **Capture `$?` before piping.** `cargo test | grep -c ok` reports grep's exit | 110 | - **Capture `$?` before piping.** `cargo test | grep -c ok` reports grep's exit |
| 107 | status, not the build's. | 111 | status, not the build's. |
| 108 | - **A green suite is not user acceptance.** Say what was verified and what was | 112 | - **A green suite is not user acceptance.** Say what was verified and what was |
RETRO.md
| Old | New | ||
|---|---|---|---|
| @@ -106,11 +106,11 @@ judgement or process. | |||
| 106 | the next section; I had attributed it to a `matches reject pattern` line | 106 | the next section; I had attributed it to a `matches reject pattern` line |
| 107 | that a *passing* governance test prints, without verifying it. It was a | 107 | that a *passing* governance test prints, without verifying it. It was a |
| 108 | control-socket path overflow, and a real product bug. | 108 | control-socket path overflow, and a real product bug. |
| 109 | - [ ] Add a `make base-check REF=<ref>` target wrapping | 109 | - [x] Add a `make base-check REF=<ref>` target wrapping |
| 110 | `git worktree add --detach` + test + `worktree remove`, so the safe way to | 110 | `git worktree add --detach` + test + `worktree remove`, so the safe way to |
| 111 | inspect a baseline is one command. A note alone demonstrably did not hold. | 111 | inspect a baseline is one command. A note alone demonstrably did not hold. |
| 112 | (Carried forward: still open, and cheaper now that a green baseline makes | 112 | Done in `ea15d81`; missing and invalid refs fail without moving `HEAD`, test |
| 113 | "is this mine?" answerable without one.) | 113 | failures fail the target, and successful checks clean their worktree. |
| 114 | - [x] Add the boundary-member checklist (empty, single, first/root, | 114 | - [x] Add the boundary-member checklist (empty, single, first/root, |
| 115 | duplicate/already-seen, freed-then-reused) to the plan-writing routine, and | 115 | duplicate/already-seen, freed-then-reused) to the plan-writing routine, and |
| 116 | require plans to state invariants beside steps. Evidence: the "Plans state | 116 | require plans to state invariants beside steps. Evidence: the "Plans state |
| @@ -206,12 +206,12 @@ subagents were unavailable. | |||
| 206 | 206 | ||
| 207 | ### Actions for the next slice | 207 | ### Actions for the next slice |
| 208 | 208 | ||
| 209 | - [ ] Add the `make base-check REF=<ref>` target (carried forward from the | 209 | - [x] Add the `make base-check REF=<ref>` target. Done in `ea15d81`; both its |
| 210 | lease slice, still open, and now cheap). | 210 | passing and failing cleanup paths were exercised without moving `HEAD`. |
| 211 | - [ ] Decide whether to spend the control-socket safety margin. See the debt | 211 | - [x] Decide whether to spend the control-socket safety margin. Keep |
| 212 | below: the fix is correct but costs sharing in a narrow band, and recovering | 212 | `SOCKET_PATH_LIMIT` at 100: sharing is an optimisation, the fallback is |
| 213 | it means raising `SOCKET_PATH_LIMIT` from 100 toward the real 104/108. That | 213 | correct, and no observed sync problem justifies spending the portable |
| 214 | is someone else's deliberate margin, so it wants a decision, not a patch. | 214 | margin. Revisit only under the retained debt's slow-sync trigger. |
| 215 | 215 | ||
| 216 | ### Retained debt | 216 | ### Retained debt |
| 217 | 217 | ||
| @@ -291,11 +291,16 @@ The worker contract is in `~/code/rad/workshop/durin/worker.sh`. | |||
| 291 | 291 | ||
| 292 | ### Actions for the next slice | 292 | ### Actions for the next slice |
| 293 | 293 | ||
| 294 | - [ ] Add the `make base-check REF=<ref>` target (carried forward twice now). | 294 | - [x] Add the `make base-check REF=<ref>` target. Done in `ea15d81`. |
| 295 | - [ ] Decide whether to spend the control-socket safety margin (carried | 295 | - [x] Decide whether to spend the control-socket safety margin. Keep 100 until |
| 296 | forward: `SOCKET_PATH_LIMIT` 100 -> 104). | 296 | the retained slow-sync trigger fires; correctness beats recovering a silent |
| 297 | - [ ] Audit the remaining lease and issue commands for full-id-only test | 297 | optimisation without evidence. |
| 298 | coverage, now that one such gap has cost a real bug. | 298 | - [x] Audit the remaining lease and issue commands for full-id-only test |
| 299 | coverage. `claim`, `renew`, and `unclaim` now exercise the abbreviated form | ||
| 300 | in `lease_cli_test` (`4d802a0`), and the release case fails against the | ||
| 301 | unfixed `4a60f08` as expected. Every local issue id-taking operation resolves | ||
| 302 | through `state::resolve_issue_ref`; its unique and ambiguous prefix rules are | ||
| 303 | covered in `abbrev_test`. | ||
| 299 | 304 | ||
| 300 | ### Retained debt | 305 | ### Retained debt |
| 301 | 306 | ||
| @@ -310,3 +315,48 @@ The worker contract is in `~/code/rad/workshop/durin/worker.sh`. | |||
| 310 | - **Forge owner, next merge-scan change:** the worker writes a `Fixes: <id>` | 315 | - **Forge owner, next merge-scan change:** the worker writes a `Fixes: <id>` |
| 311 | trailer, which nothing consumes. `merge_scan` reads `Patch:` only, so the | 316 | trailer, which nothing consumes. `merge_scan` reads `Patch:` only, so the |
| 312 | issue was not closed by the push that fixed it. | 317 | issue was not closed by the push that fixed it. |
| 318 | |||
| 319 | ## Closing the lease branch — 2026-09-06 | ||
| 320 | |||
| 321 | Closed the process actions that had accumulated behind the lease slice before | ||
| 322 | making the forge a dependency of an unattended foreman. | ||
| 323 | |||
| 324 | ### What was delivered | ||
| 325 | |||
| 326 | - `make base-check REF=<ref>` tests another revision in a detached temporary | ||
| 327 | worktree and removes it on success or failure without moving the caller's | ||
| 328 | `HEAD` or index. | ||
| 329 | - The CLI lease suite now sends abbreviated issue ids through `claim`, `renew`, | ||
| 330 | and `unclaim`, matching the ids humans and scripts get back from list output. | ||
| 331 | - Ambiguous lease-prefix coverage creates 17 issues, so the pigeonhole | ||
| 332 | guarantee replaces a random early return that could skip the assertion. | ||
| 333 | - The SSH socket limit stays at 100 bytes. The 100-to-104 band is retained debt, | ||
| 334 | not an unmeasured reason to spend a portability margin. | ||
| 335 | |||
| 336 | ### What worked | ||
| 337 | |||
| 338 | - Running the unclaim test against pre-fix `4a60f08` reproduced the real bug: | ||
| 339 | the command reported success but a second principal was still refused. | ||
| 340 | - The new baseline target's failure path found the nine trust failures on the | ||
| 341 | old `main`, then removed its linked worktree and left the lease branch | ||
| 342 | untouched. Its success path ran all tests at `HEAD` and did the same cleanup. | ||
| 343 | - Auditing the issue mutation functions found one resolver | ||
| 344 | (`state::resolve_issue_ref`) rather than a collection of command-specific | ||
| 345 | prefix rules, so representative boundary tests cover the shared rule. | ||
| 346 | |||
| 347 | ### Lessons | ||
| 348 | |||
| 349 | - A probabilistic collision test that returns early is not coverage. When the | ||
| 350 | namespace has 16 members, creating 17 inputs makes the boundary deterministic. | ||
| 351 | - A green baseline on an unlanded branch does not make the base branch green. | ||
| 352 | Landing the fixes is part of closing the slice, not release administration to | ||
| 353 | defer indefinitely. | ||
| 354 | |||
| 355 | ### Review and retained debt | ||
| 356 | |||
| 357 | Review was a deliberately adversarial self-review, not independent: subagents | ||
| 358 | were unavailable. It found and removed the probabilistic test escape. No new | ||
| 359 | duplicate predicate or temporary state remains. Existing trigger-bound debt | ||
| 360 | above is unchanged: fencing waits for server-mediated revisions, silent SSH | ||
| 361 | sharing waits for a measured slow sync, and raw ungoverned holder ids wait for | ||
| 362 | the next issue-page change. | ||