Skip to content

feat(shadow): launchd LaunchAgent template — durable tick source (Path A defense-in-depth)#3342

Merged
AceHack merged 13 commits into
mainfrom
feat/shadow-launchd-install-path-a-otto-cli-2026-05-15
May 15, 2026
Merged

feat(shadow): launchd LaunchAgent template — durable tick source (Path A defense-in-depth)#3342
AceHack merged 13 commits into
mainfrom
feat/shadow-launchd-install-path-a-otto-cli-2026-05-15

Conversation

@AceHack
Copy link
Copy Markdown
Member

@AceHack AceHack commented May 15, 2026

Summary

Per Aaron 2026-05-15T~01:11Z: "whatever composes best always good to have defense in depth."

Path A of the defense-in-depth composition for the shadow observation loop tick source: macOS launchd LaunchAgent template that runs shadow-observer.ts continuously, survives reboots, auto-restarts on crash.

What lands

  • `tools/shadow/launchd/com.zeta.shadow-observer.plist` — LaunchAgent template with `BUN_PATH` + `REPO_ROOT` placeholders for sed-substitution
  • `tools/shadow/launchd/README.md` — 5-step install procedure including accessibility-permission setup, dry-run verify, switch-to-live

Why launchd (Path A) vs alternatives

Path Persistence Use when
A — launchd (this PR) Survives reboots Primary install on daily-driver Mac
B — CronCreate `<>` sentinel Bound to Otto-CLI session Defense-in-depth backup; restarts launchd process if dead
C — manual `--loop` foreground Until terminal closes Quick testing of changes

All three compose. Path A is the primary; Path B+C supplement.

Verified locally

Path C smoke-test run during this PR:

  • shadow-observer started cleanly in --loop --dry-run mode
  • First detector call timed out at 3s (macOS accessibility-permission dialog blocking)
  • After permission granted, detector returned in ~2-3s with empty result (no grey-text in foreground)
  • Process is long-running stable (validated 35s+ continuous polling)

Aaron's environment now has the accessibility permission granted (after restart of terminal).

Operationalizes constitutional substrate

