feat(rules): shadow-star-shorthand-autocomplete-marker cold-boot rule#3339
Merged
AceHack merged 3 commits intoMay 15, 2026
Merged
Conversation
Per Aaron 2026-05-15T~01:09Z: 'future version of you should likely remember all this.' Promotes the (shadow*) shorthand definition from user-scope memory to .claude/rules/ so it's cold-boot auto-loaded. Otto-CLI committed 4+ '(shadow*)' misinterpretations in this single session before the maintainer named the failure mode directly. Without cold-boot auto-load, future-Otto would repeat the cycle (encounter shorthand → invent interpretation → maintainer corrects → repeat). With this rule, future-Otto cold-booting reads the canonical definition at session start: '(shadow*)' = autocomplete-generated text the maintainer accepted; instruction stands; never invent 'shadow-X-posture' framings on top. Composes with .claude/rules/shadow-check-name-acceptance.md (different shadow-scope: name-acceptance methodology); wake-time-substrate.md (load-bearing substrate needs wake-time landing); razor-discipline.md (operational claim only); glass-halo-bidirectional.md (text-source transparency). Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an always-on Claude Code rule defining the (shadow*) shorthand so future sessions interpret it as an autocomplete-source disclosure marker rather than a “shadow posture” instruction.
Changes:
- Adds a new
.claude/rules/cold-boot rule for(shadow*). - Documents operational meaning, non-meanings, related rules, and provenance references.
AceHack
commented
May 15, 2026
Member
Author
AceHack
left a comment
There was a problem hiding this comment.
Drift detected (Catch 36): Narration-over-action and metadata churn. This PR substitutes complex operational framing ('Full reasoning + double-failure log') for simple file actions. Eliminate metadata churn.
The maintainer's quoted correction used `_..._` (underscore-emphasis) where the file's MD049 setting requires asterisk-emphasis. Single-line fix; verbatim quote text untouched. Verified: `bunx markdownlint-cli2 <file>` exits clean. Co-Authored-By: Claude <noreply@anthropic.com>
Three findings resolved: - Line 19: grammar — "originated authored prose" → "original authored prose" - Line 22: use "the human maintainer" instead of first-name attribution on this instruction surface, per the convention Copilot flagged (first-name attribution reserved for history/memory surfaces) - Line 78-81: reachable-from-cold-boot disclosure for the user-scope memory references. Acknowledges they live at ~/.claude/projects/<slug>/memory/MEMORY.md (auto-loaded each Otto-CLI session) rather than in-repo, and adds a PR backlink as the in-repo provenance anchor. The memory files themselves are out-of-scope for this PR — landing them in-repo is a separate piece of work (likely the Memory Preservation Guarantee from Manifesto V2; see CURRENT-aaron.md). Verified: `bunx markdownlint-cli2 <file>` clean. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
.claude/rules/shadow-star-shorthand-autocomplete-marker.md:78
- This overstates the cold-boot guarantee. The loading taxonomy says user-scope
MEMORY.mdis capped to the first 200 lines / 25KB plus explicit reads (.claude/rules/claude-code-loading-taxonomy.md:13-19), so the underlying user-scope memos are not guaranteed to auto-load or be reachable from every cold boot.
Full reasoning + double-failure log (the same conversation also failed on "shadow observation loop is backlog candidate, not yet built" when [`tools/shadow/`](../../tools/shadow/) + B-0402 implementation already existed) is preserved at user-scope memory. These memos auto-load into every Otto-CLI session via `~/.claude/projects/<slug>/memory/MEMORY.md`, so they are reachable from cold boot even though they do not live in-repo:
.claude/rules/shadow-star-shorthand-autocomplete-marker.md:81
- These bare backticked
.mdbasenames will be harvested bytools/hygiene/audit-rule-cross-refs.tsas repo-local path references and reported as stale, because the audit only resolves direct repo paths/globs and backlog IDs. Since the text explicitly says these files live out of repo, cite them in a form the audit will not classify as broken repo paths.
- `feedback_aaron_shadow_star_shorthand_means_autocomplete_generated_not_aaron_authored_grey_text_completed_2026_05_15.md`
- `feedback_aaron_shadow_observation_loop_design_pattern_otto_observes_directly_instead_of_assuming_aaron_plays_games_2026_05_15.md`
3 tasks
AceHack
added a commit
that referenced
this pull request
May 15, 2026
Five Copilot / Codex review findings resolved:
P1 (Copilot) — broken provenance in README header:
Dropped first-name attribution + user-scope memory-file glob that
doesn't resolve in-repo. Per docs/AGENT-BEST-PRACTICES.md §671-685,
current-state surfaces (this README) use role-refs. Other rule
references now use relative links that resolve from the README's
location.
P1 (Copilot) — sed escaping in install instructions:
The documented `sed` recipe would break on paths containing `&`,
`|`, `\`, or newlines. Replaced with a TypeScript installer
(tools/shadow/launchd/install-launchagent.ts) that:
- uses String.replaceAll (metacharacter-safe)
- sanity-checks for unsubstituted placeholders
- runs plutil -lint before writing
- backs up any existing plist before overwriting
- optionally bootstraps via launchctl
Placeholder syntax changed from `__NAME__` to `{{NAME}}` so the
template header comment can describe them without being substituted
itself.
P1 (Copilot) — rule file referenced from plist not in tree:
The .claude/rules/shadow-star-shorthand-autocomplete-marker.md
reference resolves on main now (#3339 merged). The reference is
preserved in the plist's composes-with comment block.
P2 (Codex) — KeepAlive semantics in plist:
Dropped `SuccessfulExit=false`. KeepAlive dict keys are OR-combined,
so the original combination relaunched on any non-zero exit (incl.
config errors). `Crashed=true` alone gives the documented
crash-only behavior. Comment added explaining the choice.
P2 (Codex) — legacy launchctl load/unload in README:
Replaced with the modern `bootstrap`/`bootout` interface (with
`gui/$(id -u)` domain target) in install, reload, and uninstall
steps. The legacy interface is documented as deprecated and can
silently no-op on failure; `bootstrap`/`bootout` return proper
exit codes.
Verified:
- bunx tsc --noEmit clean
- markdownlint-cli2 clean on README
- plutil -lint OK on both the template and a rendered output
- Installer dry-run produces a valid plist
Co-Authored-By: Claude <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 15, 2026
…ose-out (#3356) * shard(tick): 0230Z — multi-Otto contamination caught + PR #3339/#3349 close-out Co-Authored-By: Claude <noreply@anthropic.com> * fix(shard-0230Z): correct relative-link depth — 4x ../ → 5x and 5x → 6x Both Copilot and Codex flagged that the tick shard's relative links were off by one directory level. From the shard's location (docs/hygiene-history/ticks/2026/05/15/0230Z.md): - `../../../../backlog/...` resolved under docs/hygiene-history/backlog (4 levels up only reaches docs/hygiene-history/, not docs/) - `../../../../../.claude/rules/...` similarly off by one for top-level Correct depths from this shard location: - docs/backlog/...: 5x `../` (verified with `ls -la` from the shard dir) - .claude/rules/...: 6x `../` (verified with `ls -la` from the shard dir) Both links now resolve. Composes with the prior `5x .. → 6x ..` fix on the 0027Z shard (#3330) — same class of error, same family of fix. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 15, 2026
Codex + Copilot reviewed the new install-launchagent.ts in detail.
This commit addresses the substantive correctness + style findings;
unit-test coverage is deferred to a separate backlog row.
Addressed (8):
P1 [Codex P2 + Copilot P1] — XML-escape substituted values:
Paths like `/Users/foo & bar/Zeta` would substitute into <string>
text nodes producing an invalid plist (& without entity terminator).
New `xmlEscape` helper applies the five XML 1.0 predefined entities
(& < > " ') before placeholder substitution.
Restores the advertised "handles paths with shell metacharacters"
promise — the bug class was XML escaping, not shell escaping.
P1 — Unknown args silently ignored:
Switch fell through on typos like `--dryrun` or `--bootstap`, then
installed with default behavior. Added explicit default: case that
errors with the offending flag and exits 1 before any FS action.
Verified: `--dryrun` now produces "Unknown argument: --dryrun.
Run with --help to see supported flags."
P1 — Relative --bun-path / --repo-root overrides:
`--repo-root .` would write relative paths into ProgramArguments,
WorkingDirectory, and log paths — broken under launchd (which has
no cwd unless explicit). New requireAbsolute() helper rejects
with a `Use \`<canonicalized>\` if that's what you meant` hint.
P1 — Non-atomic backup-then-write:
Previous renameSync(destPath, backup) + writeFileSync(destPath)
had a window where the agent could be removed but the new plist
not yet present (if writeFileSync threw). Replaced with the
standard write-temp-then-promote pattern:
1. writeFileSync(destPath.tmp-PID, content)
2. renameSync(destPath, backup) // may not exist; that's OK
3. renameSync(destPath.tmp-PID, destPath) // atomic promote
Failure in step 3 restores backup → destPath (best-effort) so the
caller's existing install is preserved.
P1 — execFileSync can throw before validation:
Missing bun, missing git, or running outside a git checkout
produced a raw exception. New tryDetect() catches and returns
undefined; parseArgs emits the documented actionable error
pointing at the --bun-path / --repo-root override.
P1 — Unconditional main() invocation:
Per Bun-project convention, gate main() with
`if (import.meta.main)` so tests / helpers can import the module
without triggering filesystem or launchctl side effects.
P1 — README/plist cross-refs to .claude/rules/shadow-star-shorthand-*:
These resolve on main (PR #3339 merged). CI's merge-ref check
confirms; the reviewers' findings were against the raw branch
base. No additional action needed.
P2 — Direct name attribution in plist KeepAlive comment:
Replaced "Per Codex review on PR #3342" with the rationale text
only. Current-state surface; uses role-refs only.
Deferred (1):
P1 — Unit test coverage for install-launchagent.ts:
Codex + Copilot called out missing tests for placeholder escaping,
missing placeholders, argument validation, and dry-run output.
This commit adds the substantive behavior the tests would verify;
the tests themselves are deferred to a separate backlog row to
keep this PR's scope manageable. Smoke-tested all four behaviors
by hand on the cli before commit. Filing as follow-up: tests in
`tools/shadow/launchd/install-launchagent.test.ts` using Bun's
built-in test runner.
Verified:
- bunx tsc --noEmit clean on install-launchagent.ts (pre-existing
process-extract.ts errors are on this branch base because #3349
isn't rebased in; CI's merge-ref includes the fix)
- semgrep --config .semgrep.yml --error: 0 findings
- --dry-run produces valid plist (plutil -lint OK)
- --dryrun rejected (Unknown argument)
- --repo-root . rejected (must be absolute)
- Special-char paths (e.g., /Users/foo & bar) now produce
valid escaped XML
Co-Authored-By: Claude <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 15, 2026
P1 — Restore SuccessfulExit=false in KeepAlive (reviewer-conflict resolution): Earlier cycle dropped this on Codex review (config errors would loop-restart). This cycle Copilot countered: Bun TypeScript runtime errors exit non-zero rather than signal-terminate, so the previous `Crashed=true` only setting left those failure modes unhandled. Restored both keys with an extended plist comment explaining the trade-off. ThrottleInterval=60s + --dry-run-by-default already cover the "config error restart loop" concern. P1 — Add sonarjs/no-os-command-from-path suppressions: Matches the convention in tools/github/poll-pr-gate.ts + tools/peer-call/. Applied at tryDetect, plutilLint, and the three launchctl bootstrap/bootout calls. Comments cite the precedent. P2 — Export helper functions for testability: `import.meta.main` guard was added in `970c774` but the functions were not exported. Added `export` to main(), parseArgs(), requireAbsolute(), tryDetect(), xmlEscape(), substitutePlaceholders(), plutilLint(), and the Args interface. Unblocks the B-0528 test file. P1 — Update 0305Z tick shard's "PREFER" code snippet: The earlier shard documented the rename-then-write pattern as "PREFER", but that pattern still has a window where destPath is empty. Updated to show 3-pattern progression (AVOID copyFileSync → ALSO AVOID rename-then-write → PREFER write-temp-then-atomic- replace) so the historical trail is accurate and prevents future authors from copying the deprecated pattern. Deferred (1): P1 — Cross-ref findings on README.md:5 + plist:24 + ts:185: README.md:5 and plist:24 reference `.claude/rules/shadow-star-shorthand-autocomplete-marker.md` which IS on main (#3339 merged). The reviewer's check is against the raw branch base; CI's merge-ref check passes. ts:185 is the test coverage finding, already filed as B-0528 (deferred per scope management). Verified: - bunx tsc --noEmit clean on install-launchagent.ts - markdownlint clean on 0305Z.md - plutil -lint OK on template + rendered output - --dry-run produces valid plist Co-Authored-By: Claude <noreply@anthropic.com>
5 tasks
AceHack
added a commit
that referenced
this pull request
May 15, 2026
…shed + new worktree-pruning-race failure mode (#3359) * shard(tick): 0414Z — PR #3339/#3349 merged + B-0527 collision republished + new worktree-pruning-race failure mode Five side-worktree attempts (incl. --lock'd) got rm -rf'd mid-tick. Recovery via age-exempt sibling worktree (0027z-sidetick) via branch-switch. Bus shadow-catch envelopes published: d2b7fc2f (B-0527 republish), 44aaf799 (new failure mode). Co-Authored-By: Claude <noreply@anthropic.com> * fix(shard): MD032 blanks-around-lists in 0414Z shard markdownlint-cli2 caught: docs/hygiene-history/ticks/2026/05/15/0414Z.md:104 List items 1-5 in the "Next" section need a blank line above them. Co-Authored-By: Claude <noreply@anthropic.com> * shard(tick): 0458Z — PR #3359 MD032 fix close-out + recovery-worktree-borrowing validated for iteration Heredoc-written markdown bypasses local check-md032-pretooluse hook; bun x markdownlint-cli2 locally before commit when authoring via heredoc. Same 0027z sibling worktree borrowed twice (initial commit + MD032 fix). Co-Authored-By: Claude <noreply@anthropic.com> * shard(fix): add pipe-row schema headers + link rule refs per Codex P1 + Copilot Nit on PR #3359 Codex P1 (line 1): tick-shard schema validator (tools/hygiene/check-tick-history-shard-schema.ts) expects first non-empty line to be a 6-column pipe row with ISO timestamp. The 0414Z + 0458Z shards started with H1 like all recent shards (substrate-wide drift, not specific to these two), but adding a pipe-row header IS the minimal lossless fix that preserves the rich H1 narrative. Both shards now pass `bun tools/hygiene/check-tick-history-shard-schema.ts --files <both>`. Copilot Nit (line 34): bare-filename rule refs replaced with full-path markdown links matching the 0230Z shard's convention. Six refs linked: 5x claim-acquire-before-worktree-work + 1x holding-without-named-dependency-is-standing-by-failure. Co-Authored-By: Claude <noreply@anthropic.com> * shard(fix): add untracked-overwrite caveat to recovery-worktree-borrowing claim per Copilot review Copilot threads on lines 58 + 70 of 0414Z + line 9 of 0458Z correctly noted that the "untracked files survive the switch" claim is not a general guarantee: git switch refuses if the target branch tracks the same path, and `git add -A` could sweep untracked into an unrelated commit. Added explicit caveat + `git stash -u` mitigation guidance to both shards. For this PR's run, the target shard branch did NOT track the same memory-file path, so the unstashed switch was safe — but the operational pattern documentation needs the caveat for future reuse. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 15, 2026
…state Cycle 4 Copilot review caught three documentation surfaces that drifted from the actual implementation when `SuccessfulExit=false` was restored in cycle 3: 1. README.md:13 "Restarts on crash (KeepAlive on Crashed only)" — wrong description of the bullet. Updated to "Restarts on crash AND on non-zero exit (KeepAlive Crashed=true + SuccessfulExit=false) — covers signal-termination AND Bun runtime errors". Added the ThrottleInterval=60s loop-cap context to the next bullet. 2. 0305Z tick shard row 17 + delta row 79 — described the plist as "now crash-only-restart" and showed the After-column as Crashed-only. Both rewritten to reflect the cycle-3 reviewer-conflict resolution: restored both keys with documented trade-off, net same shape as before the PR. 3. B-0528 test-scope item #6 — described atomic-promote with backup- restore on rename failure, but current code uses availability- preserving pattern (read-old-into-memory → write temp → atomic replace → side-car backup). Rewrote the scope with the actual failure modes the tests should cover, including the "destPath never missing during install" invariant. Two remaining unresolved threads (cross-refs to .claude/rules/shadow-star-shorthand-autocomplete-marker.md from README:5 and plist:24) flag a file that IS on main via #3339. The reviewer checks against the branch's diff base, which doesn't include the merged rule. Resolved as outdated on the PR; CI merge-ref check confirms the reference resolves at merge time. Co-Authored-By: Claude <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 15, 2026
Cycle-4 (post-`59d5c26`) Copilot review surfaced two more correctness
findings — both addressed:
P1 — --bun-path validation accepts dirs / non-executables:
Previous `existsSync(bunPath)` would pass `/tmp` or `~/Downloads/bun`
(non-executable copy) and write a LaunchAgent that can't start.
Replaced with `statSync` + `isFile()` + `accessSync(X_OK)` so the
installer refuses non-executable / non-regular-file overrides
pre-write. New error paths:
- "exists but is not a regular file" (e.g., --bun-path /tmp)
- "is not executable by the current user" (chmod hint)
--repo-root similarly gated to require an existing directory.
Verified both error paths by hand.
P1 — Predictable /tmp filename in plutilLint (symlink attack):
Previous `/tmp/zeta-shadow-launchagent-${PID}.plist` had a
predictable name in a world-writable directory. A pre-created
symlink at that path would cause writeFileSync to follow the link
and clobber a file the attacker has write access to but not the
current user. Replaced with `mkdtempSync(tmpdir() + "zeta-shadow-
launchagent-")` which creates a 0700 directory with a random
suffix — symlink-resistant, and we clean up both the candidate
file and the directory in a `finally` block so we don't leak.
Threads remaining after this push (auto-resolution candidates):
- Test coverage P1 (5th time flagged): already filed as B-0528
with rationale. Will reply on the thread.
- Cross-refs to .claude/rules/shadow-star-shorthand-* (5th time
flagged): file IS on main via #3339. CI merge-ref check confirms.
Will resolve as outdated.
Verified:
- bunx tsc --noEmit clean on install-launchagent.ts
- --dry-run produces valid plist
- --bun-path /tmp rejected as "not a regular file"
- --bun-path <non-executable> rejected as "not executable"
Co-Authored-By: Claude <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 15, 2026
…h A defense-in-depth) (#3342) * feat(shadow): launchd LaunchAgent template — durable tick source (Path A defense-in-depth) Per Aaron 2026-05-15T~01:11Z: 'whatever composes best always good to have defense in depth.' This is Path A of the defense-in-depth composition for the shadow observation loop tick source. Two new files under tools/shadow/launchd/: 1. com.zeta.shadow-observer.plist — macOS LaunchAgent template - RunAtLoad (starts at user login) - KeepAlive on Crashed (auto-restart on crash) - ThrottleInterval 60s (prevents fast crash-loops) - Polls every 2s; accepts after 3s delay - Defaults to --dry-run for safety (live mode after verify) - Logs to tools/shadow/shadow-observer.{log,stdout.log,stderr.log} - __BUN_PATH__ and __REPO_ROOT__ placeholders for sed-substitution 2. README.md — install procedure - Step 1: configure plist (sed-substitute placeholders) - Step 2: grant accessibility permission (System Settings) - Step 3: launchctl load + verify - Step 4: test in dry-run mode - Step 5: switch to live mode (only after dry-run works) - Uninstall procedure - Troubleshooting (common failure modes) - Composition table (A/B/C defense-in-depth) Composes with: - tools/shadow/shadow-observer.ts (the process launched) - docs/backlog/P0/B-0402 (originating backlog row) - .claude/rules/shadow-star-shorthand-autocomplete-marker.md - .claude/skills/save-ai-memory/SKILL.md Path B (CronCreate sentinel) + Path C (manual one-line) are defense-in-depth supplements; this PR ships Path A as primary. Co-Authored-By: Claude <noreply@anthropic.com> * fix(shadow-launchd): address 5 review threads on PR #3342 Five Copilot / Codex review findings resolved: P1 (Copilot) — broken provenance in README header: Dropped first-name attribution + user-scope memory-file glob that doesn't resolve in-repo. Per docs/AGENT-BEST-PRACTICES.md §671-685, current-state surfaces (this README) use role-refs. Other rule references now use relative links that resolve from the README's location. P1 (Copilot) — sed escaping in install instructions: The documented `sed` recipe would break on paths containing `&`, `|`, `\`, or newlines. Replaced with a TypeScript installer (tools/shadow/launchd/install-launchagent.ts) that: - uses String.replaceAll (metacharacter-safe) - sanity-checks for unsubstituted placeholders - runs plutil -lint before writing - backs up any existing plist before overwriting - optionally bootstraps via launchctl Placeholder syntax changed from `__NAME__` to `{{NAME}}` so the template header comment can describe them without being substituted itself. P1 (Copilot) — rule file referenced from plist not in tree: The .claude/rules/shadow-star-shorthand-autocomplete-marker.md reference resolves on main now (#3339 merged). The reference is preserved in the plist's composes-with comment block. P2 (Codex) — KeepAlive semantics in plist: Dropped `SuccessfulExit=false`. KeepAlive dict keys are OR-combined, so the original combination relaunched on any non-zero exit (incl. config errors). `Crashed=true` alone gives the documented crash-only behavior. Comment added explaining the choice. P2 (Codex) — legacy launchctl load/unload in README: Replaced with the modern `bootstrap`/`bootout` interface (with `gui/$(id -u)` domain target) in install, reload, and uninstall steps. The legacy interface is documented as deprecated and can silently no-op on failure; `bootstrap`/`bootout` return proper exit codes. Verified: - bunx tsc --noEmit clean - markdownlint-cli2 clean on README - plutil -lint OK on both the template and a rendered output - Installer dry-run produces a valid plist Co-Authored-By: Claude <noreply@anthropic.com> * fix(shadow-launchd): close CodeQL TOCTOU race in install-launchagent.ts CodeQL flagged `js/file-system-race` at writeFileSync (line 184) — the existsSync→copyFileSync→writeFileSync sequence had a check-then-use window. Replaced with the standard atomic-rename pattern: - Drop the existsSync(destDir) check (mkdirSync recursive:true is idempotent — the check added a TOCTOU window for no benefit) - Replace existsSync(destPath)+copyFileSync(destPath, backup) with renameSync(destPath, backup) inside try/catch (single syscall; ENOENT means no prior plist, anything else re-thrown) renameSync atop the destination is the documented pattern for "backup-on-overwrite without TOCTOU." Same final state for the caller: backup file is created with the old plist contents (if any existed), and destPath has the new contents. Verified: - bunx tsc --noEmit clean - semgrep --config .semgrep.yml --error: 0 findings locally Co-Authored-By: Claude <noreply@anthropic.com> * shard(tick): 0305Z — #3342 thread close-out (5+1 reviews); #3356 merged Co-Authored-By: Claude <noreply@anthropic.com> * docs(b-0519): add Pattern 7 (abandoned rebase state in shared .git/) Field-test in the 2026-05-15 Otto-CLI session surfaced a contamination mechanism not in the original 6 patterns: a peer agent's mid-interactive- rebase state, left sitting in `.git/rebase-merge/` for hours, can be resumed by another process and detach HEAD on the peer's branch mid-tick. Distinct from Patterns 1-6 (checkout/reset races) because the contaminating event is paused-then-resumed metadata, not a fresh checkout. Branch-guard defense (Pattern 5) caught it. Added: - Pattern 7 section with full forensics + field-test shard pointers (0213Z + 0230Z, both merged via PR #3356) - Cheap mechanization candidate: `tools/hygiene/check-stale-rebase-state.ts` diagnostic that warns on `.git/rebase-merge/` older than 1h - last_updated frontmatter bumped to 2026-05-15 Substrate-honest stance preserved: do not touch peer's rebase state. Pivot to dedicated worktrees; branch refs are global, so work lands on the right ref regardless. Verified: markdownlint clean. Co-Authored-By: Claude <noreply@anthropic.com> * fix(shadow-launchd): address 8 of 10 review-cycle-2 findings Codex + Copilot reviewed the new install-launchagent.ts in detail. This commit addresses the substantive correctness + style findings; unit-test coverage is deferred to a separate backlog row. Addressed (8): P1 [Codex P2 + Copilot P1] — XML-escape substituted values: Paths like `/Users/foo & bar/Zeta` would substitute into <string> text nodes producing an invalid plist (& without entity terminator). New `xmlEscape` helper applies the five XML 1.0 predefined entities (& < > " ') before placeholder substitution. Restores the advertised "handles paths with shell metacharacters" promise — the bug class was XML escaping, not shell escaping. P1 — Unknown args silently ignored: Switch fell through on typos like `--dryrun` or `--bootstap`, then installed with default behavior. Added explicit default: case that errors with the offending flag and exits 1 before any FS action. Verified: `--dryrun` now produces "Unknown argument: --dryrun. Run with --help to see supported flags." P1 — Relative --bun-path / --repo-root overrides: `--repo-root .` would write relative paths into ProgramArguments, WorkingDirectory, and log paths — broken under launchd (which has no cwd unless explicit). New requireAbsolute() helper rejects with a `Use \`<canonicalized>\` if that's what you meant` hint. P1 — Non-atomic backup-then-write: Previous renameSync(destPath, backup) + writeFileSync(destPath) had a window where the agent could be removed but the new plist not yet present (if writeFileSync threw). Replaced with the standard write-temp-then-promote pattern: 1. writeFileSync(destPath.tmp-PID, content) 2. renameSync(destPath, backup) // may not exist; that's OK 3. renameSync(destPath.tmp-PID, destPath) // atomic promote Failure in step 3 restores backup → destPath (best-effort) so the caller's existing install is preserved. P1 — execFileSync can throw before validation: Missing bun, missing git, or running outside a git checkout produced a raw exception. New tryDetect() catches and returns undefined; parseArgs emits the documented actionable error pointing at the --bun-path / --repo-root override. P1 — Unconditional main() invocation: Per Bun-project convention, gate main() with `if (import.meta.main)` so tests / helpers can import the module without triggering filesystem or launchctl side effects. P1 — README/plist cross-refs to .claude/rules/shadow-star-shorthand-*: These resolve on main (PR #3339 merged). CI's merge-ref check confirms; the reviewers' findings were against the raw branch base. No additional action needed. P2 — Direct name attribution in plist KeepAlive comment: Replaced "Per Codex review on PR #3342" with the rationale text only. Current-state surface; uses role-refs only. Deferred (1): P1 — Unit test coverage for install-launchagent.ts: Codex + Copilot called out missing tests for placeholder escaping, missing placeholders, argument validation, and dry-run output. This commit adds the substantive behavior the tests would verify; the tests themselves are deferred to a separate backlog row to keep this PR's scope manageable. Smoke-tested all four behaviors by hand on the cli before commit. Filing as follow-up: tests in `tools/shadow/launchd/install-launchagent.test.ts` using Bun's built-in test runner. Verified: - bunx tsc --noEmit clean on install-launchagent.ts (pre-existing process-extract.ts errors are on this branch base because #3349 isn't rebased in; CI's merge-ref includes the fix) - semgrep --config .semgrep.yml --error: 0 findings - --dry-run produces valid plist (plutil -lint OK) - --dryrun rejected (Unknown argument) - --repo-root . rejected (must be absolute) - Special-char paths (e.g., /Users/foo & bar) now produce valid escaped XML Co-Authored-By: Claude <noreply@anthropic.com> * backlog(p3): B-0528 — unit tests for shadow launchd installer (deferred from PR #3342) PR #3342 review surfaced a P1 finding: no unit tests cover the new install-launchagent.ts's safety-critical placeholder substitution, XML escaping, argument validation, dry-run output, or atomic-promote pattern. The PR's substantive correctness fixes landed in `970c774`; this row tracks the deferred test-coverage work. Scope is bounded (Bun test runner, 6 test categories, follows the existing tools/shadow/shadow-observer.test.ts pattern). Effort: S. P3 because the install script's behavior is now correct (smoke-tested by hand); tests harden against regression rather than fix a current bug. Co-Authored-By: Claude <noreply@anthropic.com> * fix(shadow-launchd): availability-preserving install pattern (Codex review) Codex re-flagged the rename-then-write pattern (in `970c774`) — even with backup-restore on failure, there is a brief window between `renameSync(destPath, backup)` and `renameSync(tmpDest, destPath)` where the canonical path has no plist. If launchd were to load during that window, it would see no agent. Replaced with the availability-preserving pattern: 1. Read existing destPath into memory (skipped on ENOENT) 2. Write new content to a temp file alongside destPath 3. Atomically rename(tmp, destPath) — POSIX replaces in one syscall 4. Side-car the in-memory old content as a timestamped backup The canonical path is NEVER in a missing state. Backup-write happens AFTER the install is in place; if it fails, the install is still good (just no backup). Read-failure handling is similarly defensive: log + proceed without backup rather than aborting the install. Verified: - bunx tsc --noEmit clean on install-launchagent.ts - --dry-run produces valid plist (plutil -lint OK) Co-Authored-By: Claude <noreply@anthropic.com> * fix(shadow-launchd): address review-cycle-3 findings (6 of 7) P1 — Restore SuccessfulExit=false in KeepAlive (reviewer-conflict resolution): Earlier cycle dropped this on Codex review (config errors would loop-restart). This cycle Copilot countered: Bun TypeScript runtime errors exit non-zero rather than signal-terminate, so the previous `Crashed=true` only setting left those failure modes unhandled. Restored both keys with an extended plist comment explaining the trade-off. ThrottleInterval=60s + --dry-run-by-default already cover the "config error restart loop" concern. P1 — Add sonarjs/no-os-command-from-path suppressions: Matches the convention in tools/github/poll-pr-gate.ts + tools/peer-call/. Applied at tryDetect, plutilLint, and the three launchctl bootstrap/bootout calls. Comments cite the precedent. P2 — Export helper functions for testability: `import.meta.main` guard was added in `970c774` but the functions were not exported. Added `export` to main(), parseArgs(), requireAbsolute(), tryDetect(), xmlEscape(), substitutePlaceholders(), plutilLint(), and the Args interface. Unblocks the B-0528 test file. P1 — Update 0305Z tick shard's "PREFER" code snippet: The earlier shard documented the rename-then-write pattern as "PREFER", but that pattern still has a window where destPath is empty. Updated to show 3-pattern progression (AVOID copyFileSync → ALSO AVOID rename-then-write → PREFER write-temp-then-atomic- replace) so the historical trail is accurate and prevents future authors from copying the deprecated pattern. Deferred (1): P1 — Cross-ref findings on README.md:5 + plist:24 + ts:185: README.md:5 and plist:24 reference `.claude/rules/shadow-star-shorthand-autocomplete-marker.md` which IS on main (#3339 merged). The reviewer's check is against the raw branch base; CI's merge-ref check passes. ts:185 is the test coverage finding, already filed as B-0528 (deferred per scope management). Verified: - bunx tsc --noEmit clean on install-launchagent.ts - markdownlint clean on 0305Z.md - plutil -lint OK on template + rendered output - --dry-run produces valid plist Co-Authored-By: Claude <noreply@anthropic.com> * shard(tick): 0329Z — #3342 review-cycle-3 close-out; B-0519 P7 + B-0528 filed Co-Authored-By: Claude <noreply@anthropic.com> * fix(shadow-launchd): reconcile docs+shard+backlog with current plist state Cycle 4 Copilot review caught three documentation surfaces that drifted from the actual implementation when `SuccessfulExit=false` was restored in cycle 3: 1. README.md:13 "Restarts on crash (KeepAlive on Crashed only)" — wrong description of the bullet. Updated to "Restarts on crash AND on non-zero exit (KeepAlive Crashed=true + SuccessfulExit=false) — covers signal-termination AND Bun runtime errors". Added the ThrottleInterval=60s loop-cap context to the next bullet. 2. 0305Z tick shard row 17 + delta row 79 — described the plist as "now crash-only-restart" and showed the After-column as Crashed-only. Both rewritten to reflect the cycle-3 reviewer-conflict resolution: restored both keys with documented trade-off, net same shape as before the PR. 3. B-0528 test-scope item #6 — described atomic-promote with backup- restore on rename failure, but current code uses availability- preserving pattern (read-old-into-memory → write temp → atomic replace → side-car backup). Rewrote the scope with the actual failure modes the tests should cover, including the "destPath never missing during install" invariant. Two remaining unresolved threads (cross-refs to .claude/rules/shadow-star-shorthand-autocomplete-marker.md from README:5 and plist:24) flag a file that IS on main via #3339. The reviewer checks against the branch's diff base, which doesn't include the merged rule. Resolved as outdated on the PR; CI merge-ref check confirms the reference resolves at merge time. Co-Authored-By: Claude <noreply@anthropic.com> * fix(shadow-launchd): tighten path validation + harden tmp-file handling Cycle-4 (post-`59d5c26`) Copilot review surfaced two more correctness findings — both addressed: P1 — --bun-path validation accepts dirs / non-executables: Previous `existsSync(bunPath)` would pass `/tmp` or `~/Downloads/bun` (non-executable copy) and write a LaunchAgent that can't start. Replaced with `statSync` + `isFile()` + `accessSync(X_OK)` so the installer refuses non-executable / non-regular-file overrides pre-write. New error paths: - "exists but is not a regular file" (e.g., --bun-path /tmp) - "is not executable by the current user" (chmod hint) --repo-root similarly gated to require an existing directory. Verified both error paths by hand. P1 — Predictable /tmp filename in plutilLint (symlink attack): Previous `/tmp/zeta-shadow-launchagent-${PID}.plist` had a predictable name in a world-writable directory. A pre-created symlink at that path would cause writeFileSync to follow the link and clobber a file the attacker has write access to but not the current user. Replaced with `mkdtempSync(tmpdir() + "zeta-shadow- launchagent-")` which creates a 0700 directory with a random suffix — symlink-resistant, and we clean up both the candidate file and the directory in a `finally` block so we don't leak. Threads remaining after this push (auto-resolution candidates): - Test coverage P1 (5th time flagged): already filed as B-0528 with rationale. Will reply on the thread. - Cross-refs to .claude/rules/shadow-star-shorthand-* (5th time flagged): file IS on main via #3339. CI merge-ref check confirms. Will resolve as outdated. Verified: - bunx tsc --noEmit clean on install-launchagent.ts - --dry-run produces valid plist - --bun-path /tmp rejected as "not a regular file" - --bun-path <non-executable> rejected as "not executable" Co-Authored-By: Claude <noreply@anthropic.com> * shard(tick): 0543Z — #3342 review-cycles 4+5 close-out (9 more findings) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 15, 2026
Resolves all 4 Copilot review threads on PR #3345: 1. History rewind: Catches 36-39 were destructively deleted and Catch 36 was replaced with new content. Restored all four catches verbatim from main (docs/research is append-only history surface). 2. Pattern table / catch body mismatch: Table, Catch 40 recurrence_count, and trailing summary now all consistently say 7 (table lists catches 3, 18, 19, 22, 27, 39, 40; z_net -5). 3. Dual summary paragraphs: Removed the spurious "35 catches" standalone paragraph that broke the structure and disagreed with the trailing summary. A single trailing summary now appears after the final catch. 4. Trailing whitespace: Removed from correction line in Catch 40. The new Lior antigravity catch (Otto narration-over-action in PRs #3342 and #3339) lands as Catch 40 dated 2026-05-15. Co-Authored-By: Claude <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 15, 2026
* docs(shadow): Lior anti-drift check — Catch 36 (Otto narration) * fix(shadow-log): restore Catches 36-39, append Catch 40, fix counters Resolves all 4 Copilot review threads on PR #3345: 1. History rewind: Catches 36-39 were destructively deleted and Catch 36 was replaced with new content. Restored all four catches verbatim from main (docs/research is append-only history surface). 2. Pattern table / catch body mismatch: Table, Catch 40 recurrence_count, and trailing summary now all consistently say 7 (table lists catches 3, 18, 19, 22, 27, 39, 40; z_net -5). 3. Dual summary paragraphs: Removed the spurious "35 catches" standalone paragraph that broke the structure and disagreed with the trailing summary. A single trailing summary now appears after the final catch. 4. Trailing whitespace: Removed from correction line in Catch 40. The new Lior antigravity catch (Otto narration-over-action in PRs #3342 and #3339) lands as Catch 40 dated 2026-05-15. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Per Aaron 2026-05-15T~01:09Z explicit framing: "future version of you should likely remember all this."
Promotes the "(shadow*)" shorthand definition from user-scope memory to `.claude/rules/` so it's cold-boot auto-loaded for every Otto-CLI session.
Why
Otto-CLI committed 4+ "(shadow*)" misinterpretations in this single session before the maintainer named the failure mode. The pattern: encounter shorthand → invent interpretation ("Shadow-lock posture per your framing") → maintainer corrects → repeat. Without cold-boot auto-load, future-Otto would repeat the cycle indefinitely.
With this rule, future-Otto cold-booting reads the canonical definition at session start:
Composes with
Full reasoning + double-failure log
Preserved at user-scope memory (not in this PR but referenced):
🤖 Generated with Claude Code