fix: converge Polylogue hook writers on configured archive root - #14
fix: converge Polylogue hook writers on configured archive root#14Sinity wants to merge 6 commits into
Conversation
Use the installed polylogue-hook for Claude and Codex without baked sidecar paths. Make parity tests exercise the generated archive root and remove the wrapper-only Home Manager surface.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdc7e645a1
ℹ️ 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".
| # These are Polylogue archive inputs, so their destination must follow | ||
| # the same archive-root option as the daemon and hook spool. | ||
| systemd.tmpfiles.rules = [ | ||
| "d ${cfg.dataDir}/inbox 0755 ${userName} users -" |
There was a problem hiding this comment.
On a fresh activation where cfg.dataDir does not yet exist, this nested d rule causes systemd-tmpfiles to create the missing archive-root parent as root:root; only the final inbox directory receives ${userName} ownership. The user-run hooks and daemon then cannot create ${cfg.dataDir}/hooks or other archive files. Add an explicit tmpfiles rule for cfg.dataDir with the service user's ownership before creating inbox.
Useful? React with 👍 / 👎.
| { | ||
| type = "command"; | ||
| command = "polylogue-hook SessionStart --provider codex --sidecar-dir /home/sinity/.local/share/polylogue/hooks"; | ||
| command = "polylogue-hook SessionStart --provider codex"; |
There was a problem hiding this comment.
When Codex is launched from a shell or project environment that sets POLYLOGUE_ARCHIVE_ROOT or POLYLOGUE_CONFIG, this new flag-less command allows the inherited override to win over the generated polylogue.toml; notably, the added smoke test avoids this exact case by explicitly unsetting both variables. The hook envelopes then land in a fixture or stale archive instead of the configured shared spool, so clear those overrides when invoking the hook or otherwise bind the command to the generated configuration.
Useful? React with 👍 / 👎.
| "${home}/.cache/claude-cli-nodejs" | ||
| "${home}/.local/state/claude-code" | ||
| "/realm/state/polylogue/hooks" | ||
| "${polylogueDataDir}/hooks" |
There was a problem hiding this comment.
When enrichment starts against a fresh or newly configured archive before any agent hook has created ${polylogueDataDir}/hooks, systemd treats the nonexistent unprefixed ReadWritePaths= entry as a namespace setup error and refuses to start the service. This conflicts with sinnix-enrich-dump, which explicitly treats an absent hooks directory as an optional input; prefix this path with - or create the hooks directory declaratively.
Useful? React with 👍 / 👎.
| echo 'repository hook command bakes a Polylogue sidecar path' >&2 | ||
| exit 1 | ||
| fi | ||
| test ! -e "$repoRoot/modules/features/dev/agents/polylogue-hook.nix" |
There was a problem hiding this comment.
This assertion only freezes the removal of a particular helper filename and does not verify any evaluated or runtime behavior, so a future valid helper at that path would fail the check even if every hook still targets the correct archive. The repository explicitly requires deletion to be verified through evaluation/build/runtime behavior and a final source census rather than tests that merely require an old spelling to disappear.
AGENTS.md reference: AGENTS.md:L193-L194
Useful? React with 👍 / 👎.
|
Superseded by direct master integration in 68da68f. The functional hook and archive-root parity changes are patch-equivalent in master; the unmatched intermediate documentation commit was superseded by the final hook design. |
1 participant
Summary
Make Claude and Codex invoke the installed upstream
polylogue-hookwithout baked sidecar paths, so generated Polylogue configuration is the single archive-root authority.Problem
Codex hook settings wrote envelopes under the XDG fallback while the daemon watched the configured
/realm/state/polyloguearchive. A proposed wrapper introduced an activation-order gap because Claude settings are live symlinks.Solution
Retain the already-installed upstream hook command for both clients, remove baked
--sidecar-dirarguments and the unnecessary wrapper/options, and exercise root resolution through generatedpolylogue.toml. Preserve enrichment hardening and move inbox links under the configured archive root.Verification
lintjob47b6c9db-6991-4116-a678-b74885244827: succeeded atcdc7e645a13087e9648a034d407531a8b0393211.POLYLOGUE_ARCHIVE_ROOTunset.e488f96b-ccee-4ff9-8a94-6a3dd7c214fb: PASS.No host activation or spool movement is included.