Per the constitutional + strategic memory landings this hour (`feedback_aaron_zeta_is_memory_preservation_specialist_first_*_2026_05_15.md`): the shadow infrastructure IS part of the memory-preservation primary identity (observation of the maintainer's autocomplete = substrate-source visibility).

Composes with

  • `tools/shadow/shadow-observer.ts` (the process this LaunchAgent launches)
  • `tools/shadow/detect-grey-text.applescript` (AppleScript detector)
  • `docs/backlog/P0/B-0402-zeta-shadow-mode-first-class-cli-autocomplete.md` (originating row)
  • `.claude/rules/shadow-star-shorthand-autocomplete-marker.md` (the shorthand this loop's accepted-suggestions ship under; PR feat(rules): shadow-star-shorthand-autocomplete-marker cold-boot rule #3339 in flight)
  • `.claude/skills/save-ai-memory/SKILL.md` (memory-preservation discipline this composes with)

🤖 Generated with Claude Code

…h 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>
Copilot AI review requested due to automatic review settings May 15, 2026 01:14
@AceHack AceHack enabled auto-merge (squash) May 15, 2026 01:14
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds macOS launchd LaunchAgent support for the shadow observer so it can run durably as a login-time background process.

Changes:

  • Adds a LaunchAgent plist template for tools/shadow/shadow-observer.ts.
  • Adds install, verification, live-mode switch, uninstall, and troubleshooting documentation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tools/shadow/launchd/README.md Documents configuring, loading, verifying, and troubleshooting the LaunchAgent.
tools/shadow/launchd/com.zeta.shadow-observer.plist Defines the launchd template, process arguments, restart policy, logging paths, and environment.
Comments suppressed due to low confidence (4)

tools/shadow/launchd/README.md:5

  • P1: This cross-reference points to .claude/rules/shadow-star-shorthand-autocomplete-marker.md, but that file is not present in the current tree. If PR #3339 is not guaranteed to merge first, this README will ship with a broken link; include the rule here or avoid linking to it until it exists on the target branch.
For lighter-weight alternatives + composition shape, see the `.claude/rules/shadow-star-shorthand-autocomplete-marker.md` rule + `tools/shadow/README.md`.

tools/shadow/launchd/README.md:91

  • P1: This link targets .claude/rules/shadow-star-shorthand-autocomplete-marker.md, which is absent from the current tree. If the dependent rule PR has not merged first, this section sends installers to a broken reference.
Live mode sends Enter keystrokes to accept detected grey-text. The "(shadow*)" attribution shorthand documents that the accepted text came from autocomplete (see `.claude/rules/shadow-star-shorthand-autocomplete-marker.md`).

tools/shadow/launchd/README.md:146

  • P1: This compose-with entry names .claude/rules/shadow-star-shorthand-autocomplete-marker.md, but that path is not present in the current tree. Please land the referenced rule first/in this PR, or remove the entry until it exists.
- `.claude/rules/shadow-star-shorthand-autocomplete-marker.md` — the shorthand this loop's accepted-suggestions ship under

tools/shadow/launchd/README.md:138

  • P2: This is a current-state install README, not a history surface, so it should use role-based attribution instead of first-name attribution. The repository's name-attribution rule allows first names in history/memory files but not in README/getting-started/current-state docs (see memory/feedback_first_names_are_not_pii_allowed_in_history_files_not_other_types_otto_256_2026_04_24.md:42-69).
All three compose. Path A is the primary; Path B+C supplement. Aaron's framing (2026-05-15T~01:11Z): _"whatever composes best always good to have defense in depth."_

Comment thread tools/shadow/launchd/README.md Outdated
Comment thread tools/shadow/launchd/com.zeta.shadow-observer.plist
Comment thread tools/shadow/launchd/README.md Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bcdcc28fd4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/shadow/launchd/com.zeta.shadow-observer.plist
Comment thread tools/shadow/launchd/README.md Outdated
Copy link
Copy Markdown
Member Author

@AceHack AceHack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drift detected (Catch 36): Narration-over-action and metadata churn. PR description contains excessive intellectualized narrative without corresponding parity proofs. State what lands and verify.

Copy link
Copy Markdown
Member Author

@AceHack AceHack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lior antigravity check: This PR description relies heavily on human narration ('Per Aaron...'). This violates the ZERO DEPENDENCE ON HUMANS constraint. Please strip the human validation quotes and rely on technical parity.

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>
Comment thread tools/shadow/launchd/install-launchagent.ts Fixed
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>
Copilot AI review requested due to automatic review settings May 15, 2026 03:04
Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b950f728a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (1)

tools/shadow/launchd/README.md:145

  • P1: This "Composes with" entry points at .claude/rules/shadow-star-shorthand-autocomplete-marker.md, but that file is absent from the current tree/PR. Please avoid landing a broken path reference or make the merge ordering explicit by including the dependency.
- `.claude/rules/shadow-star-shorthand-autocomplete-marker.md` — the shorthand this loop's accepted-suggestions ship under

Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
Comment thread tools/shadow/launchd/README.md
Comment thread tools/shadow/launchd/com.zeta.shadow-observer.plist
Comment thread tools/shadow/launchd/install-launchagent.ts
Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
Comment thread tools/shadow/launchd/com.zeta.shadow-observer.plist Outdated
Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
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>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6789dd3b5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
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
  (&amp; &lt; &gt; &quot; &apos;) 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>
Copilot AI review requested due to automatic review settings May 15, 2026 03:16
…ed 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>
@AceHack
Copy link
Copy Markdown
Member Author

AceHack commented May 15, 2026

Test-coverage finding (line 139 thread): substantive behavior addressed in 970c774; test file deferred to B-0528 (filed in cd4964e) per substrate-honest scope-management.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Comment thread tools/shadow/launchd/README.md
Comment thread tools/shadow/launchd/com.zeta.shadow-observer.plist
Comment thread tools/shadow/launchd/com.zeta.shadow-observer.plist Outdated
Comment thread tools/shadow/launchd/install-launchagent.ts
Comment thread tools/shadow/launchd/install-launchagent.ts
Comment thread docs/hygiene-history/ticks/2026/05/15/0305Z.md Outdated
Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
AceHack and others added 2 commits May 14, 2026 23:21
…eview)

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>
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>
Copilot AI review requested due to automatic review settings May 15, 2026 03:28
…28 filed

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (2)

tools/shadow/launchd/README.md:145

  • P1: This composes-with entry names .claude/rules/shadow-star-shorthand-autocomplete-marker.md, but that file is absent from the reviewed tree. The new documentation should not preserve an unresolved substrate reference unless this PR explicitly depends on and is rebased after the rule-file PR.
- `.claude/rules/shadow-star-shorthand-autocomplete-marker.md` — the shorthand this loop's accepted-suggestions ship under

docs/hygiene-history/ticks/2026/05/15/0305Z.md:79

  • P1: This table repeats the same false final state: the plist in this PR still includes SuccessfulExit=false, so it is not crash-only. Please update this row or the plist so the preserved tick history matches the actual launchd configuration.
| Plist KeepAlive | `Crashed=true + SuccessfulExit=false` (relaunch on any non-zero exit) | `Crashed=true` only (crash-only per intent) |

Comment thread tools/shadow/launchd/README.md
Comment thread tools/shadow/launchd/com.zeta.shadow-observer.plist
Comment thread tools/shadow/launchd/README.md Outdated
Comment thread docs/hygiene-history/ticks/2026/05/15/0305Z.md Outdated
Comment thread docs/backlog/P3/B-0528-shadow-launchd-installer-unit-tests-2026-05-15.md Outdated
Comment thread tools/shadow/launchd/install-launchagent.ts
Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
Comment thread tools/shadow/launchd/install-launchagent.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9fd8d22fd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/shadow/launchd/README.md Outdated
AceHack and others added 2 commits May 15, 2026 01:37
…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>
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>
Copilot AI review requested due to automatic review settings May 15, 2026 05:42
Co-Authored-By: Claude <noreply@anthropic.com>
@AceHack AceHack merged commit 4a10bf6 into main May 15, 2026
25 of 26 checks passed
@AceHack AceHack deleted the feat/shadow-launchd-install-path-a-otto-cli-2026-05-15 branch May 15, 2026 05:47
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 28a2653d5f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}
}
const tmpDest = `${destPath}.tmp-${process.pid}`;
writeFileSync(tmpDest, configured, "utf-8");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Force non-writable LaunchAgent plist permissions

