aboutsummaryrefslogtreecommitdiff
path: root/.ai/session-context.org
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-07-24 10:56:50 -0500
committerCraig Jennings <c@cjennings.net>2026-07-24 10:56:50 -0500
commit267d1de7b8a8e7fd22b156433567c88216ec3d0f (patch)
tree4289b12c333ea3a0c0c027c4de2c38224a5aced5 /.ai/session-context.org
parentb34fb4e413eea994f2492669d64d2e5ee9aa8110 (diff)
downloadrulesets-267d1de7b8a8e7fd22b156433567c88216ec3d0f.tar.gz
rulesets-267d1de7b8a8e7fd22b156433567c88216ec3d0f.zip
fix: harden the org-file mutators and close four verified bugs
An overnight hygiene run over this repo, reviewed adversarially afterward. Every problem below was reproduced before it was fixed, and every fix was re-checked by a skeptic who was told to refute it. cj-remove-block could silently destroy content. Its range check validated only the first and last lines, so a span from one cj block's opener to a later block's closer passed and the removal deleted everything between — prose, headings, whole tasks, with a zero exit. That is the failure the check exists to prevent, and drift is its normal case, since the calling skill edits the file while processing. It now refuses a range holding more than one block, backs up first, and writes atomically while preserving the file's mode and following symlinks to the real target. todo-cleanup rewrote todo.org with no backup, alone among the three tools that mutate these files. It now copies to the temp dir like lint-org does. Both backup helpers suffix rather than overwrite, because a second-resolution stamp collapsed two back-to-back invocations into one backup holding already-mutated content — and running todo-cleanup twice in a row is the shipped path. audit.bats failed about one run in eight, in teardown rather than in its assertions, so a passing test reported as a false red. git commit spawns a detached maintenance process that is still writing packs when the commit returns, racing the cleanup. The fixture disables it. route_recommend downgraded a correct strong match to weak when two projects shared a basename. lint.sh now sweeps the scripts that get symlinked onto PATH, which were the only shell in the repo with no gate over them. Two test defects found in review and fixed here: a suite that deleted real backups from the shared temp dir, and one that depended on that directory's contents. Isolation now lives in an autouse fixture rather than in each test's memory. Known and filed, not closed: the range check still cannot prove the range is the block that was scanned, so drift by exactly one whole block deletes the wrong annotation. That needs a content assertion and a CLI contract change.
Diffstat (limited to '.ai/session-context.org')
-rw-r--r--.ai/session-context.org153
1 files changed, 152 insertions, 1 deletions
diff --git a/.ai/session-context.org b/.ai/session-context.org
index e3f539c..d095f80 100644
--- a/.ai/session-context.org
+++ b/.ai/session-context.org
@@ -6,7 +6,7 @@
** Active Goal
-Sentry armed for the night on =sentry/2026-07-23-ratio=, hourly. Morning teardown is Craig's.
+Sentry re-armed for the night 2026-07-24 on =sentry/2026-07-24-ratio=, hourly, with the implement pass ON (=:SENTRY_MAY_IMPLEMENT: yes=). This run implements solo, decision-free tasks and arriving solo work on the branch, never pushing; shared-asset/convention proposals still park. Bug+refactor finding files graded tasks. Every review runs the premise check first (reproduce before judging). Morning teardown + merge is Craig's. Earlier-tonight run (2026-07-24 00:xx) shipped the sentry workflow addition itself (pass 11 refactor finding, pass 12 opt-in implement, the =:SENTRY_MAY_IMPLEMENT:= marker) to main, plus the speedrun (4 solo tasks) and eight parked VERIFYs.
** Decisions
@@ -173,6 +173,157 @@ Filed [#A], SCHEDULED today — the only [#A] of the night. The fix is find-not-
Two fires' worth of evidence now says bundle-to-bundle propagation is where this repo leaks changes.
+** 00:51 — Fire 1 of the 2026-07-24 run (working) — first fire with pass 12 live
+
+Lock acquired, branch =sentry/2026-07-24-ratio= verified, tree clean outside the spine. Digest:
+
+- P1 roam pull — ran, already up to date.
+- P2 inbox zero — ran, no-op. Inbox at zero (the question-capture proposal was parked during entry).
+- P3 triage intake — skipped: no active source surviving the mail/messenger exclusion.
+- P4 todo cleanup — ran with real work. =--archive-done= moved three completed speedrun tasks out of Open Work into Resolved; =--convert-subtasks= normalized the tree. Committed.
+- P5 task audit — mechanical subset. Staleness 13, back to the pre-speedrun baseline now that the four solo tasks closed.
+- P6 working-files hygiene — ran. All =working/= dirs still have open backing VERIFY tasks (the parked proposals). No orphans.
+- P7 spec status board — ran. Two READY specs, nothing stuck.
+- P8 link integrity — ran. The docs-lifecycle spec still shows its two known prose false positives (=file:→id:= and =keep-file:-links-through-pilot=, bare =file:= tokens org parses as bracketless links). Correctly unaffected by tonight's spec-scoping, since =link-to-local-file= is org-lint's own checker, not a todo-format one. No new findings.
+- P9 git health — main level with origin, sentry branch correctly has no upstream, no stale branches.
+- P10 prep freshness — skipped: no =daily-prep/=.
+- P11 bug and refactor finding — ran, first fire under the widened pass. Rotating area: the cross-project routing scripts (=route_recommend.py=, =broadcast.py=), untouched by the previous six areas. One verified latent bug filed [#D] (below). Refactor note not worth a task: =recommend='s two weak-tier branches collapse to a single =_tiebreak= call, since =_tiebreak= on a one-element list returns that element. Two lines, no behavior change, so it's a digest line rather than backlog noise.
+- P12 solo-task implementation — *active this run* (=:SENTRY_MAY_IMPLEMENT: yes=) but a correct no-op: zero eligible tasks. All four open =:solo:= TODOs were completed in tonight's speedrun, so the ready bucket is empty. Nothing to implement, nothing deferred.
+
+*** The finding
+
+=route_recommend='s =discover_destination_names= collapses projects to bare basenames, so two projects sharing a basename across roots would both literal-match and read as an ambiguous tie, downgrading a correct strong match to weak. Reproduced by direct probe. Latent rather than live: 27 projects, 27 distinct basenames today. The destination stays right, only the tier is wrong, so the cost is an extra routing prompt. Filed [#D] with the order-preserving dedupe as the fix.
+
+** 01:33 — Fire 2 (working) — pass 12's first real implementations
+
+Digest:
+
+- P1 roam pull — ran, already up to date.
+- P2 inbox zero — ran. Project inbox at zero. The roam inbox dropped 9 → 5 (another session filed some); all 5 remaining belong to archsetup or work, none rulesets-claimed, so roam mode is correctly a no-op here.
+- P3 triage intake — skipped: no active source.
+- P4 todo cleanup — ran, all three checks clean (fire 1 did the archiving).
+- P5 task audit — mechanical subset. Staleness 13, flat.
+- P6 working-files hygiene — ran. All =working/= dirs still backed by open VERIFYs.
+- P7 spec status board — ran. Two READY specs, nothing stuck.
+- P8 link integrity — ran. Only the docs-lifecycle spec's two known prose false positives.
+- P9 git health — main level with origin, sentry branch correctly upstream-less.
+- P10 prep freshness — skipped: no =daily-prep/=.
+- P11 bug and refactor finding — rotating area: the cj-comment tooling (=cj-scan.py=, =cj-remove-block.py=). Two findings filed, one of them serious.
+- P12 solo-task implementation — *two tasks implemented and committed to the branch* (17f5d48, 1b0f284). Both premise-checked before a line was written.
+
+*** The serious find: cj-remove-block destroys content
+
+=looks_like_cj_range= validated only the first and last lines of a range. A span from one cj block's opener to a *later* block's closer passed, and the removal then deleted everything between — prose, headings, whole tasks — silently, exit 0. That is exactly the failure the check exists to prevent, and drift is its normal case, since =respond-to-cj-comments= edits the file while processing and a file under cj review usually holds several blocks. Reproduced on a two-block fixture that lost a heading and two content lines. Filed [#B], then implemented in pass 12 after an independent re-verification on a different fixture shape.
+
+Same file, second defect fixed alongside: =remove_range= rewrote the org file with a bare =write_text= (truncates on open) and took no backup, so a mid-write failure would leave =todo.org= truncated with nothing to recover from. =lint-org.el= already backs these files up to =/tmp= before mutating; cj-remove-block now matches that and writes atomically.
+
+*** The measurement lesson, third time tonight
+
+A full-suite run went red on =audit.bats= test 4. I stashed my changes, saw it pass clean, and had a one-sample A/B pointing straight at my own diff. Re-ran three times with the changes restored and it passed every time. The failure is an intermittent teardown flake (=rm -rf= racing something still writing into a fixture =.git/objects=), not my change, and I nearly filed the wrong cause off a single sample. Filed [#C] with the git-background-gc theory explicitly labelled a lead rather than a verified cause.
+
+Committed only on a genuinely green re-run, not on the red with a hand-wave.
+
+** 02:33 — Fire 3 (working) — the flake's cause traced, and my own lead disproved
+
+Digest:
+
+- P1 roam pull — ran, already up to date.
+- P2 inbox zero — ran, no-op. Project inbox at zero; roam holds 5, none rulesets-claimed.
+- P3 triage intake — skipped: no active source.
+- P4 todo cleanup — ran. =--archive-done= moved fire 2's two completed tasks to Resolved. Committed.
+- P5 task audit — mechanical subset. Staleness 13, flat.
+- P6 working-files hygiene — ran, all dirs backed.
+- P7 spec status board — ran, two READY, nothing stuck.
+- P8 link integrity — ran, only the known docs-lifecycle prose false positives.
+- P9 git health — main level with origin, branch upstream-less, nothing stale.
+- P10 prep freshness — skipped.
+- P11 bug and refactor finding — no new findings. The fire's whole investigative budget went to confirming fire 2's filed flake, which is the honest place for it; a hunt that finds nothing new is a result.
+- P12 solo-task implementation — one task implemented and committed (7f45d4b).
+
+*** Confirming the cause before fixing, and disproving my own lead
+
+Fire 2 filed the =audit.bats= flaky teardown with a stated theory: =gc.auto='s loose-object threshold. Pass 12's premise check went after that theory rather than the fix, and killed it — a fixture holds five objects against a default threshold of 6700, so that mechanism cannot fire.
+
+The real cause came from a =GIT_TRACE= run: =git commit= spawns =git maintenance run --auto --quiet --detach= on git 2.55. The commit returns while the detached process is still writing a pack, and teardown's =rm -rf= races it. Every failed run had left a =tmp_pack_*= behind, which is the thread that led there.
+
+Fix: =maintenance.auto false= plus =gc.auto 0= in the fixture, killing the background writer rather than retrying the delete (a retry loop hides a live process instead of removing it). Validated over 20 consecutive clean runs against a ~1-in-8 baseline, and recorded honestly in the task that 20 clean runs alone would be ~7% likely by luck — the trace is the evidence, the runs confirm.
+
+This is the second night running where the premise check changed the outcome. Fire 2 it stopped a wrong causation call; here it stopped me implementing a fix for a mechanism that was never operating. Both times the cost of checking was minutes and the cost of not checking would have been a plausible, wrong, committed change.
+
+** 03:33 — Fire 4 (working) — two hypotheses killed, one real gap closed
+
+Digest:
+
+- P1 roam pull — ran, up to date. P2 inbox zero — no-op, both surfaces clean. P3 triage — skipped, no active source.
+- P4 todo cleanup — ran. Archived fire 3's completed task to Resolved. Committed.
+- P5 task audit — mechanical subset. Staleness 13, flat all night.
+- P6 working-files — all dirs backed. P7 spec board — two READY, nothing stuck. P8 link integrity — only the known prose false positives. P9 git health — clean. P10 — skipped, no prep dir.
+- P11 bug and refactor finding — rotating area: the org-file mutators, chosen because =cj-remove-block= yielded a serious find in that same class last fire. One gap filed.
+- P12 solo-task implementation — one task implemented and committed (0686784).
+
+*** What the area review actually found, and what it disproved
+
+The lead was that =todo-cleanup.el= might lose data. It rewrites =todo.org=, creates archive files, and moves subtrees *between* files, which is the shape that bit =cj-remove-block=. Two specific hypotheses, both tested, both wrong:
+
+- *"A mid-move failure loses a subtree from both files."* No. The order is delete-from-buffer, write-archive, save-todo.org-last, so an archive-write failure aborts before the save. Verified by making the archive directory unwritable: exit 255, =todo.org= byte-identical, content intact.
+- *"Errors are swallowed on the mutation path."* No. The only =ignore-errors= in the file wrap =call-process "git"=, never a write.
+
+What survived was narrower and real: todo-cleanup mutates with *no backup*, while both sibling mutators (=lint-org.el=, =wrap-org-table.el=) copy to =/tmp= first, and =cj-remove-block= joined them last fire. It is also the one that runs most often. Confirmed empirically that Emacs's own backup does not fire under =--batch -q=, so there was genuinely no undo short of git. Filed [#C], then implemented in P12.
+
+Also checked my own change for regression rather than assuming: a missing input file exits 255 and creates nothing, identical to the pre-change version tested from git.
+
+*** Running tally on the premise habit
+
+Three fires, three times it changed the outcome. Fire 2 it stopped a wrong causation call. Fire 3 it disproved my own filed =gc.auto= theory before I could implement against it. Here it killed two data-loss hypotheses before they became tasks, leaving only the gap that was actually there. The pattern is consistent: the cheap check keeps a plausible story from becoming a committed change.
+
+** 04:33 — Fire 5 (working) — the first real defer, and last fire's fix proving itself
+
+Digest:
+
+- P1 roam pull — ran, up to date. P2 inbox zero — no-op, both surfaces clean. P3 triage — skipped, no active source.
+- P4 todo cleanup — ran, archived fire 4's completed task. *Confirmed last fire's backup fix working live*: the real =--archive-done= run left =/tmp/todo.org.before-todo-cleanup.20260724-043331=. Dogfooded within an hour of shipping.
+- P5 task audit — staleness 13, flat all night. P6 working-files — all backed. P7 spec board — two READY. P8 link integrity — only the known prose false positives. P9 git health — clean. P10 — skipped.
+- P11 bug and refactor finding — rotating area: the attachment/email handlers, chosen because they parse genuinely untrusted input, unlike every internal-tooling area covered so far. One finding filed.
+- P12 solo-task implementation — *deferred*, and correctly. First defer of the run.
+
+*** The find: attachment filenames are partly sanitized, in two different ways
+
+Both writers derive on-disk names from the =filename= an email declares, and both sanitize incompletely, covering *different* gaps. =eml-view= cleans the name but interpolates the extension raw. =gmail-fetch='s =safe_filename= handles path separators and leading =..= and nothing else. Probed both with the same adversarial set: a =; rm -rf ~= extension survives in both, a literal newline survives in both, a 300-character extension produces a 314-character filename in both.
+
+Two things I checked so this doesn't get over-graded later. Not RCE — files are written through Python =open=, never a shell. Not path traversal — =splitext= only returns an extension when the last dot follows the last separator, so =ext= can never hold a slash, and the traversal case is neutralized in both scripts. The genuine harms are narrower: a newline in a filename breaks downstream tooling that reads the directory as a line-delimited list, and an unbounded extension blows the 255-byte limit so a crafted attachment aborts extraction. Graded [#C] on that honest read rather than the scarier one.
+
+I also corrected my own framing mid-investigation. I first read this as the familiar "one sibling hardened, the other not" pattern from the last two fires. It isn't — =safe_filename= is narrower than it looks, and the two scripts are *differently* incomplete. Neither handles newlines or length.
+
+*** Why pass 12 deferred instead of implementing
+
+The fix itself is clear, but where the shared sanitizer lives is a design call: a shared helper module (clean, but a new synced template file plus =importlib= gymnastics for kebab-named scripts), duplicate it in both (self-contained, but drift — the exact defect class that produced three separate findings tonight), or patch each in place (smallest diff, permanent divergence). That is deliberation, not a quick factual question, so checklist item 4 fires and the unattended loop defers. Filed the VERIFY with the three options and my lean, and left the task *un-=:solo:=-tagged* so a later run doesn't pick it up and guess.
+
+This is the checklist discriminating rather than rubber-stamping. Four fires implemented; this one correctly didn't.
+
+sentry at 05:33: nothing (bug hunt swept =scripts/*.py=; two candidate gaps both disproved — =workflow-integrity.py= *is* gated, its bats runs the real checker against the real canonical tree under =make test=, and =update-skills.py= is an on-demand maintenance command rather than a gate. Recorded so neither gets re-investigated.)
+
+sentry at 06:33: nothing (bug hunt swept =wrap-org-table.el=, the third org-file mutator and the one with prior history — its load-time dispatch caused the 2026-07-09 corruption. Clean on every probe: the entry-script guard correctly refuses to dispatch when lint-org merely =require='s it, it backs up before writing like its siblings, and it is block-aware — a table inside =#+begin_example= stayed verbatim while a real over-budget table wrapped onto continuation rows with rules. Second consecutive quiet hunt.)
+
+** 07:33 — Fire 8 (working) — a time bomb I planted four fires ago went off
+
+Digest: P1-P10 all no-op (roam current, both inboxes clean, todo cleanup clean, staleness 13, working dirs backed, two READY specs, only the known prose link false positives, git clean). P11 found a real coverage gap. P12 implemented it, and the suite caught a regression of my own making.
+
+*** The find: the only ungated shell in the repo
+
+=scripts/lint.sh= sweeps =scripts/*.sh=, the language hooks, and the language githooks. It never touched =claude-templates/bin/= — zero references. Those four scripts (=ai=, =agent-text=, =agent-page=, =install-ai=) are the ones =make install= symlinks onto PATH, which makes them the *most* exposed shell in the repo and left them the only shell with no gate over it. All four are clean today, so the guard is a no-op by design; it exists so a future regression can't pass silently. The =ai= launcher was hardened to 42 tests recently and nothing enforced that going forward.
+
+The test pins *coverage*, not cleanliness: it plants a broken file in each swept location and asserts lint complains, so a location that stops being swept fails the suite rather than passing quietly.
+
+Surfaced a bigger question I did *not* answer overnight: rulesets ships shellcheck enforcement to consuming projects (the bash bundle's pre-commit, =validate-bash.sh=) and runs none on itself. Filed as a VERIFY, because turning it on would surface the false positives dispositioned earlier this session and choosing between fixing them or adding disable-directives is a preference, not a fact.
+
+*** The regression: my own test, detonating on schedule
+
+The full suite went red on the two backup tests I added in fire 4. Not a flake — a time bomb. They globbed =/tmp/todo.org.before-todo-cleanup.*=, but the backup name derives from the file's *basename*, and the real =todo.org= shares it. So a live sentry run's genuine backup was indistinguishable from the test's own artifact, and the check-mode test (which asserts *no* backup exists) failed the moment fire 5's real archive pass created one.
+
+They passed when written only because no real backup existed yet. Three now sit in =/tmp=. Both tests rebind =temporary-file-directory= to a private dir, and I verified they pass with the real backups present rather than by clearing them.
+
+Worth naming plainly: I shipped a test whose correctness depended on the state of a shared directory that the code under test writes to in production. That is the "no shared mutable state" rule in =testing.md=, and I broke it while fixing a different durability bug. The suite caught it two fires later, which is the argument for running the full suite every fire rather than only the touched file.
+
* Sentry approval queue (2026-07-23)
Five items. Each names what, why, and the exact edit. Items 2 through 5 are also filed as =VERIFY [#B]= tasks in =todo.org= with prepared diffs under =working/=, so they survive this anchor being archived — say "approve the parked <topic>" for any of them.