feat(core): add bm hook producer front door for harness capture#1070
Merged
Conversation
…t door Port the SPEC-55 producer envelope from the #1064 salvage branch into core (basic_memory/hooks/): a frozen Envelope dataclass with the 2026-07-15 revision fields (UUIDv7 id, actor, caused_by, promotion_status), metadata-only idempotency keying, and the provenance observation/frontmatter projections. The factory redacts every payload, so nothing built through it can carry unredacted values into the inbox. The Stage-1 redaction floor combines detect-secrets scanning (known token formats plus an entropy threshold, with the eager-scan entropy limit re-applied) with the salvaged recursive deny-key / deny-path / env-pair / truncation rules, separator-normalized for Windows and hardened to be pure and idempotent. detect-secrets lands as a core dependency: its tree is light (pyyaml already ships; requests is the only addition) and the floor must be unconditionally present on the capture hot path — an optional extra would make redaction availability configuration-dependent. Includes a ~15-line RFC 9562 UUIDv7 generator; stdlib uuid.uuid7() exists only on Python 3.14+, so swap when the floor rises. Part of issue #997 (SPEC-55 revision 2026-07-15). Co-authored-by: sourrrish <officialsourish0@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
The inbox is the SPEC-55 append-only WAL: one <uuid7>.json file per envelope under <bm-home>/inbox/, written atomically (tmp + rename) so a crash can never leave a half-written envelope visible. Filename order is capture order, so consumers need no mtime/stat dependence. BM home resolves through core's resolve_data_dir() — plugin directories are ephemeral and uninstalling a plugin must never delete captured trace. Processed envelopes move to processed/ for audit and are pruned after a retention window, with age derived from the uuid7 timestamp in the filename; files that don't parse as UUIDs are never deleted. Adapters normalize each harness's hook stdin dialect (Claude Code per the platform hooks reference and the shipped hook scripts; Codex per its hook scripts) into one NormalizedHookEvent so everything downstream is harness-agnostic. Realistic payloads are recorded as test fixtures. Part of issue #997 (SPEC-55 revision 2026-07-15). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
bm hook flush's engine: sweep the whole inbox in filename order, group by (source, source_session_id), and project SessionNote skeletons plus ToolLedger entries through the same internal write path the CLI's write-note uses (the MCP write_note tool via the async client — no subprocess). Idempotent by construction: envelopes are hints, deduped on idempotency_key within a sweep and against the processed audit trail, and artifacts are re-derived with deterministic titles and overwrite=True — WAL replays and duplicate hooks can never double-write. Artifacts carry created_by and caused_by_event frontmatter plus the required [source] observation. Envelopes without a resolvable project mapping stay pending (fail fast, never write to the wrong project), as do groups whose write fails — both self-heal on the next sweep. Corrupt envelope files are counted and left in place, never deleted or guessed at. Part of issue #997 (SPEC-55 revision 2026-07-15). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
The harness producer front door: plugins reduce to shims that exec 'bm hook <event> --harness claude|codex' with hook JSON on stdin. session-start and pre-compact port the existing plugin hook behavior into core — the session context brief (structured task/decision/session queries, shared-project read set, placement guidance, nudges) printed to stdout under the 10k char cap with graph-derived content fenced and labeled as reference data, not instructions; and the extractive pre-compaction checkpoint note written through the same internal path as bm tool write-note. Settings resolution is ported from the hook scripts: the basicMemory block of the nearest-ancestor .claude settings files over the user-level settings.json for Claude, and .codex/basic-memory.json for Codex; --project-dir overrides the payload cwd for project mapping. When captureEvents is the JSON boolean true (fail-closed: strings never enable), both verbs also capture a floor-redacted envelope into the inbox. Harness verbs are fail-open: any error logs to stderr and exits 0, never disrupting a session. flush runs the projector; status is the debuggability surface (inbox depth, last flush, settings summary, bm/uv versions). Part of issue #997 (SPEC-55 revision 2026-07-15). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
The plugin hook scripts are now zero-logic shims: resolve the Basic Memory
CLI (BM_BIN override -> basic-memory/bm on PATH -> uvx at a released floor)
and exec 'bm hook <event> --harness claude|codex' with the hook JSON passed
through on stdin. Nothing resolvable exits 0 silently (fail-open). The
Claude shims pass ${CLAUDE_PROJECT_DIR} as --project-dir so project mapping
never trusts cwd; Codex mapping keeps using the payload cwd.
The Codex standalone Python hook implementations are deleted — their logic
lives in basic_memory/hooks/ and the hook CLI since the core phase — and the
Codex validator drops the .py script requirements accordingly. hooks.json
manifests are unchanged (${CLAUDE_PLUGIN_ROOT}/${PLUGIN_ROOT} commands,
matchers, and timeouts preserved); both plugin layouts already conform
(.claude-plugin/ and .codex-plugin/ hold manifests only, components at root).
The shell-hook tests are rewritten as a shim e2e suite covering both
plugins: fake recording CLIs assert resolver order, stdin passthrough, the
uvx version floor, BM_BIN override semantics (spaced paths and multi-token
launchers), project-dir plumbing, and silent fail-open — keeping the Git
Bash Windows resolution pattern.
Part of #997 (SPEC-55).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
'bm hook install' wires the lifecycle hooks into the user-level harness config for standalone (non-marketplace) users: the Claude Code settings hooks block (~/.claude/settings.json) or a Codex hooks.json (~/.codex/hooks.json) using the plugin's hooks.json schema and matchers. Entries are ownership-tagged through their command shape (the codex-honcho ownership-regex approach): 'bm hook remove' deletes exactly the entries matching OWNED_HOOK_COMMAND_RE — surgical against pre-existing user hooks, including mixed groups — and both verbs are idempotent. Malformed configs fail fast instead of being clobbered. install also checks for uv and prints a per-platform install hint when missing (warning only: a PATH-installed basic-memory works without uv). Also carries the Codex 'focus' config key into the unified session brief header — the one codex-script feature the core-phase unification dropped. Tests cover merge/remove against fixture configs with pre-existing user hooks, idempotency both directions, fail-fast on malformed configs, the per-platform uv hint, and the focus header. Part of #997 (SPEC-55). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
scripts/update_versions.py now rewrites the 'basic-memory>=FLOOR' pin in all four plugin hook shims alongside the manifests it already touches (packages scope), so every release moves the uvx fallback to a version that actually ships the 'bm hook' verbs. The floor keeps the Python version form (e.g. 0.21.3b1) — it is a pip requirement spec, not the npm semver mapping. Regression tests: packages scope bumps all four shims, core scope leaves them untouched, and prereleases land in pip form. Part of #997 (SPEC-55). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
… opt-in Both plugin READMEs now list uv as the documented per-platform prerequisite, disclose that the hook shims fall back to 'uvx basic-memory>=FLOOR' (network fetch from PyPI on first run, floor bumped by release tooling), and document the captureEvents gate as opt-in and off by default (JSON boolean true only). settings.example.json and the Codex config example gain 'captureEvents': false. CHANGELOG gains the Unreleased entry for the bm hook front door, and the plugin CHANGELOG notes the shim reshape. Part of #997 (SPEC-55). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Five findings from the multi-lens review of the branch survived adversarial verification and are fixed here: - projector: rebuild session artifacts from the complete session (processed + fresh envelopes) so incremental flushes no longer erase events projected on an earlier sweep (was overwriting from still-pending envelopes only). - projector: use the full session id in artifact titles; an 8-char prefix let distinct sessions collide on the same permalink and clobber each other. - inbox: mark_processed tolerates an envelope already retired by a concurrent sweep instead of crashing the whole flush with FileNotFoundError. - inbox/_uuid7: uuid7_unix_ms fails fast on non-v7 UUIDs so retention never derives a garbage capture time from a v1/v4 name and deletes it. - hook: pre-compact checkpoint transcript excerpts now pass the Stage-1 redaction floor (new redaction.redact_text) before entering the note title, summary, and observations — closing the #997 'redact before writing' gap. Regression tests added for each finding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
![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: 63bc6c837d
ℹ️ 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".
…ows shim Static Checks + Windows unit failures and the two Codex P2 findings: - redaction: deny-path redaction now replaces the matching path token wherever it appears, not only when the whole value starts with the prefix. Free-text checkpoint excerpts such as "please read ~/.ssh/id_rsa" had the path left intact by redact_text(); the prefixes now compile to substring matchers that span either separator and consume the path tail. Whole-value paths still collapse to the marker, so existing behavior is preserved. - shims: PATH-resolved basic-memory/bm are probed for `hook` support (`hook --help`, stdin detached) before they win over the uvx floor. A stale pre-hook install on PATH no longer shadows the floor and exec a CLI that errors — it falls back, keeping the fail-open contract. Applied to all four Claude/Codex shims. - shims: the BM_BIN single-word test uses `-e` (exists) instead of `-x` (executable). Git Bash reports extensionless files as non-executable, so a spaced BM_BIN path was word-split on Windows, breaking test_bm_bin_executable_path_with_spaces_stays_one_word. - test: narrow mock_write.await_args before .kwargs so `ty` stops flagging attribute access on `_Call | None` (matches the sibling assertions already in the file). Regression tests added for embedded-path redaction (free text and payload values) and the stale-install fallback (falls back to uvx; stays silent when nothing else is resolvable). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review Addressed both P2 findings plus the Static Checks (ty) and Windows unit failures in 7fd1d03:
All added regression tests, plus |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fd1d03098
ℹ️ 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".
Two follow-up P2 findings from the re-review of 7fd1d03: - redaction: the deny-path floor only carried the *expanded* home paths (/home/.../.ssh/), so a literal "~/.ssh/id_rsa" — the form prose and transcript excerpts actually type — survived redact_text(). The default list now denies both the expanded and the literal "~/" prefixes, deduped. - shims: the trailing `exec` tail-exec'd a resolved launcher's non-zero exit to the harness. A cold uvx that cannot reach PyPI, an unbuildable floor, or a bad BM_BIN would then disrupt the session — violating the "advisory, must NEVER disrupt" contract in the shim header. All four shims now run the command instead of exec-ing it and always exit 0; stdout still reaches the session context and stdin still passes through. Regression tests: unexpanded-tilde redaction (free text + payload) and fail-open on a launcher that errors at runtime (all four shims). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review Round-2 findings addressed in aacb9ab:
Regression tests added for both. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aacb9ab643
ℹ️ 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".
Two more P2 findings from the re-review of aacb9ab — the resolved hook cwd bypassed the redaction floor: - envelope: create_envelope only scrubbed `payload`; the top-level `cwd` was stored raw in the inbox WAL. A session under a configured redactPaths (or a default deny dir) now has its cwd run through redact_text before the Envelope is built. project_hint is a project name, not a path, and the projector resolves against it, so it is intentionally left intact. - checkpoint: the pre-compact note wrote `event.cwd` verbatim into the frontmatter `cwd:` field and the "Working in ..." body line. It is now redacted once at extraction (alongside the transcript excerpts) so both draw from the scrubbed string. The functional uses of cwd (git status, project mapping) run earlier on the real path and are unaffected. Regression tests: envelope cwd redaction (denied + ordinary) and checkpoint cwd redaction under a configured redactPaths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review Round-3 findings addressed in 709a622 — the resolved hook
Regression tests added; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 709a62237e
ℹ️ 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".
…(PR #1070 round-4) Two P2 findings from the re-review of 709a622: - projector: an in-group idempotency replay was retired to processed/ before the fresh envelope it duplicated was written. When that write failed, the next sweep read the replay's key from processed/ and retired the still-pending fresh envelope as a replay too — the session never projected, breaking the documented self-heal. Replays are now split: a replay of an already-projected envelope is retired immediately (its original is durable), but an in-group replay is retired only after its fresh sibling's write succeeds, so both self-heal together on a failed sweep. - inbox: a session that never resolves a project mapping (primaryProject unset for its whole lifetime) produced envelopes the projector could never route and held pending forever — flush routes solely on the stored project_hint and never reloads settings. Retention now prunes pending envelopes on the same window it already applies to processed/, so unresolvable trace can't accumulate without limit. Capture-without-mapping is unchanged (a later same-session hinted capture still resolves the whole group via the merge); invalid entries are preserved so retention never eats the corruption trace `bm hook status` surfaces. Regression tests: reproject-after-write-failure with an in-group replay; prune of expired unresolved pending envelopes; recent unmapped pending kept. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review Round-4 findings addressed in e908d94:
Regression tests added; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e908d94366
ℹ️ 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".
round-5) Follow-on from the round-4 replay fix. A same-minute replay retired into processed/ lives next to its original (same idempotency key, distinct uuid7 id). When a later distinct event triggers a full rebuild, the reconstruction merged all processed envelopes back into the artifact without re-deduping — so the SessionNote/ToolLedger rendered duplicate rows for the replayed event, violating the idempotency contract. The merged (prior + fresh) list is now deduped by idempotency key before building the notes, keeping the earliest (chronological uuid7) envelope per key — the original, not its replay. Regression test: test_flush_dedups_retired_replay_history_on_rebuild. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review Round-5 finding addressed in c04e0c8: the projector now dedups the merged prior+fresh envelope list by idempotency key before rebuilding, so a replay retired into processed/ next to its original no longer produces duplicate rows in the SessionNote/ToolLedger. Regression test added; ruff/ty/format clean and 159 hook tests green locally. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c04e0c869a
ℹ️ 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".
round-6) Follow-on from the round-4/5 replay handling. When two same-minute captures of the same event share an idempotency key and the mapping was set only for the later one (primaryProject configured between the two hooks), the first envelope is fresh with an empty project_hint and the second lands in group_replays. The routing scan ran over the deduped fresh+prior list only, so it missed the replay's hint, left the whole group pending, and retention could eventually prune trace that actually had a valid mapping. Rendered rows still dedup (one per key), but routing now scans the complete group including the in-group replays, so a hint carried only by a replay still routes the group. Regression test: test_flush_routes_on_project_hint_from_in_group_replay. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review Round-6 finding addressed in 0a39f5f: the projector's project-hint routing now scans the complete group (fresh + prior + in-group replays), so a mapping carried only by a same-minute replay routes the group rather than leaving it pending. Rendered event rows still dedup by key. Regression test added; ruff/ty/format clean, 160 hook tests green locally. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a39f5fd06
ℹ️ 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".
…files (PR #1070 round-7) Two P2 findings from the re-review of 0a39f5f: - install: `bm hook install` always wrote a bare `basic-memory hook ...` command. A standalone user with uv but no persistent basic-memory/bm on PATH (installed via `uvx basic-memory hook install`) would then hit command-not-found at SessionStart/PreCompact. Install now resolves the launcher like the shims do — basic-memory / bm on PATH, else a `uvx "basic-memory>=<floor>"` fallback pinned to the running release floor — and writes that. The ownership regex now keys on the `hook <verb> --harness <harness>` suffix so `remove` still recognizes every launcher form (including the uvx one). With nothing resolvable it still writes the basic-memory form and warns about the missing uv (unchanged). - flush: the inbox load loop only caught JSON/shape errors, so a pending file vanishing between list_envelopes() and read_text() (overlapping flushes) or a transiently unreadable file raised OSError and aborted the whole sweep — skipping valid envelopes and the flush marker. OSError is now caught and the entry skipped (not counted invalid, since it isn't corrupt trace). Regression tests: install resolves uvx / bm launchers and remove strips the uvx form; flush skips a vanished inbox file and still projects + records the marker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review Round-7 findings addressed in 3d6e78a:
Regression tests added; ruff/ty/format clean, 163 hook tests green locally. |
…1070) The projector hand-built the artifact frontmatter as a `---` text block. One of its fields, `envelope_turn_id`, comes from `source_turn_id`, which SPEC-55 defines as opaque, surface-defined text. A value with YAML-special content — `turn: 42`, a stray colon, a newline — produced an invalid frontmatter block, so write_note failed to persist the artifact and the whole session stayed pending on every flush retry (the envelope shape is otherwise valid, so it never ages out as invalid either). This is the same failure the pre-compact checkpoint hit and fixed in round-21: build a `metadata` dict and pass it through `write_note(metadata=...)`, which serializes/quotes values safely, instead of splicing raw text into a hand-built block. `_artifact_metadata` now returns that dict (`created_by`, `caused_by_event`, plus `to_frontmatter_fields`, and `status: open` for the session note); `_session_note`/`_tool_ledger_note` return `(title, body, metadata)`; `_write_artifact` forwards it. The note `type` still rides the `note_type` arg. Body content no longer carries a frontmatter block. Regression test: an envelope whose `source_turn_id` contains `turn: 42\n...` flushes successfully, with the raw value reaching write_note as a structured metadata value rather than spliced into the body. Reported by Codex on PR #1070 (projector.py:140). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64b7f2e4fb
ℹ️ 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".
…(PR #1070) Two overlapping `bm hook flush` runs could race: flush B lists {B, C}, writes the full artifact, and retires C to processed/; a stale flush A (snapshot {B}) then overwrites the SessionNote/ToolLedger from {A, B} with overwrite=True, dropping C's row. Because C is already retired, a later sweep with no fresh pending in that group never rebuilds the artifact, so C stays missing until another event arrives in the session (the envelope trace itself is safe in processed/ — it's the rendered artifact that loses the row). flush() now runs under an exclusive advisory lock over the inbox (`inbox.flush_lock()`, via filelock — fcntl on POSIX, msvcrt on Windows). A flush that arrives while the lock is held skips rather than blocks: the running flush sweeps the whole inbox, so the skipped run loses no work, and skipping avoids exactly the stale overwrite. The lock releases on process death, so a crashed flush strands nothing. The flush body moved to _flush_locked; a new FlushResult.skipped flag is surfaced by `bm hook flush`. filelock was already resolved transitively (via huggingface-hub); declared as a direct dependency now that core imports it. Regression test: with the inbox lock held, flush() skips — writes nothing, leaves the envelope pending for the next unlocked sweep. Reported by Codex on PR #1070 (projector.py:360). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14b07c7d6f
ℹ️ 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".
… (PR #1070) When a Codex checkpoint runs from a directory covered by `redactPaths`, the cwd is redacted — but the "## Working tree" section rendered raw `git status --short` rows, which are repo-relative filenames (`M customer-roadmap.md`) with no absolute prefix. Per-row `redact_text` can't match the deny path against a bare relative filename, so the denied workspace's file list leaked into the checkpoint, defeating the user's redaction intent. Skip the working-tree section entirely when `safe_cwd == REDACTED_PATH` (the cwd matched a redactPaths entry, so the whole workspace is denied and every row is sensitive). Rows are still per-row redacted when the workspace is not denied, to catch a secret in a filename or an absolute denied path appearing in a row. Only the Codex path renders this section (include_workspace_sections). Regression test: a codex pre-compact whose cwd is under redactPaths omits the working-tree section and the filenames, while the cwd stays redacted in frontmatter. Reported by Codex on PR #1070 (hook.py:729). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d651ba2e8b
ℹ️ 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".
…e (PR #1070) _write_hook_config used a bare write_text over ~/.claude/settings.json / .codex/hooks.json — the user's entire harness config, not just our entries — so a crash mid-write could truncate it to a partial JSON document. Write tmp + os.replace instead, mirroring the inbox WAL pattern, and pin the crash case with a regression test that fails the tmp write and asserts the original file survives intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
…(PR #1070) Marketplace plugin installs are git clones, and Git for Windows defaults to core.autocrlf=true: without an eol pin the hook shims check out with CRLF endings and bash exits 2 with syntax errors on every session start, the one exit code Claude Code surfaces to the user — breaking the fail-open contract for the whole Windows audience. All tracked .sh files are already LF in the index, so this only pins checkout behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cd43b7210
ℹ️ 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".
#1070) The tmp + os.replace rewrite introduced in 5d61444 published the tmp file's umask-default mode, silently widening a deliberately private (0600) settings file to 0644. Create the tmp at the original's mode via os.open (O_CREAT's mode is umask-masked, so the tmp is never observable wider than the original), then chmod to the exact mode before replace. A missing target keeps umask defaults, matching a plain write. Pinned by a POSIX-only regression test; the crash-safety test now injects its failure through the os.fdopen handle the rewrite actually uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0bc0abfa6
ℹ️ 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".
) O_CREAT's mode argument applies only at creation, so a stale tmp left by a crashed earlier run kept its old (possibly world-readable) mode and private config content was written into it before the exact-mode chmod. Unlink any stale tmp and open with O_EXCL so the tmp is always freshly created at the intended mode. Pinned by a regression test that seeds a 0644 stale tmp beside a 0600 settings file and asserts the fd's mode is 0600 at open time, before any content is written. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3db833e451
ℹ️ 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".
…s (PR #1070) Identity fields (source_session_id, source_turn_id) are opaque surface-defined strings that skip the payload redaction floor, so a newline-carrying id — hostile hook stdin or a corrupt replayed inbox file — became extra parsed observation/relation lines in the projected SessionNote body, the same injection class the frontmatter path already blocks via metadata= serialization. Collapse control characters in every interpolated field at emission, which covers replayed envelopes that never pass through the capture factory. Pinned by a regression test injecting observation and relation lines through both ids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 123a44fc0e
ℹ️ 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".
The four shell shims were 68 lines each of resolver edge cases — Git Bash -x lies, word-splitting, quote-stripping, stdin-detached probes, CRLF fragility — to solve a problem uv already solves. Each hook is now a self-contained PEP 723 script run via uv run --quiet --script: the inline metadata pins basic-memory>=<floor>, uv resolves and caches the environment, and the script invokes the Typer hook command in-process. BM_BIN (a path or quoted launcher string) remains the development override. Fail-open is a single guard: every failure path exits 0. This deletes the resolver logic outright — no PATH probing, no stale-binary hook --help probe, no uvx fallback ladder — and makes the hooks native on Windows (no bash, no CRLF exposure). uv becomes a hard prerequisite, already documented for the former fallback path. Each script gets a co-located test file in the same directory (pytest testpaths now includes plugins/) executing the real launcher via sys.executable: BM_BIN argv/stdin passthrough, launcher quoting, project-dir plumbing (claude) or its absence (codex), fail-open on an unresolvable BM_BIN, the in-process CLI path against an empty HOME, and the PEP 723 floor pinned to __version__. Release tooling bumps the floor on the anchored dependencies line (HOOK_SCRIPTS in update_versions.py), which also retires the two-match regex hazard flagged in review; the updater tests seed the realistic script shape. Both plugin validators now require the PEP 723 floor. Central plugin tests pin the hooks.json uv run wiring, shebangs, exec bits, and that no .sh shims reappear. Docs and unreleased changelog entries updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
…#1070) The provenance-observation fix (123a44f) collapsed control characters in to_provenance_observations but missed the sibling emission sites: note titles, SessionNote Events lines, ToolLedger Entries lines, and the ledger's [source] observation all interpolate the raw session id or replayed envelope fields, so a newline-carrying value could still parse as injected observations/relations. The sanitizer is now exported as single_line and applied to every dynamic line both note builders emit. Regression test drives a newline-injecting session id through flush and asserts neither artifact carries the injected lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7dd1972df
ℹ️ 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".
Member
Author
|
@codex review |
This was referenced Jul 20, 2026
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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
1 participant
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
Implements the harness capture producer front door (SPEC-55 revision 2026-07-15, the plan posted on #997). Coding-agent lifecycle hooks are captured as normalized, redacted producer envelopes in a local WAL and projected into knowledge-graph artifacts out of band — with the plugins reduced to configuration and all logic living in the
basic-memorypackage where it is tested, typed, and released.Supersedes #998 and #1064: the envelope design from those PRs carries forward into
basic_memory/hooks/with commit credit to @sourrrish. The prior attempts embedded Python in shell heredocs and vendored a shared module that never survived packaging; this replaces both with a CLI front door.Architecture
bm hook <event> --harness <name>with stdin passthrough. bm is resolved installed-first (BM_BIN→basic-memory/bmon PATH), falling back touvx "basic-memory>=<floor>"so no local install is required; uv is the documented per-platform prerequisite and the floor is bumped by the release version updater.bm hookCLI (basic_memory/hooks/+cli/commands/hook.py): per-harness stdin adapters → envelope → Stage-1 redaction floor → atomic WAL inbox. Verbs:session-start,pre-compact,flush,status,install,remove. Execution is fail-open (a hook never disrupts a session); thecaptureEventsgate is fail-closed (must be booleantrue).<bm-home>/inbox/<uuid7>.json(atomic tmp+rename, one file per envelope). UUIDv7 ids sort chronologically, so the projector processes in capture order with no mtime dependence, and ids double as thecaused_bytarget. Lives in BM home by requirement — plugin dirs are ephemeral and deleted on uninstall.bm hook flush): sweeps the whole inbox, groups by session, dedups on idempotency key (envelopes are hints, never double-written), and projects SessionNote + ToolLedger artifacts carryingcreated_by+caused_by_eventprovenance. No LLM — the SPEC-54 daemon worker supersedes it later without reworking the inbox contract.Review
Built and reviewed via a multi-agent workflow: five review lenses (correctness, security/privacy, portability, platform conformance, style/tests) over the branch diff, each finding adversarially verified. Five confirmed findings were fixed with regression tests (final commit):
mark_processedcrashed the sweep on a double-retire — now tolerated.uuid7_unix_msnow fails fast.Test plan
uv run pytest tests/hooks tests/cli/test_hook_command.py— 146 passed (unit coverage of uuid7, envelope, redaction floor +redact_text, inbox atomicity/ordering/retention/concurrency, adapters against recorded fixtures, projector dedup/replay-safety/incremental-flush/mapping-gate, and the CLI verbs incl. captureEvents fail-closed and checkpoint redaction).uv run ruff check+ruff format— clean.uv run ty check src/basic_memory/hooks src/basic_memory/cli/commands/hook.py— clean.just package-check— passes (Claudeplugin validate --strict, Codex layout, Hermes, OpenClaw, skills).Fixes #997
Closes #998
🤖 Generated with Claude Code