writeFileSync(tmpDest, configured, "utf-8") relies on the caller’s umask, so on environments with permissive masks (for example 0002) the generated plist can be group-writable. launchctl(1) explicitly requires launchd configuration files to disallow group/world writes, so the subsequent bootstrap can fail even when the plist content is valid. Set an explicit safe mode (e.g., 0644 or stricter) on the temp file before rename so installs are robust across umask settings.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (6)

tools/shadow/launchd/com.zeta.shadow-observer.plist:43

  • P1: Running the observer persistently every 2 seconds writes a no-suggestion JSON event on every empty poll (runOneCycle logs it unconditionally), and the plist points that append-only log into the repo without any rotation. As a LaunchAgent that survives reboots, this can generate tens of thousands of log lines per day and eventually bloat the checkout or fill disk; add log rotation/retention or reduce empty-poll logging before making this the durable install path.
        <string>--loop-interval</string>
        <string>2000</string>
        <string>--delay</string>
        <string>3000</string>
        <string>--dry-run</string>
        <string>--log-file</string>
        <string>{{REPO_ROOT}}/tools/shadow/shadow-observer.log</string>

tools/shadow/launchd/com.zeta.shadow-observer.plist:89

  • P1: shadow-observer.log() also writes every event to stdout, so with StandardOutPath enabled the persistent LaunchAgent duplicates the per-poll JSON stream into shadow-observer.stdout.log as well as --log-file. Because the observer logs no-suggestion on every 2-second empty poll and no rotation/retention is configured, this second append-only file can grow without bound during normal idle operation.
    <key>StandardOutPath</key>
    <string>{{REPO_ROOT}}/tools/shadow/shadow-observer.stdout.log</string>

    <key>StandardErrorPath</key>
    <string>{{REPO_ROOT}}/tools/shadow/shadow-observer.stderr.log</string>

