From 01356fa036847f9d216dd92a006049105c2d5461 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Thu, 23 Jul 2026 08:25:40 -0500 Subject: chore: file eight bug findings and stage four shared-asset proposals Overnight hygiene sweep of this repo. Everything here is a finding or a staged proposal. No rule, workflow, script, or bundle file was modified. The one that matters: the python and typescript language bundles ship no pre-commit hook, so a project installing either gets no credential scan on commit. Both bundles predate the hook rollout that swept go and bash, and install-lang skips missing components without a word, so it went unnoticed for two months. Live on two projects, one of them work. Filed [#A]. Seven more findings landed. The largest is a non-atomic write in the cross-project inbox tool, which can strand an empty handoff in another project's inbox and block a turn there. The rest are lint noise and two gaps in the Signal channel. An eighth was filed and then retracted the same night, once a retest showed I had compared two different files and read the difference as a bug. Four proposals from other projects are staged under working/ with verified diffs, each behind a VERIFY task, waiting on a decision. --- todo.org | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) (limited to 'todo.org') diff --git a/todo.org b/todo.org index 507f08d..2b299c9 100644 --- a/todo.org +++ b/todo.org @@ -39,6 +39,102 @@ Tags are assigned and refreshed by =task-audit=; =task-review= keeps them honest * Rulesets Open Work +** TODO [#A] Python and TypeScript bundles ship no secret-scan hook :bug: +SCHEDULED: <2026-07-23 Thu> +The =python= and =typescript= language bundles carry rules, a coverage script, and a gitignore fragment. They carry no =githooks/pre-commit=, no =claude/hooks/=, no =claude/settings.json=, and no =CLAUDE.md=. The =bash=, =elisp=, and =go= bundles carry all four. + +The pre-commit hook is the secret scanner. So a project installing the Python or TypeScript bundle gets no credential scan on commit, no validate-on-edit hook, and no settings — while README's "Bundle structure" section documents all four as what each bundle follows, and notes.org describes the bundles as "rules + hooks + settings". + +Live as of 2026-07-23, verified by scanning every project carrying =.claude/rules/=: =work= (python) and =clock-panel= (python + typescript) both have no =githooks/= and no =.claude/settings.json=. The four elisp projects all have both. =work= is the one that matters — a work repo is where a leaked credential is most costly and most likely to reach a company remote. + +Not a design choice. Both bundles were added 2026-05-31; =go='s githooks landed 2026-06-02 and =bash='s 2026-06-23, so the hook rollout swept the two bundles added *after* these and skipped these. =install-lang.sh= guards its copy with =[ -d "$SRC/githooks" ]=, so the install succeeds silently and reports nothing missing — which is why this stayed invisible for nearly two months. + +Grading: Major severity (a documented security control absent, silently, with the install reporting success) x every user, every time (every install of either bundle, standing on 2 of the 6 bundle-using projects) = P1 = [#A]. + +Fix direction: port =githooks/pre-commit= to both bundles, adapting the language-specific half (the secret scan is common; =gofmt=/=shellcheck= becomes a formatter/linter check per language), add =claude/hooks/validate-*.sh= and =claude/settings.json=, and seed =CLAUDE.md=. Then make the gap loud: =install-lang= should warn when a bundle lacks a component the README documents, so the next partial bundle announces itself instead of installing quietly. Re-run =make install-lang= on =work= and =clock-panel= afterward. + +** TODO [#C] Two language bundles' pre-commit hooks lost the cd guard :bug:quick:solo: +=languages/bash/githooks/pre-commit= line 8 reads =cd "$REPO_ROOT" || exit 1=. The =go= and =elisp= copies of the same line read a bare =cd "$REPO_ROOT"=. Three siblings of one file, one hardened and two not — the guard landed in bash and never propagated. + +Low impact, stated honestly: git chdirs to the working-tree root before running a hook, so if =cd= fails the cwd is already correct and the checks still run against the right tree. Neither script sets =-e=, so a failure wouldn't abort them either. It takes a =GIT_WORK_TREE= or bare-repo edge case for =rev-parse --show-toplevel= to disagree with the cwd at all. + +Grading: Minor severity (belt-and-braces guard, no silent-pass path found — the checks run regardless) x rare edge case (needs a git configuration that makes toplevel differ from the hook's cwd) = P4... graded [#C] rather than [#D] because it's a two-character fix in a security-relevant gate and the drift pattern is the real signal: a fix landed in one bundle copy and stopped there, which is the same shape as the [#A] above. + +** VERIFY [#B] Parked: zero markup in chat output, fences included (from org-drill) +What arrived: interaction.md's no-reverse-video rule carves out fenced code blocks as acceptable "when the user explicitly wants a block to copy"; org-drill relays your 2026-05-30 direction that chat output carries no markup at all, fences included. +Recommendation: accept as-is. The carve-out contradicts the direction, and the rule is where it belongs so every project inherits it instead of it living as one project's memory. I kept the rule's factual claim honest — fences genuinely don't invert the way inline spans do, so the new text says they read as markup you didn't ask for rather than inventing a rendering problem. Checked: this is the only copy of the carve-out. +Prepared diff: [[file:working/interaction-no-markup/proposed.diff]] — apply is mechanical on your go. +Say "approve the parked no-markup rule" (or adjust / reject) and it gets applied. + +** VERIFY [#B] Parked: sentry Living Document updates from two dogfood runs (from takuzu + archangel) +What arrived: both projects ran sentry live overnight 2026-07-20/21 and reported back — takuzu 8 fires, archangel 7. No engine defects in either. +Recommendation: accept with changes. Four of the five findings fold in cleanly: the archive-done .gitignore note, the mirror-only "git log understates the night" note, property sweeps as quiet-fire work, and splitting the task audit into a mechanical hourly subset plus a nightly judgment half. The fifth (make the bug hunt an official pass) already landed tonight as pass 11, so it reduces to recording their corroboration. Takuzu's lint-org finding I handled separately as a filed bug task rather than a workflow edit, since it's a defect not a design change. +Prepared diff: [[file:working/sentry-living-document/proposed.diff]] — apply is mechanical on your go. +Say "approve the parked sentry updates" (or adjust / reject) and it gets applied. + +** VERIFY [#B] Parked: clear four lint flags in the notes.org template (from smoke) +What arrived: the synced notes.org template trips misplaced-heading twice on column-0 bold lines and invalid-block twice around the Pending Decisions example, so every project inherits the noise on every sweep. +Recommendation: accept as-is, and I found two more instances smoke didn't list plus the actual cause of the block flags (the example contains a literal =** Feature Name= line that org reads as a heading, breaking block tracking). Verified: the proposed file lints with zero mechanical and zero heading/block findings. Worth your attention separately — those two flags are mechanical, not judgment, so =lint-org --fix= run in any project would rewrite the template and drift it from canonical. +Prepared diff: [[file:working/notes-template-lint/proposed.diff]] — apply is mechanical on your go. +Say "approve the parked template fix" (or adjust / reject) and it gets applied. + +** VERIFY [#B] Parked: clear temp/ during wrap-up teardown (from your roam capture) +What arrived: your roam inbox item asked that every project carry working/ and temp/, and that temp/ be "deleted as a part of the wrap up sequence". The rest shipped 2026-07-20; wrap-it-up.org never got the temp/ clause and has no mention of the directory. +Recommendation: accept as-is. Added as a Step 3 sub-step after the archive pass, with two guards — confirm before deleting anything that reads as in-progress rather than throwaway (that belongs in working/, so move it), and skip in a project where temp/ isn't gitignored. It parks rather than lands because it's both a shared-asset edit and a destructive one. +Prepared diff: [[file:working/wrapup-temp-cleanup/proposed.diff]] — apply is mechanical on your go. +Say "approve the parked temp cleanup" (or adjust / reject) and it gets applied. + +** TODO [#B] inbox-send leaves a phantom empty handoff in the target inbox :bug:solo: +=inbox-send.py= writes straight to the destination path in the target project's =inbox/=. =Path.write_text= opens with mode =w=, which creates and truncates before any content is written, so *any* failure between opening and finishing leaves a zero-byte =.org= file sitting in another project's inbox. + +That file is not inert. =inbox-status= counts it as a pending handoff, so it trips the receiving project's =inbox-boundary-check= Stop hook and blocks a turn there. The receiving agent then has to resolve a handoff with no content and no sender context, which it cannot do from the file. Meanwhile the sender saw an error and will most likely retry, so the target gets a second file too. + +Reproduced 2026-07-23 end to end: a send whose text contains non-ASCII under =LC_ALL=C PYTHONUTF8=0 PYTHONCOERCECLOCALE=0= fails on the ASCII codec, leaves a zero-byte file in the destination inbox, and =inbox-status= in that project then reports it as pending. + +The encoding case is one trigger, not the defect. =write_text= and =read_text= are both called with no =encoding= argument, so they follow the locale; passing =encoding="utf-8"= closes that trigger. The defect underneath is the non-atomic write, which a full disk, a revoked permission, or an interrupted process reaches just as easily. + +Grading: Major severity (a phantom handoff in a *different* project's inbox, unresolvable from its own content, that blocks a turn there via the boundary hook) x some users, sometimes (any mid-write failure, of which the locale case is only the one reproduced) = P2 = [#B]. + +Fix direction: write to a temp file in the destination directory and =os.replace= into place, so the inbox only ever sees a complete file. Pin =encoding="utf-8"= on both =write_text= and the =read_text= in =resolve_roots=. Same treatment for =send_file=, whose =shutil.copy2= has the same shape. Test by forcing a mid-write failure and asserting the destination directory is unchanged. + +** TODO [#C] Two Signal-channel gaps: unbounded send, second account never received :bug: +Both found 2026-07-23 reading =claude-templates/bin/agent-text= and =scripts/signal-receive.sh=. Shellcheck is clean on all four =claude-templates/bin/= scripts; these are logic gaps, not style. + +1. *The direct send has no timeout.* =agent-text='s relay path passes =-o ConnectTimeout=10= to ssh, but the local-account path calls =signal-cli send= with no bound at all. A stalled network or a lock contended by the receive timer hangs the call indefinitely. That matters more than it looks: agent-text is invoked *by agents*, so an unbounded call blocks the calling turn with no output, and the caller can't distinguish a hang from a slow send. The receive path already takes ~16s of wall clock every cadence, so the two contending on the same account is not hypothetical. Fix: wrap the direct send in =timeout=, matching the relay's bound, and let the existing non-zero path print the desktop-fallback message. + +2. *ratio's second Signal account is never received.* =signal-cli listAccounts= on ratio warns "Messages have been last received 8 days ago". Established by elimination, not inference: ratio holds two accounts (=+15045173983= pager, =+15103169357= Craig's personal), =signal-receive.sh= hardcodes the pager as its default and takes no others, and the timer's journal shows it draining the pager cleanly every 15 minutes (last run 2 minutes before the warning appeared). So the 8-day-stale account is the personal one, and nothing on the machine keeps it warm. + + Honest limit on item 2: the staleness is verified, the *harm* is not. That registration is described in the session history as note-to-self with no push, so it may be fine to leave cold, and Signal's own tolerance for a quiet linked account isn't something I established. Filed so the question gets answered rather than rediscovered. The script already accepts an account argument, so if it does matter the fix is a second timer instance rather than a code change — which makes it a dotfiles handoff (that repo owns the unit) with rulesets owning only the script. + +Grading: Minor severity for both (one is a latent hang in a tool with a documented fallback, the other a warning on an account nothing currently depends on) x most users, frequently (the stale warning is a standing condition on this host; the hang needs a stall) = P3 = [#C]. + +** TODO [#C] Two smaller inbox-send defects: uncaught traceback, duplicate roots :bug:quick:solo: +Both verified 2026-07-23, both in =.ai/scripts/inbox-send.py=, both low-harm. Grouped because they're the same file and the same fix session. + +1. *Uncaught =PermissionError=.* =main= catches =(ValueError, FileNotFoundError)= around the send, but =send_file= reaches =shutil.copy2=, which raises =PermissionError= on an unreadable source. That is an =OSError=, not caught, so the user gets a raw Python traceback instead of the clean =inbox-send: = error every other failure path produces. Reproduced with a =chmod 000= source. No partial file is left in this case. Fix: catch =OSError= alongside the other two. + +2. *Duplicate roots list a project twice.* =discover_projects= appends without deduping, so a roots config naming both a parent and one of its children (=/x= and =/x/proj=) lists =proj= at two different numeric indices. Reproduced via =INBOX_SEND_ROOTS=. Harmless today — both indices resolve to the same project, so no message goes to the wrong place, and Craig's current =~/.claude/inbox-roots.txt= has no overlap. Fix: dedupe on =resolve()= before returning. + +Grading: Minor severity (one is cosmetic output noise, the other an ugly but accurate failure message; neither loses or misroutes a message) x rare edge case (an unreadable source file, or a roots config with an overlap) = P4... graded up to [#C] rather than [#D] because both fixes are one line each and sit in a script every project depends on for cross-project messaging. + +** TODO [#C] lint-org todo-format checkers fire on spec files :bug:solo: +=level2-done-without-closed= and =level-2-dated-header= encode =todo.org= completion conventions, but they run against every org file, including =docs/specs/=. A spec's Decisions section legitimately uses =** DONE = with no =CLOSED:= cookie, and its Review-and-iteration-history section legitimately uses =** — — = headings. Neither is a completion defect there. + +Reproduced 2026-07-23 across rulesets' own specs: 100 findings over 7 of the 9 files in =docs/specs/= (docs-lifecycle 27, sentry-workflow 25, autonomous-batch 11, wrapup-routing 11, inbox-consolidation 10, agent-kb 8, encourage-kb 8; the two 2026-07-20 specs are clean). Every project carrying =docs/specs/= inherits the same noise on every sweep. Reported independently by takuzu's first sentry dogfood run. + +Grading: Minor severity (judgment-kind output, so nothing mutates; the cost is noise that trains the reader to skim) x every user, every time (fires on every sweep in every project with specs) = P2... graded down to [#C] because the two inputs disagree: the frequency row is genuinely universal, but Minor severity with zero mutation risk and a known cause reads as P3. Recorded so the read can be argued. + +Fix direction: scope both checkers away from =docs/specs/=, or gate them on the file carrying a =todo.org=-shaped structure. Note (established 2026-07-23) that these are *org-lint's own* checkers, not ones lint-org.el implements, so the fix filters upstream output by file path rather than editing a checker. Takuzu's alternative was to bless =CLOSED:= dates in =spec-create=, which fixes the symptom in new specs and leaves the nine existing ones noisy. + +** TODO [#C] notes.org template trips four lint-org flags in every project :bug:quick:solo: +The synced =claude-templates/.ai/notes.org= carries two boilerplate lines that open with markdown-style bold at column 0 (=**Session history is NOT in this file.**=, =**For protocols and conventions, see:**=). Org parses a line-initial =**= as a level-2 heading, so =misplaced-heading= flags both, and the =#+begin_example= block in the Pending Decisions instructions trips =invalid-block= twice. Every project inherits all four on every sweep. + +Reproduced 2026-07-23. Confirms smoke's proposal. Additional finding from the same run: these two are =mechanical-fixed=, not judgment, so =lint-org --fix= silently rewrites the template's =**bold**= to org =*bold*=. The rewrite is correct org, but it lands on a synced template, so whichever project runs =--fix= first creates drift against canonical. + +Grading: Minor severity (cosmetic noise; the mechanical rewrite is correct org and harmless in isolation) x every user, every time (every project, every sweep) = P2... same disagreement as the checker task above; graded [#C] on the no-real-harm read. + +Fix direction: rephrase both lines in the canonical template so no line starts with =**= (a list dash, or move the bold off column 0), and comma-escape the example block's own markers. One canonical fix clears it everywhere. + ** TODO Manual testing and validation *** Sentry — entry gates fire with Craig present What we're verifying: the interactive entry gates stop for the right states and start the loop only on a clean, green baseline. @@ -1610,3 +1706,12 @@ Origin: the work project's 2026-06-18 fold-in request; preserved bundle [[file:d ** CANCELLED [#D] Fully-unattended scheduled inbox check (/schedule cron pass) :feature: CLOSED: [2026-07-20 Mon] Merged 2026-07-20 into [[file:todo.org::*Unattended /schedule cron contract][Unattended /schedule cron contract — no-session variant]]. Same no-session design problem as the sentry /schedule variant (mutation rights, async surfacing, cross-run dedup, cron auth context); its inbox-specific context was absorbed there. +** CANCELLED [#B] lint-org resolves file: links against cwd, not the linted file :bug:solo: +CLOSED: [2026-07-23 Thu] +Not a defect. I filed this on a bad comparison and retracted it the same night. + +The claimed evidence was that linting =claude-templates/.ai/notes.org= from the repo root reports =protocols.org= and =workflows/first-session.org= missing, while linting it from its own directory reports neither. The first half of that was never run. The findings came from a =/tmp= copy of the template made while preparing the smoke diff, and in =/tmp= those two siblings genuinely are missing, so org-lint was right. I compared two different files and read the difference as a cwd bug. + +Retested three ways: the real file from the repo root gives zero link findings, the real file from its own directory gives zero, and only the =/tmp= copy gives two. A direct probe confirms =find-file-noselect= already sets =default-directory= to the linted file's directory, so the mechanism I proposed could not have been the cause either. + +Worth keeping from the episode: =link-to-local-file= is org-lint's own checker, not one lint-org.el implements, so a real fix here would mean pre- or post-filtering upstream output rather than editing a local checker. -- cgit v1.2.3