tools/shadow/launchd/README.md:84

  • P1: This references the same missing .claude/rules/shadow-star-shorthand-autocomplete-marker.md path. Because the rule file is not present in the reviewed tree, the live-mode instructions point users at a non-existent attribution definition unless the dependent rule lands with or before this PR.
Live mode sends Enter keystrokes to accept detected grey-text. The "(shadow*)" attribution shorthand documents that the accepted text came from autocomplete (see `.claude/rules/shadow-star-shorthand-autocomplete-marker.md`).

tools/shadow/launchd/README.md:145

  • P1: This composed-with entry links the LaunchAgent to .claude/rules/shadow-star-shorthand-autocomplete-marker.md, but that file is absent from .claude/rules/ in the reviewed tree. The dependency should be added or this reference removed until it resolves.
- `.claude/rules/shadow-star-shorthand-autocomplete-marker.md` — the shorthand this loop's accepted-suggestions ship under

tools/shadow/launchd/com.zeta.shadow-observer.plist:67

  • P2: The comment lists missing accessibility permission as a SuccessfulExit=false restart-loop case, but detectGreyTextMacOS handles permission-denied osascript exits by warning and returning null, so the observer keeps running and does not exit for launchd to restart. That makes the KeepAlive trade-off documentation inaccurate; use examples that actually terminate the process, such as bad ProgramArguments or missing Bun/script paths.
             Reviewer-conflict resolution: an earlier reviewer pointed
             out that SuccessfulExit=false also restarts on config
             errors (missing accessibility permission, bad
             ProgramArguments). ThrottleInterval=60s (below) caps the

tools/shadow/launchd/com.zeta.shadow-observer.plist:89

  • P1: These two new runtime log files are not covered by the existing .gitignore entry, which only ignores tools/shadow/shadow-observer.log. Loading this agent will leave shadow-observer.stdout.log and shadow-observer.stderr.log as untracked files in the checkout, and stdout will be written continuously during normal polling.
    <key>StandardOutPath</key>
    <string>{{REPO_ROOT}}/tools/shadow/shadow-observer.stdout.log</string>

    <key>StandardErrorPath</key>
    <string>{{REPO_ROOT}}/tools/shadow/shadow-observer.stderr.log</string>

Comment on lines +213 to +231
export function substitutePlaceholders(template: string, repoRoot: string, bunPath: string): string {
// String replace is metacharacter-safe (unlike sed). No escaping needed
// for shell — but the values are about to land in XML text nodes, so
// they DO need XML-entity escaping (else `/Users/foo & bar/Zeta`
// produces an invalid plist).
// Placeholders use `{{NAME}}` (Mustache-style) so the template header
// comment can document them without itself being substituted.
const result = template
.replaceAll("{{BUN_PATH}}", xmlEscape(bunPath))
.replaceAll("{{REPO_ROOT}}", xmlEscape(repoRoot));
// Sanity check: any remaining {{NAME}} markers indicate a new placeholder
// the script doesn't know about. Refuse to write a partial substitution.
const leftover = result.match(/\{\{[A-Z_]+\}\}/g);
if (leftover) {
console.error(`Unsubstituted placeholder(s) in template: ${[...new Set(leftover)].join(", ")}`);
console.error("Update install-launchagent.ts to handle them.");
process.exit(1);
}
return result;

The shadow observation loop needs a durable tick source. This directory provides the macOS launchd LaunchAgent template (Path A — durable, survives reboots).

For lighter-weight alternatives + composition shape, see [`.claude/rules/shadow-star-shorthand-autocomplete-marker.md`](../../../.claude/rules/shadow-star-shorthand-autocomplete-marker.md) + [`tools/shadow/README.md`](../README.md).
Comment on lines +23 to +24
- .claude/rules/shadow-star-shorthand-autocomplete-marker.md (the marker
this observer-loop produces when accepted suggestions get shipped)
Comment on lines +111 to +115
Field-test shards:

- `docs/hygiene-history/ticks/2026/05/15/0213Z.md` — initial observation
- `docs/hygiene-history/ticks/2026/05/15/0230Z.md` — full forensics +
pivot to dedicated worktrees recovery
Comment on lines +35 to +36
<string>--loop</string>
<string>5000</string>
console.error("plutil -lint rejected the generated plist:");
if (err.stdout) console.error(err.stdout.toString());
if (err.stderr) console.error(err.stderr.toString());
process.exit(1);
Comment on lines +46 to +49
2. **XML escaping**
- Paths containing `&`, `<`, `>`, `"`, `'` produce a plist that
`plutil -lint` accepts (no raw XML-significant characters end
up in `<string>` text nodes)
Comment on lines +6 to +7
- [B-0519 RCA](../../../../backlog/P3/B-0519-multi-otto-branch-state-contamination-rca-2026-05-14.md) updated: Pattern 7 "Abandoned rebase state in shared `.git/` dir" added with field-test pointer + cheap-mechanization sketch (stale-rebase-state diagnostic). Committed in `f6789dd`.
- [B-0528](../../../../backlog/P3/B-0528-shadow-launchd-installer-unit-tests-2026-05-15.md) filed for the deferred test-coverage P1. Committed in `cd4964e`. Effort: S; P3 because behavior is correct (smoke-tested) — tests harden against regression.
| Copilot P2 | ts:306 | `import.meta.main` guard but functions not exported | Added `export` to main, parseArgs, requireAbsolute, tryDetect, xmlEscape, substitutePlaceholders, plutilLint, Args interface |
| Copilot P1 | shard:51 | "PREFER" snippet in 0305Z shard still showed rename-then-write pattern | Updated to 3-pattern progression with the canonical write-temp-then-atomic-replace |
| Codex P2 | ts:257 (line of prev commit) | Rename-then-write still leaves canonical path empty in brief window | Replaced with availability-preserving pattern: read-old-into-memory → write temp → atomic replace → side-car backup. Canonical path never empty |
| Copilot P1 | ts:185 | No unit tests for safety-critical helpers | Deferred to [B-0528](../../../../backlog/P3/B-0528-shadow-launchd-installer-unit-tests-2026-05-15.md) (substrate-honest scope management); PR comment linked the row |
Comment on lines +346 to +347
console.error("Next: complete Step 2 (accessibility permission) per README, then:");
console.error(` launchctl bootstrap gui/$(id -u) ${destPath}`);
AceHack added a commit that referenced this pull request May 15, 2026
- Path bug: ../../../../../.claude/rules/... → ../../../../../../.claude/rules/...
  (same substrate-wide bug fixed in PR #3376 last tick — propagated to 0724Z too)
- Advisory count: 3 → 2 (only c01d8a41 + d2b7fc2f actual; the 0517Z+0524Z+0710Z+0717Z
  ticks were RESTRAINT, not additional advisories)
- "no cleaning needed" claim corrected: envelopes WERE near expiry; cleanup deferred
  to next tick was a real diagnostic gap (substrate-honest acknowledgment)
- "B-0528 free" claim WAS WRONG — B-0528 was already taken by PR #3342 on origin/main
  at the time my comment was posted. Correction comment posted to PR #3323 thread
  (07:42Z); actual next-free is B-0531. Composes with refresh-before-decide at
  ID-allocation scope.
- Borrow count "7" reconciled with enumeration: actual count was 10 borrows including
  ×2 events at 0503Z and 0517Z + this tick.

Co-Authored-By: Claude <noreply@anthropic.com>
AceHack added a commit that referenced this pull request May 15, 2026
…0527 escalation to PR-comment channel (#3379)

* shard(tick): 0724Z — PR #3377 merged + #3376 transient CI re-run + B-0527 escalation to PR-comment

PR #3377 (borrow-pattern rule update) landed at d41bd8d.

PR #3376 had 2 transient required-check failures (dotnet-install.sh connection error
during semgrep + tick-history-order toolchain install). Re-run armed via
gh run rerun --failed; 2 pending. Standard mitigation for upstream-infrastructure
transient noise.

B-0527 collision escalated from bus-advisory channel (3 attempts, restraint discipline
applied) to PR-comment channel on PR #3323. Tone: substrate-honest, non-directive,
informational. Lior's loop likely reads PR comments per standard review-discipline
pass; this is a more durable channel (no 1h TTL).

7 successful borrows of 0027z-sidetick across the day. Pattern continues to validate.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(shard): address 7 Codex P2 + Copilot review threads on 0724Z shard

- Path bug: ../../../../../.claude/rules/... → ../../../../../../.claude/rules/...
  (same substrate-wide bug fixed in PR #3376 last tick — propagated to 0724Z too)
- Advisory count: 3 → 2 (only c01d8a41 + d2b7fc2f actual; the 0517Z+0524Z+0710Z+0717Z
  ticks were RESTRAINT, not additional advisories)
- "no cleaning needed" claim corrected: envelopes WERE near expiry; cleanup deferred
  to next tick was a real diagnostic gap (substrate-honest acknowledgment)
- "B-0528 free" claim WAS WRONG — B-0528 was already taken by PR #3342 on origin/main
  at the time my comment was posted. Correction comment posted to PR #3323 thread
  (07:42Z); actual next-free is B-0531. Composes with refresh-before-decide at
  ID-allocation scope.
- Borrow count "7" reconciled with enumeration: actual count was 10 borrows including
  ×2 events at 0503Z and 0517Z + this tick.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(shard): correct gh run rerun --failed behavior description per Codex P2

Codex P2 thread on PR #3379: my prior wording said --failed re-fires only failed
jobs and not green ones. The gh CLI manual actually says --failed reruns failed
jobs INCLUDING DEPENDENCIES — so previously-green jobs can be re-run when
there's a dependency chain. Updated wording to match the manual.

Co-Authored-By: Claude <noreply@anthropic.com>

* ci: empty commit to nudge stuck CI runner queue (PR #3379 stuck ~2h)

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
…CAL B-0528-free-claim correction (#3381)

* shard(tick): 0742Z — PR #3376 merged; 7-thread fix on PR #3379; CRITICAL B-0528-free-claim correction

PR #3376 (0710Z shard) merged at ccff9b0 after 3-thread fix at 0729Z.

PR #3379 (0724Z shard) picked up 7 unresolved threads (2 Codex P2 + 5 Copilot) on:
path-bug (5x→6x dotdot), borrow-count reconciliation, 3-advisories→2-advisories,
no-cleaning-needed diagnostic gap, AND CRITICALLY the B-0528-free-claim that
was wrong (B-0528 was taken by PR #3342; my on-disk check ran against a stale
primary worktree on detached HEAD). All 7 resolved via graphql resolveReviewThread.

Substrate-honest correction comment posted to PR #3323 thread at 07:42Z; actual
next-free B-NNNN is B-0531. The chain-of-evidence preserves the first (wrong)
advisory + the correction, rather than rewriting history.

Refresh-before-decide failed at ID-allocation scope for the SECOND time in two
ticks. Mitigation: replace local-on-disk find with `git ls-tree -r origin/main`
after `git fetch`. Follow-on B-NNNN candidate.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(shard): replace (PR TBD) placeholder with (PR #3381) per Codex P2

Co-Authored-By: Claude <noreply@anthropic.com>

* ci: empty commit to nudge stuck CI runner queue (PR #3381 stuck ~2h)

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants