diff --git a/plugins/codex/README.md b/plugins/codex/README.md index 06fc21400..0376d6dd7 100644 --- a/plugins/codex/README.md +++ b/plugins/codex/README.md @@ -11,8 +11,8 @@ verification, decision capture, and resumable checkpoints. - **Orient from memory.** The `bm-orient` skill reads active tasks, open decisions, and recent Codex checkpoints before substantial work. -- **Checkpoint work.** `PreCompact` records a private request and the `Stop` - hook asks the active Codex turn to run `bm-checkpoint` once after compaction. +- **Checkpoint work after compaction.** The post-compaction `SessionStart` + context asks the resumed Codex turn to run `bm-checkpoint`. The resulting `codex_session` or `coding_session` note is agent-authored from the compacted working context, with repository and pull-request evidence. - **Capture decisions.** The `bm-decide` skill records durable engineering @@ -33,19 +33,24 @@ verification, decision capture, and resumable checkpoints. | --- | --- | | `.codex-plugin/plugin.json` | Codex plugin manifest | | `.mcp.json` | Basic Memory MCP server configuration | -| `hooks/hooks.json` | SessionStart, PreCompact, and Stop hook registration | +| `hooks/hooks.json` | SessionStart, PreCompact, and rollout-compatibility Stop registration | | `hooks/session_start.py` | uv script: runs `basic-memory hook session-start --harness codex` | | `hooks/pre_compact.py` | uv script: runs `basic-memory hook pre-compact --harness codex` | -| `hooks/stop.py` | uv script: runs `basic-memory hook stop --harness codex` | +| `hooks/stop.py` | uv script: preserves the previous checkpoint handshake during the runtime-pin rollout | | `skills/` | Codex-native Basic Memory workflows | | `schemas/` | Seed schemas for Codex sessions, decisions, and tasks | -The hook scripts carry no logic: the brief, checkpoint coordination, and +The hook scripts carry no logic: the brief, checkpoint prompting, and lifecycle-event capture all live in the pinned Basic Memory revision behind `bm hook`. Each is a self-contained PEP 723 script pinned to a Basic Memory Git ref. All refs are updated together with `just set-codex-hook-version `. +The Stop shim is a temporary rollout bridge. While these scripts remain pinned +to the last durable merged runtime, it preserves that runtime's checkpoint +handshake. The dependency-only follow-up that pins the post-compaction +`SessionStart` implementation also removes the Stop registration. + ## Requirements - **[uv](https://docs.astral.sh/uv/)** — required: the hooks are PEP 723 @@ -81,8 +86,7 @@ be moved into the plugin directory. Configuration can live at user level in `~/.codex/basic-memory.json` or at project level in `.codex/basic-memory.json`. User-level settings are the base; -the nearest project file overrides only the keys it declares. `redactKeys` and -`redactPaths` are the privacy exception: their user and project lists accumulate. +the nearest project file overrides only the keys it declares. The setup skill asks which scope to use and recommends user-level configuration by default. @@ -101,32 +105,37 @@ Run the setup skill, or create `~/.codex/basic-memory.json` for shared defaults: "secondaryProjects": [], "teamProjects": {}, "focus": "code/dev", - "rememberFolder": "codex-remember", + "rememberFolder": "codex/remember", "recallTimeframe": "7d", + "checkpointOnCompact": true, "captureEvents": true, - "redactKeys": [], - "redactPaths": [], - "placementConventions": "Put decisions in decisions/ and work checkpoints in codex//." + "placementConventions": "Put decisions in codex/decisions/ and work checkpoints in codex//." } } ``` Codex event capture is on by default. Set the JSON boolean `false` at user or -project level to opt out; malformed values fail closed. Captured, redacted +project level to opt out; malformed values fail closed. Captured lifecycle-event envelopes land in a local inbox under your Basic Memory home. The lifecycle trace stays local: `basic-memory hook flush` only moves valid -envelopes into the local retention archive and never creates graph notes. Add -`redactKeys` and `redactPaths` arrays to extend the built-in redaction floor. +envelopes into the local retention archive and never creates graph notes. + +Checkpoint prompting is on by default. Set `checkpointOnCompact` to the JSON +boolean `false` to opt out. Codex ignores PreCompact stdout; after compaction, +Codex runs SessionStart with the `compact` trigger. When the setting is enabled, +that context asks the resumed agent to run `bm-checkpoint` from its compacted +working context. -Codex ignores PreCompact stdout, so PreCompact cannot ask the model to write a -note directly. It leaves a private request for the Stop hook. Stop then blocks -the turn once with a request to run `bm-checkpoint`; the active model writes an -agent-authored checkpoint from its compacted context, and the next Stop is a -no-op to prevent loops. +`sessionProfile` only selects whether the skill writes a `codex_session` or +`coding_session` note. When `captureFolder` is omitted, Codex resolves the Git top-level directory and writes to `codex/`. An explicit folder still wins. +Decision notes default to `codex/decisions`, and lightweight `bm-remember` +captures default to `codex/remember`. Project placement conventions and an +explicit `rememberFolder` can override those destinations. + For a coding profile, keep both the profile and checkout-specific repository identifier in the project file without duplicating the shared settings: diff --git a/plugins/codex/skills/bm-checkpoint/SKILL.md b/plugins/codex/skills/bm-checkpoint/SKILL.md index 2e5f49517..4010dc312 100644 --- a/plugins/codex/skills/bm-checkpoint/SKILL.md +++ b/plugins/codex/skills/bm-checkpoint/SKILL.md @@ -7,22 +7,18 @@ description: Save a deliberate Codex work checkpoint to Basic Memory with change Create a durable handoff note for current Codex work. Use this when the user asks to checkpoint, wrap up, hand off, remember the state of the work, or when the -Stop hook requests the deliberate handoff after compaction. +post-compaction SessionStart context requests the deliberate handoff. ## Gather Read `~/.codex/basic-memory.json`, then the nearest project -`.codex/basic-memory.json`; project keys override user keys except that -`redactKeys` and `redactPaths` accumulate so project config cannot weaken user -privacy: +`.codex/basic-memory.json`; project keys override user keys: - `primaryProject`, default omitted - `captureFolder`, default `codex/` - `placementConventions`, optional - `sessionProfile`, default `general` - `repository`, required when `sessionProfile` is `coding` -- `redactKeys`, optional additional secret field names -- `redactPaths`, optional additional private directory prefixes Apply the `bm-writing` skill before drafting the note. @@ -47,31 +43,6 @@ Gather repo evidence: Do not claim a test passed unless you ran it or the user supplied the result. -## Privacy Gate - -Apply this gate to deliberate checkpoints and Stop-requested checkpoints alike. -It is mandatory before any `write_note` call: - -1. Merge user and project `redactKeys` and `redactPaths` by accumulation. Include - the built-in secret-key families (`secret`, `token`, `password`, `credential`, - `auth`, API/access/private keys) and private roots (`~/.ssh`, `~/.aws`, and - `~/.gnupg`). Never copy the redaction rules themselves into the note. -2. Expand `~` for comparison and normalize both slash styles. A denied path - matches its exact directory or a descendant, not a sibling that merely shares - its text prefix. -3. Scrub **every string** passed to `write_note`: title, directory, tags, - frontmatter, body, observations, relations, changed-file paths, command - output, repository evidence, and pull-request evidence. Replace a whole path - value under a denied root, or an embedded denied-path token, with - `[REDACTED_PATH]`. Do not omit schema-required path fields; use the marker. -4. When gathered structured evidence has a key matching a built-in or configured - `redactKeys` entry, replace its entire value with `[REDACTED]`. Never reproduce - credentials, tokens, or private-key material from config, environment, tool - output, or compacted conversation context. -5. Review the final note arguments once more before writing. If any value cannot - be confidently scrubbed, fail closed: skip the checkpoint and report that - privacy policy blocked the write. Do not fall back to an unredacted note. - ## Write A checkpoint is a durable handoff, not a status dump or commit-by-commit diff --git a/plugins/codex/skills/bm-decide/SKILL.md b/plugins/codex/skills/bm-decide/SKILL.md index 5922b8cc9..8870c02ad 100644 --- a/plugins/codex/skills/bm-decide/SKILL.md +++ b/plugins/codex/skills/bm-decide/SKILL.md @@ -14,7 +14,7 @@ choice with rationale and consequences, not a casual preference. `.codex/basic-memory.json`; project keys override user keys: - write to `primaryProject` when set - follow `placementConventions` for the directory when they are specific - - otherwise use `decisions` + - otherwise use `codex/decisions` Apply the `bm-writing` skill before drafting the note. diff --git a/plugins/codex/skills/bm-remember/SKILL.md b/plugins/codex/skills/bm-remember/SKILL.md index 0f31be422..a3ead9de0 100644 --- a/plugins/codex/skills/bm-remember/SKILL.md +++ b/plugins/codex/skills/bm-remember/SKILL.md @@ -13,7 +13,7 @@ a small fact that should survive the current thread. 1. Read `~/.codex/basic-memory.json`, then the nearest project `.codex/basic-memory.json`; project keys override user keys: - `primaryProject`, default omitted - - `rememberFolder`, default `codex-remember` + - `rememberFolder`, default `codex/remember` Apply the `bm-writing` skill before drafting the note. Match its depth to this lightweight capture; do not pad a small fact into an essay. diff --git a/plugins/codex/skills/bm-setup/SKILL.md b/plugins/codex/skills/bm-setup/SKILL.md index 039efc98a..9c33aa481 100644 --- a/plugins/codex/skills/bm-setup/SKILL.md +++ b/plugins/codex/skills/bm-setup/SKILL.md @@ -27,8 +27,7 @@ repo, default project, current directory, or previous local state. - config level: user-level `~/.codex/basic-memory.json` or project-level `.codex/basic-memory.json`. Ask explicitly and recommend user level by default. - Project settings override user settings key by key, except `redactKeys` and - `redactPaths`, which accumulate so project config cannot weaken user privacy. + Project settings override user settings key by key. - storage mode: cloud, local, or mixed. Prefer the user's stated mode over any CLI default. - `focus`: code/dev, research, writing, planning, or mixed. @@ -41,14 +40,14 @@ repo, default project, current directory, or previous local state. - `teamProjects`: optional share targets for `bm-share`. - `captureFolder`: default `codex/`, derived from the Git top-level directory. Ask only when the user wants an explicit override. -- `rememberFolder`: default `codex-remember`. +- `rememberFolder`: default `codex/remember`. - `placementConventions`: a short note about where decisions, tasks, and research - notes should land. -- `captureEvents`: whether to record redacted lifecycle-event envelopes in the + notes should land. Default decisions to `codex/decisions` so Codex-authored + memory stays under one tree. +- `checkpointOnCompact`: whether post-compaction SessionStart asks Codex to run + `bm-checkpoint`. Default to `true`; an explicit JSON boolean `false` opts out. +- `captureEvents`: whether to record lifecycle-event envelopes in the local hook inbox. Default to `true`; an explicit JSON boolean `false` opts out. -- `redactKeys` and `redactPaths`: optional additions to the built-in redaction - floor. Ask for these only when event capture is enabled or the user has - repo-specific privacy requirements. For the `coding` session profile, verify the current directory is inside a Git repository. Resolve a stable `repository` identifier such as `owner/name` from @@ -57,8 +56,8 @@ confirmation. Do not guess when the remote is missing or ambiguous. Explain that coding checkpoints store structured repository, branch, SHA, working-directory, and optional pull-request metadata in Basic Memory. -Explain the capture tradeoff before asking: enabled capture adds a local, -redacted event trail that stays queued until `bm hook flush` archives it locally. +Explain the capture tradeoff before asking: enabled capture adds a local event +trail that stays queued until `bm hook flush` archives it locally. It never creates knowledge-graph notes or writes to team projects. The default is enabled; an explicit JSON boolean `false` disables it, and malformed values fail closed. @@ -84,12 +83,11 @@ project-level file: "teamProjects": {}, "focus": "", "sessionProfile": "", - "rememberFolder": "codex-remember", + "rememberFolder": "codex/remember", "recallTimeframe": "7d", + "checkpointOnCompact": true, "captureEvents": true, - "redactKeys": [], - "redactPaths": [], - "placementConventions": "" + "placementConventions": "Put decisions in codex/decisions/ and work checkpoints in codex//." } } ``` @@ -97,12 +95,8 @@ project-level file: Omit `captureFolder` to use `codex/`; persist it only for an explicit override. Preserve unrelated keys if the chosen file already exists. Include `projectMode` when the user chose cloud, local, or mixed routing. Always persist -`captureEvents` as a JSON boolean. Empty `redactKeys` and `redactPaths` lists may -be omitted; when present, they must be JSON arrays of strings. `redactKeys` -extends payload-key redaction, while `redactPaths` also protects -working-directory and path-bearing checkpoint content. User and project -redaction lists accumulate. These files are intentionally Codex-specific; do -not write `.claude/settings.json`. +`checkpointOnCompact` and `captureEvents` as JSON booleans. These files are +intentionally Codex-specific; do not write `.claude/settings.json`. For a user-level coding setup, omit `sessionProfile` from the shared user file and keep both the coding profile and confirmed repository identifier in the project @@ -164,10 +158,11 @@ Before closing, prove the mapping works: - Run `basic-memory hook status --harness codex --project-dir ` (using `bm` or `uvx basic-memory` if needed). Confirm that it finds this repo's settings, reports the selected project, session profile, repository, and - intended capture state. + intended checkpoint-prompt and capture states. Its inbox counts are shared across harnesses. - If any check errors, fix the project ref or hook launcher before finishing. -Finish with the project mapping, schemas seeded or skipped, capture/redaction -choices, shared inbox status, and the verification result. Tell the user that -plugin hooks need to be reviewed and trusted in Codex before they run. +Finish with the project mapping, schemas seeded or skipped, checkpoint prompt, +capture choice, shared inbox status, and the verification result. +Tell the user that plugin hooks need to be reviewed and trusted in Codex before +they run. diff --git a/plugins/codex/skills/bm-status/SKILL.md b/plugins/codex/skills/bm-status/SKILL.md index c3b23cf36..1372cf5d7 100644 --- a/plugins/codex/skills/bm-status/SKILL.md +++ b/plugins/codex/skills/bm-status/SKILL.md @@ -20,20 +20,20 @@ Gather a concise diagnostic. Do not over-investigate. 2. Plugin config: - read `~/.codex/basic-memory.json`, then the nearest project - `.codex/basic-memory.json`; project keys override user keys, while - `redactKeys` and `redactPaths` accumulate + `.codex/basic-memory.json`; project keys override user keys - report the resolved `primaryProject`, `secondaryProjects`, `teamProjects`, `captureFolder`, `rememberFolder`, `recallTimeframe`, `focus`, - `sessionProfile`, `repository`, `captureEvents`, `redactKeys`, and - `redactPaths` + `sessionProfile`, `repository`, `checkpointOnCompact`, and `captureEvents` + - resolve omitted Codex defaults as `rememberFolder=codex/remember`, + and `checkpointOnCompact=true` 3. Core hook health: - with the first available launcher, run `basic-memory hook status --harness codex --project-dir ` - - report the shared inbox path, pending envelopes, archived envelopes, - pending checkpoint requests, last + - report the shared inbox path, pending envelopes, archived envelopes, last flush, settings state, resolved primary project, capture state, capture - folder, Basic Memory version, and uv version from that command + folder, checkpoint-prompt state, Basic Memory version, and uv version from + that command - inbox counts are global across supported harnesses; do not attribute a backlog solely to Codex - treat the command's settings resolution as canonical for hook behavior; if @@ -69,13 +69,11 @@ Basic Memory for Codex - Recall timeframe: - Session profile: - Repository: +- Checkpoint on compact: - Event capture: -- Redact keys: -- Redact paths: - Shared hook inbox: - Shared pending envelopes: - Shared archived envelopes: -- Pending checkpoint requests: - Last flush: - Hook runtime: basic-memory ; uv - Recent checkpoints: diff --git a/scripts/validate_codex_plugin.py b/scripts/validate_codex_plugin.py index 5324bec16..96f9a327d 100755 --- a/scripts/validate_codex_plugin.py +++ b/scripts/validate_codex_plugin.py @@ -25,22 +25,21 @@ ) REQUIRED_SKILL_TEXT: dict[str, tuple[str, ...]] = { "bm-setup": ( + "checkpointOnCompact", "captureEvents", "user-level", "project-level", "codex/", - "redactKeys", - "redactPaths", "sessionProfile", "coding-session.md", "hook status --harness codex", ), "bm-status": ( "~/.codex/basic-memory.json", + "checkpointOnCompact", "hook status --harness codex", "pending envelopes", "archived envelopes", - "Pending checkpoint requests", "last flush", "type=codex_session", "type=coding_session", @@ -53,10 +52,6 @@ "bm-checkpoint": ( "Apply the `bm-writing` skill", "A checkpoint is a durable handoff, not a status dump", - "## Privacy Gate", - "`redactKeys` and `redactPaths` accumulate", - "[REDACTED_PATH]", - "skip the checkpoint", "username: ", "hostname: ", "type: coding_session", @@ -66,6 +61,8 @@ "- relates_to [[Exact existing note title]]", "Never write `[relates_to]`", ), + "bm-decide": ("codex/decisions",), + "bm-remember": ("codex/remember",), "bm-writing": ( "intentionally user-customizable", "problem -> approach -> current state and impact", @@ -204,8 +201,8 @@ def validate_plugin(plugin_dir: Path) -> None: readme = (plugin_dir / "README.md").read_text(encoding="utf-8") for required_text in ( "lifecycle trace stays local", - "Stop hook", - "agent-authored checkpoint", + "post-compaction `SessionStart`", + "note is agent-authored", ): if required_text not in readme: raise SystemExit(f"README.md: missing schema ownership text {required_text!r}") diff --git a/src/basic_memory/cli/commands/hook.py b/src/basic_memory/cli/commands/hook.py index ddaf6e2c0..8ca8f1ce1 100644 --- a/src/basic_memory/cli/commands/hook.py +++ b/src/basic_memory/cli/commands/hook.py @@ -3,12 +3,16 @@ Harness plugins reduce to manifests plus one-line shims that exec ``bm hook --harness claude|codex`` with the hook JSON on stdin. All logic lives here: per-harness stdin adapters, the session-start context brief, -checkpoint coordination, lifecycle-event capture into the inbox WAL, and the +checkpoint prompting, lifecycle-event capture into the inbox WAL, and the flush/status operator surface. Contracts: - - Harness verbs (session-start, pre-compact, stop) are fail-open: any error logs + - Active harness verbs (session-start, pre-compact) are fail-open: any error logs to stderr and exits 0 — a hook must never disrupt an agent session. + - The retired stop verb remains a JSON no-op for upgraded installs whose + existing Codex configuration has not been reinstalled yet. + - Codex checkpoint prompting defaults on. An explicit JSON boolean ``false`` + disables it; malformed values and malformed config fail closed. - Codex event capture defaults on. An explicit JSON boolean ``false`` turns it off, while malformed values and malformed config fail closed. - Graph-derived brief content is fenced and labeled as reference data, not @@ -72,8 +76,9 @@ class Harness(str, Enum): # Cap how many shared projects we read per session — bounds latency and output. MAX_SHARED = 6 CODING_SESSION_PROFILE = "coding" +CODEX_DEFAULT_CHECKPOINT_ON_COMPACT = True CODEX_DEFAULT_CAPTURE_EVENTS = True -CODEX_CHECKPOINT_REASON = ( +CODEX_CHECKPOINT_PROMPT = ( "Basic Memory checkpoint required after compaction. Use the " "`codex:bm-checkpoint` skill now to write one deliberate, durable handoff " "for the work completed in this turn. Capture the problem, approach, actual " @@ -140,8 +145,8 @@ class HarnessProfile: checkpoint_tags=("codex", "auto-capture"), setup_nudge=( "_This repo is not configured for Basic Memory yet. Run `Use Basic Memory " - "for Codex to set up this repo` to map a project, seed schemas, and turn " - "on Codex checkpoints._" + "for Codex to set up this repo` to map a project, seed schemas, and " + "configure optional Codex checkpoints._" ), status_hint="Run `Use bm-status` to check the Basic Memory project mapping.", pin_tip=( @@ -286,14 +291,15 @@ def load_codex_settings(directory: Path) -> tuple[dict, bool]: """Merge user and project Codex settings, then resolve checkout defaults. Precedence (lowest to highest): ``~/.codex/basic-memory.json``, then the - nearest project ``.codex/basic-memory.json``. Redaction lists accumulate so - a project cannot weaken user-level privacy rules. Codex capture is enabled - when omitted, and its default folder is namespaced by the Git repository - directory. Any malformed source counts as configured and fails closed for - the whole evaluation so a later source cannot rebuild routing without the - missing redaction policy. + nearest project ``.codex/basic-memory.json``. Codex lifecycle capture and + checkpoint prompting are enabled when omitted. The default folder is + namespaced by the Git repository directory. Any malformed source counts as + configured and fails closed for the whole evaluation so a later source + cannot rebuild routing from incomplete settings. Existing redaction lists + continue to accumulate only for local lifecycle-envelope capture. """ defaults: dict = { + "checkpointOnCompact": CODEX_DEFAULT_CHECKPOINT_ON_COMPACT, "captureEvents": CODEX_DEFAULT_CAPTURE_EVENTS, "captureFolder": _codex_default_capture_folder(directory), } @@ -313,10 +319,14 @@ def load_codex_settings(directory: Path) -> tuple[dict, bool]: found = True if block is None: # Trigger: any configured source exists but cannot be trusted. - # Why: continuing could combine a later checkpoint route with a - # missing earlier redaction policy and persist unredacted data. + # Why: continuing could combine a later route with incomplete + # earlier settings and write to an unintended project. # Outcome: discard every route and disable capture for this event. - return {**defaults, "captureEvents": False}, True + return { + **defaults, + "checkpointOnCompact": False, + "captureEvents": False, + }, True cumulative_redactions: dict[str, list[str]] = {} for key in ("redactKeys", "redactPaths"): values = [*_string_list(merged.get(key)), *_string_list(block.get(key))] @@ -325,6 +335,11 @@ def load_codex_settings(directory: Path) -> tuple[dict, bool]: merged.update(block) merged.update(cumulative_redactions) + # This legacy option imposed an extra model-authored checkpoint scan. + # Ignore it so existing config cannot restore that gate. Redaction lists + # remain available only to the separate lifecycle-envelope capture path. + merged.pop("checkpointPrivacyReview", None) + return merged, found @@ -565,8 +580,10 @@ def _build_brief( profile: HarnessProfile, cfg: dict, configured: bool, + checkpoint_prompt: str | None = None, ) -> str: """Assemble the session-start context brief (ported from the hook scripts).""" + prompt_prefix = f"{checkpoint_prompt}\n\n---\n\n" if checkpoint_prompt else "" primary = str(cfg.get("primaryProject") or "").strip() timeframe = str(cfg.get("recallTimeframe") or profile.default_recall_timeframe) recall_prompt = str(cfg.get("recallPrompt") or profile.default_recall_prompt) @@ -580,7 +597,7 @@ def _build_brief( if cfg.get("sessionProfile") == CODING_SESSION_PROFILE: configured_repository = cfg.get("repository") if not isinstance(configured_repository, str) or not configured_repository.strip(): - return ( + return prompt_prefix + ( "# Basic Memory\n\n" "_Coding session setup is incomplete: `basicMemory.repository` is missing. " f"Rerun Basic Memory setup before recalling repository work. {profile.status_hint}_" @@ -597,9 +614,9 @@ def _build_brief( # Outcome: first-run → setup nudge; configured-but-broken → one-line signal. if context.tasks is None and context.decisions is None and context.sessions is None: if not configured: - return f"# Basic Memory\n\n{profile.setup_nudge}" + return prompt_prefix + f"# Basic Memory\n\n{profile.setup_nudge}" project_name = primary or "the default project" - return ( + return prompt_prefix + ( "# Basic Memory\n\n" f"_Couldn't read from `{project_name}` — it may be misnamed or unreachable. " f"{profile.status_hint}_" @@ -666,11 +683,11 @@ def _build_brief( # a visible notice INSIDE the fence, and guidance is emitted after `closing`, # so the caller's slice can only ever trim guidance — never reopen the fence. notice = "\n… [truncated]" - room = MAX_BRIEF_CHARS - len(opening) - len(closing) + room = MAX_BRIEF_CHARS - len(prompt_prefix) - len(opening) - len(closing) data_text = "\n".join(data_lines) if len(data_text) > room: data_text = data_text[: max(0, room - len(notice))].rstrip() + notice - lines = [opening + data_text + closing] + lines = [prompt_prefix + opening + data_text + closing] # Placement guidance — surfaced so the "follow the project's stored placement # conventions" reflex has something concrete to follow. @@ -678,7 +695,7 @@ def _build_brief( lines += [ "", "## Where to write", - f"- Session checkpoints (the PreCompact auto-capture) go to `{capture_folder}/`.", + f"- Session checkpoints go to `{capture_folder}/`.", ] if placement_conventions: lines.append( @@ -1029,7 +1046,18 @@ def _session_start(harness: Harness, project_dir: Optional[Path]) -> None: _capture_envelope(profile, event, SESSION_STARTED, cfg, mapping_dir, capture_folder) - brief = _build_brief(profile, cfg, configured) + primary = str(cfg.get("primaryProject") or "").strip() + checkpoint_prompt = ( + CODEX_CHECKPOINT_PROMPT + if ( + harness is Harness.codex + and event.trigger == "compact" + and primary + and cfg.get("checkpointOnCompact") is True + ) + else None + ) + brief = _build_brief(profile, cfg, configured, checkpoint_prompt) print(brief[:MAX_BRIEF_CHARS]) @@ -1053,15 +1081,9 @@ def _pre_compact(harness: Harness, project_dir: Optional[Path]) -> None: return if harness is Harness.codex: - # PreCompact stdout is ignored by Codex, so this hook cannot ask the - # active model to author memory directly. Persist a private request; the - # Stop hook turns it into a one-time continuation prompt after Codex has - # compacted the same turn and still has its summarized working context. - if not event.session_id: - return - from basic_memory.hooks import checkpoint_requests - - checkpoint_requests.create(event.session_id, event.turn_id) + # Codex ignores PreCompact stdout. SessionStart runs again with the + # `compact` trigger after compaction and asks the resumed agent to write + # the checkpoint from its summarized working context. return conversation = _transcript_turns(event.transcript_path, harness) @@ -1110,55 +1132,6 @@ def _pre_compact(harness: Harness, project_dir: Optional[Path]) -> None: print(f"bm hook pre-compact: checkpoint write failed: {result['error']}", file=sys.stderr) -def _codex_stop_response(payload: dict[str, Any]) -> dict[str, Any]: - """Resolve one Codex Stop event against its pending checkpoint request.""" - session_id = str(payload.get("session_id") or "") - if not session_id: - return {"continue": True} - - from basic_memory.hooks import checkpoint_requests - - request = checkpoint_requests.read(session_id) - if request is None: - return {"continue": True} - - current_turn_id = payload.get("turn_id") - # Trigger: the saved request belongs to an earlier Codex turn, or the - # current Stop cannot prove it belongs to the requesting turn. - # Why: an interrupted continuation can leave the request behind; blocking - # a later turn would author a checkpoint from the wrong working context. - # Outcome: abandon the stale handshake and let the current turn end. - if request.source_turn_id is not None and current_turn_id != request.source_turn_id: - checkpoint_requests.clear(session_id) - return {"continue": True} - - # Trigger: Codex says this Stop is already a continuation. Why: blocking - # again would create a Stop-hook loop. Outcome: clear the local handshake - # and allow the turn to end. Until Codex confirms that continuation state, - # leave the request pending so a failed hook response cannot lose it. - if payload.get("stop_hook_active") is True: - checkpoint_requests.clear(session_id) - return {"continue": True} - - return {"decision": "block", "reason": CODEX_CHECKPOINT_REASON} - - -def _stop(harness: Harness) -> None: - payload = _read_stdin_payload() - response = _codex_stop_response(payload) if harness is Harness.codex else {"continue": True} - print(json.dumps(response, separators=(",", ":"))) - - -def _run_json_fail_open(verb: str, run: Callable[[], None]) -> None: - """Run a JSON-output hook and always leave Codex with a valid response.""" - try: - run() - except (Exception, SystemExit) as exc: - logger.exception(f"bm hook {verb} failed") - print(f"bm hook {verb}: {exc}", file=sys.stderr) - print('{"continue":true}') - - # --- Typer verbs --- HARNESS_OPTION = typer.Option(Harness.claude, "--harness", help="Which harness fired the hook") @@ -1189,8 +1162,9 @@ def pre_compact( @hook_app.command("stop") def stop(harness: Harness = HARNESS_OPTION) -> None: - """Request one agent-authored Codex checkpoint after compaction.""" - _run_json_fail_open("stop", lambda: _stop(harness)) + """Allow stale pre-upgrade Stop hooks to finish without blocking Codex.""" + del harness + print('{"continue":true}') @hook_app.command("flush") @@ -1227,6 +1201,8 @@ def flush( # ``hook --harness `` suffix (rather than the launcher prefix) # matches every launcher form we may write — ``basic-memory``, ``bm``, and the # ``uvx "basic-memory>=X"`` fallback — while staying distinctive to our CLI. +# Keep the retired ``stop`` verb in the pattern so reinstall/remove cleans up +# entries written by older releases. OWNED_HOOK_COMMAND_RE = re.compile( r"\bhook\s+(?:session-start|pre-compact|stop)\s+--harness\s+(?:claude|codex)\b" ) @@ -1315,7 +1291,6 @@ def group(verb: str, timeout: int, matcher: str | None) -> dict[str, Any]: return { "SessionStart": group("session-start", 30, "startup|resume|compact"), "PreCompact": group("pre-compact", 60, "manual|auto"), - "Stop": group("stop", 30, None), } @@ -1416,7 +1391,8 @@ def install(harness: Harness = HARNESS_OPTION) -> None: typer.echo(f"error: {config_path}: 'hooks' is not an object; fix it and retry", err=True) raise typer.Exit(1) - for event, group in _owned_hook_groups(harness).items(): + desired_groups = _owned_hook_groups(harness) + for event, group in desired_groups.items(): existing = hooks.get(event) if existing is not None and not isinstance(existing, list): typer.echo( @@ -1429,6 +1405,23 @@ def install(harness: Harness = HARNESS_OPTION) -> None: groups.append(group) hooks[event] = groups + # Older Codex installs included a Stop hook. It is no longer part of the + # checkpoint flow, so reinstall removes only our retired entry while + # preserving any user-authored hooks in the same event. + for event in list(hooks): + if event in desired_groups: + continue + groups = hooks[event] + if not isinstance(groups, list): + continue + stripped = _strip_owned_hooks(groups) + if stripped == groups: + continue + if stripped: + hooks[event] = stripped + else: + del hooks[event] + _write_hook_config(config_path, data) typer.echo(f"installed {harness.value} hooks in {config_path}") @@ -1498,7 +1491,7 @@ def status( ) -> None: """Show inbox depth, last flush, settings summary, and tool versions.""" import basic_memory - from basic_memory.hooks import checkpoint_requests, inbox + from basic_memory.hooks import inbox pending = len(inbox.list_envelopes()) processed = len(list(inbox.processed_dir().glob("*.json"))) @@ -1509,7 +1502,6 @@ def status( typer.echo(f"inbox: {inbox.inbox_dir()}") typer.echo(f"pending envelopes: {pending}") typer.echo(f"archived envelopes: {processed}") - typer.echo(f"pending checkpoint requests: {checkpoint_requests.pending_count()}") typer.echo(f"last flush: {inbox.last_flush() or 'never'}") typer.echo( f"settings ({harness.value}, {mapping_dir}): {'found' if configured else 'not found'}" @@ -1517,6 +1509,9 @@ def status( typer.echo(f"primary project: {str(cfg.get('primaryProject') or '').strip() or '(not set)'}") typer.echo(f"session profile: {str(cfg.get('sessionProfile') or 'general').strip()}") typer.echo(f"repository: {str(cfg.get('repository') or '').strip() or '(not set)'}") + typer.echo( + f"checkpoint on compact: {'on' if cfg.get('checkpointOnCompact') is True else 'off'}" + ) typer.echo(f"capture events: {'on' if cfg.get('captureEvents') is True else 'off'}") typer.echo( f"capture folder: {str(cfg.get('captureFolder') or profile.default_capture_folder).strip()}" diff --git a/src/basic_memory/hooks/__init__.py b/src/basic_memory/hooks/__init__.py index f6090fdf2..458c4a775 100644 --- a/src/basic_memory/hooks/__init__.py +++ b/src/basic_memory/hooks/__init__.py @@ -15,5 +15,4 @@ - ``archive`` idempotent local audit-archive sweep - ``projector`` compatibility imports for the retired graph projector - ``project_ref`` project-name / project-id routing helpers - - ``checkpoint_requests`` private PreCompact -> Stop handoff state for Codex """ diff --git a/src/basic_memory/hooks/checkpoint_requests.py b/src/basic_memory/hooks/checkpoint_requests.py deleted file mode 100644 index 9f2ebcb97..000000000 --- a/src/basic_memory/hooks/checkpoint_requests.py +++ /dev/null @@ -1,108 +0,0 @@ -"""Private handoff state between Codex PreCompact and Stop hooks. - -PreCompact cannot ask the active model to author durable memory, and its stdout -is ignored. It records a small local request instead. The next Stop hook turns -that request into a one-time continuation prompt for the same Codex turn. -""" - -from __future__ import annotations - -import hashlib -import json -import os -import uuid -from dataclasses import asdict, dataclass -from datetime import datetime, timezone -from pathlib import Path - -from basic_memory.config import CONFIG_DIR_MODE, CONFIG_FILE_MODE, resolve_data_dir - -REQUESTS_DIR_NAME = "checkpoint-requests" - - -@dataclass(frozen=True, slots=True) -class CheckpointRequest: - source_session_id: str - source_turn_id: str | None - requested_at: str - - -def requests_dir() -> Path: - return resolve_data_dir() / REQUESTS_DIR_NAME - - -def pending_count() -> int: - """Count pending requests for the hook status surface.""" - return sum(1 for path in requests_dir().glob("*.json") if path.is_file()) - - -def _ensure_private_dir() -> Path: - directory = requests_dir() - directory.mkdir(parents=True, exist_ok=True) - if os.name != "nt": - resolve_data_dir().chmod(CONFIG_DIR_MODE) - directory.chmod(CONFIG_DIR_MODE) - return directory - - -def _request_path(session_id: str) -> Path: - if not session_id: - raise ValueError("checkpoint request requires a Codex session id") - digest = hashlib.sha256(f"codex\0{session_id}".encode()).hexdigest() - return requests_dir() / f"{digest}.json" - - -def _write(request: CheckpointRequest) -> Path: - directory = _ensure_private_dir() - target = _request_path(request.source_session_id) - tmp = directory / f"{target.name}.{uuid.uuid4().hex}.tmp" - tmp.write_text(json.dumps(asdict(request), sort_keys=True) + "\n", encoding="utf-8") - if os.name != "nt": - tmp.chmod(CONFIG_FILE_MODE) - os.replace(tmp, target) - return target - - -def create(session_id: str, turn_id: str | None) -> CheckpointRequest: - """Create or replace the pending request for one Codex session.""" - request = CheckpointRequest( - source_session_id=session_id, - source_turn_id=turn_id, - requested_at=datetime.now(timezone.utc).isoformat(timespec="seconds"), - ) - _write(request) - return request - - -def read(session_id: str) -> CheckpointRequest | None: - """Read a pending request, returning ``None`` when none exists.""" - path = _request_path(session_id) - try: - payload = json.loads(path.read_text(encoding="utf-8")) - except FileNotFoundError: - return None - if not isinstance(payload, dict): - raise ValueError(f"invalid checkpoint request: {path}") - try: - source_session_id = payload["source_session_id"] - source_turn_id = payload.get("source_turn_id") - requested_at = payload["requested_at"] - except KeyError as exc: - raise ValueError(f"invalid checkpoint request: {path}") from exc - if ( - not isinstance(source_session_id, str) - or source_session_id != session_id - or (source_turn_id is not None and not isinstance(source_turn_id, str)) - or not isinstance(requested_at, str) - ): - raise ValueError(f"invalid checkpoint request: {path}") - return CheckpointRequest( - source_session_id=source_session_id, - source_turn_id=source_turn_id, - requested_at=requested_at, - ) - - -def clear(session_id: str) -> None: - """Clear a satisfied or deliberately abandoned request.""" - _request_path(session_id).unlink(missing_ok=True) diff --git a/tests/cli/test_hook_command.py b/tests/cli/test_hook_command.py index a45945dc8..a1cffa0dc 100644 --- a/tests/cli/test_hook_command.py +++ b/tests/cli/test_hook_command.py @@ -468,6 +468,168 @@ def test_session_start_codex_does_not_query_lifecycle_trace(bm_home: Path, tmp_p assert mock_search.await_args_list[2].kwargs["note_types"] == ["codex_session"] +@pytest.mark.parametrize( + "session_settings", + [ + {}, + {"sessionProfile": "coding", "repository": "owner/repo"}, + ], +) +def test_codex_compact_session_start_requests_agent_authored_checkpoint( + bm_home: Path, + tmp_path: Path, + session_settings: dict[str, str], +) -> None: + project = tmp_path / "codex-proj" + (project / ".codex").mkdir(parents=True) + (project / ".codex" / "basic-memory.json").write_text( + json.dumps( + { + "basicMemory": { + "primaryProject": "demo", + "checkpointOnCompact": True, + **session_settings, + } + } + ), + encoding="utf-8", + ) + + with patch( + "basic_memory.mcp.tools.search_notes", + new_callable=AsyncMock, + return_value=SEARCH_EMPTY, + ): + result = runner.invoke( + cli_app, + ["hook", "session-start", "--harness", "codex", "--project-dir", str(project)], + input=_payload(project, trigger="compact"), + ) + + assert result.exit_code == 0 + assert result.stdout.count("`codex:bm-checkpoint`") == 1 + assert "Do not write lifecycle telemetry or a transcript dump" in result.stdout + + +def test_codex_compact_checkpoint_prompt_defaults_on(bm_home: Path, tmp_path: Path) -> None: + project = tmp_path / "codex-proj" + (project / ".codex").mkdir(parents=True) + (project / ".codex" / "basic-memory.json").write_text( + json.dumps({"basicMemory": {"primaryProject": "demo"}}), + encoding="utf-8", + ) + + with patch( + "basic_memory.mcp.tools.search_notes", + new_callable=AsyncMock, + return_value=SEARCH_EMPTY, + ): + result = runner.invoke( + cli_app, + ["hook", "session-start", "--harness", "codex", "--project-dir", str(project)], + input=_payload(project, trigger="compact"), + ) + + assert result.exit_code == 0 + assert "`codex:bm-checkpoint`" in result.stdout + + +@pytest.mark.parametrize("gate_value", ["true", 1, "yes", {"on": True}]) +def test_codex_compact_checkpoint_prompt_fails_closed_on_non_boolean( + bm_home: Path, tmp_path: Path, gate_value: object +) -> None: + project = tmp_path / "codex-proj" + (project / ".codex").mkdir(parents=True) + (project / ".codex" / "basic-memory.json").write_text( + json.dumps( + { + "basicMemory": { + "primaryProject": "demo", + "checkpointOnCompact": gate_value, + } + } + ), + encoding="utf-8", + ) + + with patch( + "basic_memory.mcp.tools.search_notes", + new_callable=AsyncMock, + return_value=SEARCH_EMPTY, + ): + result = runner.invoke( + cli_app, + ["hook", "session-start", "--harness", "codex", "--project-dir", str(project)], + input=_payload(project, trigger="compact"), + ) + + assert result.exit_code == 0 + assert "`codex:bm-checkpoint`" not in result.stdout + + +def test_codex_startup_does_not_request_enabled_checkpoint(bm_home: Path, tmp_path: Path) -> None: + project = tmp_path / "codex-proj" + (project / ".codex").mkdir(parents=True) + (project / ".codex" / "basic-memory.json").write_text( + json.dumps( + { + "basicMemory": { + "primaryProject": "demo", + "checkpointOnCompact": True, + } + } + ), + encoding="utf-8", + ) + + with patch( + "basic_memory.mcp.tools.search_notes", + new_callable=AsyncMock, + return_value=SEARCH_EMPTY, + ): + result = runner.invoke( + cli_app, + ["hook", "session-start", "--harness", "codex", "--project-dir", str(project)], + input=_payload(project, trigger="startup"), + ) + + assert result.exit_code == 0 + assert "`codex:bm-checkpoint`" not in result.stdout + + +def test_codex_compact_checkpoint_prompt_survives_unreachable_memory( + bm_home: Path, tmp_path: Path +) -> None: + project = tmp_path / "codex-proj" + (project / ".codex").mkdir(parents=True) + (project / ".codex" / "basic-memory.json").write_text( + json.dumps( + { + "basicMemory": { + "primaryProject": "demo", + "checkpointOnCompact": True, + } + } + ), + encoding="utf-8", + ) + + with patch( + "basic_memory.mcp.tools.search_notes", + new_callable=AsyncMock, + side_effect=RuntimeError("offline"), + ): + result = runner.invoke( + cli_app, + ["hook", "session-start", "--harness", "codex", "--project-dir", str(project)], + input=_payload(project, trigger="compact"), + ) + + assert result.exit_code == 0 + assert "`codex:bm-checkpoint`" in result.stdout + assert "Couldn't read from `demo`" in result.stdout + + # --- session-start / pre-compact: envelope capture gate --- @@ -495,11 +657,14 @@ def test_capture_events_true_boolean_writes_envelope(bm_home: Path, claude_proje assert envelope["payload"]["capture_folder"] == "sessions" -def test_codex_capture_events_defaults_on(bm_home: Path, tmp_path: Path) -> None: +def test_codex_capture_events_defaults_on_and_preserves_legacy_redaction( + bm_home: Path, tmp_path: Path +) -> None: project = tmp_path / "codex-proj" (project / ".codex").mkdir(parents=True) (project / ".codex" / "basic-memory.json").write_text( - json.dumps({"basicMemory": {"primaryProject": "demo"}}), encoding="utf-8" + json.dumps({"basicMemory": {"primaryProject": "demo", "redactKeys": ["trigger"]}}), + encoding="utf-8", ) with patch( "basic_memory.mcp.tools.search_notes", new_callable=AsyncMock, return_value=SEARCH_EMPTY @@ -515,6 +680,7 @@ def test_codex_capture_events_defaults_on(bm_home: Path, tmp_path: Path) -> None assert len(envelopes) == 1 assert envelopes[0]["source"] == "codex" assert envelopes[0]["payload"]["capture_folder"] == "codex" + assert envelopes[0]["payload"]["trigger"] == "[REDACTED]" @pytest.mark.parametrize("gate_value", ["true", "false", 1, "yes", {"on": True}]) @@ -769,11 +935,7 @@ def test_pre_compact_surfaces_write_error_on_stderr( assert "checkpoint write failed" in result.stderr -def test_codex_pre_compact_requests_agent_authored_checkpoint_at_stop( - bm_home: Path, tmp_path: Path -) -> None: - from basic_memory.hooks import checkpoint_requests - +def test_codex_pre_compact_only_captures_lifecycle_event(bm_home: Path, tmp_path: Path) -> None: project = tmp_path / "codex-proj" (project / ".codex").mkdir(parents=True) _init_git_repo(project) @@ -798,31 +960,22 @@ def test_codex_pre_compact_requests_agent_authored_checkpoint_at_stop( assert result.exit_code == 0 mock_write.assert_not_awaited() - request = checkpoint_requests.read("s-abc12345") - assert request is not None - assert request.source_turn_id == "turn-42" - - first_stop = runner.invoke( - cli_app, - ["hook", "stop", "--harness", "codex"], - input=_payload(project, turn_id="turn-42", stop_hook_active=False), - ) + envelopes = _inbox_envelopes(bm_home) + assert len(envelopes) == 1 + assert envelopes[0]["event"] == "compaction_imminent" + assert not (bm_home / "checkpoint-requests").exists() - assert first_stop.exit_code == 0 - response = json.loads(first_stop.stdout) - assert response["decision"] == "block" - assert "`codex:bm-checkpoint`" in response["reason"] - assert "lifecycle telemetry" in response["reason"] - assert checkpoint_requests.read("s-abc12345") is not None - second_stop = runner.invoke( +def test_deprecated_codex_stop_is_json_noop(bm_home: Path) -> None: + """Older installed Stop entries must remain fail-open until reinstall.""" + result = runner.invoke( cli_app, ["hook", "stop", "--harness", "codex"], - input=_payload(project, turn_id="turn-42", stop_hook_active=True), + input=json.dumps({"session_id": "stale", "stop_hook_active": False}), ) - assert json.loads(second_stop.stdout) == {"continue": True} - assert checkpoint_requests.read("s-abc12345") is None + assert result.exit_code == 0 + assert json.loads(result.stdout) == {"continue": True} def test_codex_transcript_parser_reads_response_items_only(tmp_path: Path) -> None: @@ -834,39 +987,6 @@ def test_codex_transcript_parser_reads_response_items_only(tmp_path: Path) -> No ] -def test_codex_stop_without_checkpoint_request_is_json_noop(bm_home: Path) -> None: - result = runner.invoke( - cli_app, - ["hook", "stop", "--harness", "codex"], - input=json.dumps({"session_id": "none", "stop_hook_active": False}), - ) - - assert result.exit_code == 0 - assert json.loads(result.stdout) == {"continue": True} - - -def test_codex_stop_clears_checkpoint_request_from_prior_turn(bm_home: Path) -> None: - from basic_memory.hooks import checkpoint_requests - - checkpoint_requests.create("s-abc12345", "turn-42") - - result = runner.invoke( - cli_app, - ["hook", "stop", "--harness", "codex"], - input=json.dumps( - { - "session_id": "s-abc12345", - "turn_id": "turn-43", - "stop_hook_active": False, - } - ), - ) - - assert result.exit_code == 0 - assert json.loads(result.stdout) == {"continue": True} - assert checkpoint_requests.read("s-abc12345") is None - - def test_pre_compact_codex_malformed_project_does_not_use_user_checkpoint_route( bm_home: Path, tmp_path: Path ) -> None: @@ -878,7 +998,6 @@ def test_pre_compact_codex_malformed_project_does_not_use_user_checkpoint_route( "basicMemory": { "primaryProject": "user-wide", "captureEvents": True, - "redactPaths": ["/shared/private"], } } ), @@ -916,7 +1035,6 @@ def test_pre_compact_codex_malformed_user_blocks_project_checkpoint_route( "basicMemory": { "primaryProject": "project-level", "captureEvents": True, - "redactPaths": ["/project/private"], } } ), @@ -1054,10 +1172,10 @@ def test_status_reports_inbox_and_settings( assert result.exit_code == 0 assert "pending envelopes: 1" in result.stdout assert "archived envelopes: 1" in result.stdout - assert "pending checkpoint requests: 0" in result.stdout assert "last flush: 2026-07-15T10:00:00+00:00" in result.stdout assert "found" in result.stdout assert "primary project: demo" in result.stdout + assert "checkpoint on compact: off" in result.stdout assert "capture events: on" in result.stdout assert "capture folder: sessions" in result.stdout assert "basic-memory version:" in result.stdout @@ -1077,10 +1195,28 @@ def test_status_defaults_when_nothing_configured( assert "pending envelopes: 0" in result.stdout assert "last flush: never" in result.stdout assert "primary project: (not set)" in result.stdout + assert "checkpoint on compact: off" in result.stdout assert "capture events: off" in result.stdout assert "uv: (not found)" in result.stdout +def test_codex_status_reports_enabled_checkpoint_prompt(bm_home: Path, tmp_path: Path) -> None: + project = tmp_path / "codex-proj" + (project / ".codex").mkdir(parents=True) + (project / ".codex" / "basic-memory.json").write_text( + json.dumps({"basicMemory": {"checkpointOnCompact": True}}), + encoding="utf-8", + ) + + result = runner.invoke( + cli_app, + ["hook", "status", "--harness", "codex", "--project-dir", str(project)], + ) + + assert result.exit_code == 0 + assert "checkpoint on compact: on" in result.stdout + + # --- install / remove --- @@ -1132,10 +1268,26 @@ def test_install_codex_writes_hooks_json_with_matchers() -> None: "basic-memory hook session-start --harness codex" ) assert data["hooks"]["PreCompact"][0]["matcher"] == "manual|auto" - assert data["hooks"]["Stop"][0]["hooks"][0]["command"] == ( - "basic-memory hook stop --harness codex" + assert "Stop" not in data["hooks"] + + +def test_install_codex_removes_retired_stop_hook() -> None: + path = _codex_hooks_path() + path.parent.mkdir(parents=True, exist_ok=True) + retired = { + "type": "command", + "command": "basic-memory hook stop --harness codex", + } + path.write_text( + json.dumps({"hooks": {"Stop": [{"hooks": [USER_HOOK, retired]}]}}), + encoding="utf-8", ) + result = runner.invoke(cli_app, ["hook", "install", "--harness", "codex"]) + + assert result.exit_code == 0 + assert _read_json(path)["hooks"]["Stop"] == [{"hooks": [USER_HOOK]}] + def test_install_preserves_existing_user_settings_and_hooks() -> None: path = _claude_settings_path() @@ -1745,7 +1897,11 @@ def test_codex_settings_broken_file_counts_as_configured(tmp_path: Path) -> None merged, found = hook_module.load_codex_settings(project) - assert merged == {"captureEvents": False, "captureFolder": "codex"} + assert merged == { + "checkpointOnCompact": False, + "captureEvents": False, + "captureFolder": "codex", + } assert found is True @@ -1758,7 +1914,6 @@ def test_codex_settings_malformed_project_invalidates_user_fallback(tmp_path: Pa "basicMemory": { "primaryProject": "user-wide", "captureEvents": True, - "redactPaths": ["/shared/private"], } } ), @@ -1770,7 +1925,11 @@ def test_codex_settings_malformed_project_invalidates_user_fallback(tmp_path: Pa merged, found = hook_module.load_codex_settings(project) - assert merged == {"captureEvents": False, "captureFolder": "codex"} + assert merged == { + "checkpointOnCompact": False, + "captureEvents": False, + "captureFolder": "codex", + } assert found is True @@ -1786,7 +1945,6 @@ def test_codex_settings_malformed_user_blocks_project_fallback(tmp_path: Path) - "basicMemory": { "primaryProject": "project-level", "captureEvents": True, - "redactPaths": ["/project/private"], } } ), @@ -1795,7 +1953,11 @@ def test_codex_settings_malformed_user_blocks_project_fallback(tmp_path: Path) - merged, found = hook_module.load_codex_settings(project) - assert merged == {"captureEvents": False, "captureFolder": "codex"} + assert merged == { + "checkpointOnCompact": False, + "captureEvents": False, + "captureFolder": "codex", + } assert found is True @@ -1805,7 +1967,11 @@ def test_codex_settings_non_dict_document(tmp_path: Path) -> None: (project / ".codex" / "basic-memory.json").write_text("[1]", encoding="utf-8") assert hook_module.load_codex_settings(project) == ( - {"captureEvents": False, "captureFolder": "codex"}, + { + "checkpointOnCompact": False, + "captureEvents": False, + "captureFolder": "codex", + }, True, ) @@ -1818,7 +1984,11 @@ def test_codex_settings_non_dict_basic_memory_block(tmp_path: Path) -> None: ) assert hook_module.load_codex_settings(project) == ( - {"captureEvents": False, "captureFolder": "codex"}, + { + "checkpointOnCompact": False, + "captureEvents": False, + "captureFolder": "codex", + }, True, ) @@ -1828,7 +1998,11 @@ def test_codex_settings_default_on_without_config(tmp_path: Path) -> None: project.mkdir() assert hook_module.load_codex_settings(project) == ( - {"captureEvents": True, "captureFolder": "codex"}, + { + "checkpointOnCompact": True, + "captureEvents": True, + "captureFolder": "codex", + }, False, ) @@ -1871,6 +2045,7 @@ def test_codex_settings_merge_user_then_project_with_checkout_folder(tmp_path: P assert found is True assert merged["primaryProject"] == "project-level" assert merged["recallTimeframe"] == "9d" + assert merged["checkpointOnCompact"] is True assert merged["captureEvents"] is True assert merged["captureFolder"] == "codex/widgets" assert merged["redactKeys"] == ["token", "shared-secret", "repo-secret"] @@ -1904,6 +2079,50 @@ def test_codex_project_settings_override_user_capture_defaults(tmp_path: Path) - assert merged["captureFolder"] == "private/checkpoints" +def test_codex_project_settings_can_disable_user_checkpoint_setting(tmp_path: Path) -> None: + home = Path.home() + (home / ".codex").mkdir(parents=True, exist_ok=True) + (home / ".codex" / "basic-memory.json").write_text( + json.dumps({"basicMemory": {"checkpointOnCompact": True}}), + encoding="utf-8", + ) + project = tmp_path / "proj" + (project / ".codex").mkdir(parents=True) + (project / ".codex" / "basic-memory.json").write_text( + json.dumps({"basicMemory": {"checkpointOnCompact": False}}), + encoding="utf-8", + ) + + merged, found = hook_module.load_codex_settings(project) + + assert found is True + assert merged["checkpointOnCompact"] is False + + +def test_codex_settings_ignore_legacy_checkpoint_privacy_review(tmp_path: Path) -> None: + project = tmp_path / "proj" + (project / ".codex").mkdir(parents=True) + (project / ".codex" / "basic-memory.json").write_text( + json.dumps( + { + "basicMemory": { + "checkpointPrivacyReview": True, + "redactKeys": ["token"], + "redactPaths": ["/private"], + } + } + ), + encoding="utf-8", + ) + + merged, found = hook_module.load_codex_settings(project) + + assert found is True + assert "checkpointPrivacyReview" not in merged + assert merged["redactKeys"] == ["token"] + assert merged["redactPaths"] == ["/private"] + + def test_string_list_guards_config_types() -> None: assert hook_module._string_list(None) == [] assert hook_module._string_list("not-a-list") == [] diff --git a/tests/hooks/test_checkpoint_requests.py b/tests/hooks/test_checkpoint_requests.py deleted file mode 100644 index 7155258eb..000000000 --- a/tests/hooks/test_checkpoint_requests.py +++ /dev/null @@ -1,81 +0,0 @@ -"""Tests for the private Codex PreCompact-to-Stop handshake state.""" - -import json -import os -import stat -from pathlib import Path - -import pytest - -from basic_memory.hooks import checkpoint_requests - - -def test_checkpoint_request_lifecycle_is_private(bm_home: Path) -> None: - request = checkpoint_requests.create("session-1", "turn-1") - - assert checkpoint_requests.pending_count() == 1 - assert checkpoint_requests.read("session-1") == request - - paths = list(checkpoint_requests.requests_dir().glob("*.json")) - assert len(paths) == 1 - if os.name != "nt": - assert stat.S_IMODE(checkpoint_requests.requests_dir().stat().st_mode) == 0o700 - assert stat.S_IMODE(paths[0].stat().st_mode) == 0o600 - - checkpoint_requests.clear("session-1") - assert checkpoint_requests.read("session-1") is None - assert checkpoint_requests.pending_count() == 0 - - -def test_checkpoint_request_replaces_same_session_atomically(bm_home: Path) -> None: - first = checkpoint_requests.create("session-1", "turn-1") - second = checkpoint_requests.create("session-1", "turn-2") - - assert second.requested_at >= first.requested_at - assert checkpoint_requests.read("session-1") == second - assert checkpoint_requests.pending_count() == 1 - assert not list(checkpoint_requests.requests_dir().glob("*.tmp")) - - -def test_checkpoint_request_requires_session_id(bm_home: Path) -> None: - with pytest.raises(ValueError, match="requires a Codex session id"): - checkpoint_requests.create("", None) - - -def test_checkpoint_request_rejects_non_object(bm_home: Path) -> None: - checkpoint_requests.create("session-1", None) - path = next(checkpoint_requests.requests_dir().glob("*.json")) - path.write_text("[]", encoding="utf-8") - - with pytest.raises(ValueError, match="invalid checkpoint request"): - checkpoint_requests.read("session-1") - - -def test_checkpoint_request_rejects_missing_fields(bm_home: Path) -> None: - checkpoint_requests.create("session-1", None) - path = next(checkpoint_requests.requests_dir().glob("*.json")) - path.write_text("{}", encoding="utf-8") - - with pytest.raises(ValueError, match="invalid checkpoint request"): - checkpoint_requests.read("session-1") - - -@pytest.mark.parametrize( - "override", - [ - {"source_session_id": "other"}, - {"source_turn_id": 42}, - {"requested_at": 42}, - ], -) -def test_checkpoint_request_rejects_invalid_field_types( - bm_home: Path, override: dict[str, object] -) -> None: - checkpoint_requests.create("session-1", None) - path = next(checkpoint_requests.requests_dir().glob("*.json")) - payload = json.loads(path.read_text(encoding="utf-8")) - payload.update(override) - path.write_text(json.dumps(payload), encoding="utf-8") - - with pytest.raises(ValueError, match="invalid checkpoint request"): - checkpoint_requests.read("session-1") diff --git a/tests/test_codex_plugin_package.py b/tests/test_codex_plugin_package.py index 044f0b36f..8a29679bc 100644 --- a/tests/test_codex_plugin_package.py +++ b/tests/test_codex_plugin_package.py @@ -85,6 +85,10 @@ def test_codex_plugin_docs_explain_global_install_and_repo_mapping() -> None: assert "marketplace file should not" in readme assert "Configuration can live at user level in `~/.codex/basic-memory.json`" in readme assert "the nearest project file overrides only the keys it declares" in readme + assert '"checkpointOnCompact": true' in readme + assert '"rememberFolder": "codex/remember"' in readme + assert "Checkpoint prompting is on by default" in readme + assert "Decision notes default to `codex/decisions`" in readme assert "keep both the profile and checkout-specific repository" in readme @@ -106,6 +110,18 @@ def test_user_level_coding_profile_stays_with_repository_override() -> None: assert '"sessionProfile": "coding",\n "repository": "owner/name"' in setup +def test_codex_manual_capture_defaults_share_codex_tree() -> None: + repo_root = Path(__file__).resolve().parents[1] + setup = (repo_root / "plugins/codex/skills/bm-setup/SKILL.md").read_text(encoding="utf-8") + decide = (repo_root / "plugins/codex/skills/bm-decide/SKILL.md").read_text(encoding="utf-8") + remember = (repo_root / "plugins/codex/skills/bm-remember/SKILL.md").read_text(encoding="utf-8") + + assert '"rememberFolder": "codex/remember"' in setup + assert "Default decisions to `codex/decisions`" in setup + assert "otherwise use `codex/decisions`" in decide + assert "`rememberFolder`, default `codex/remember`" in remember + + def test_coding_session_schema_is_shared_across_host_plugins() -> None: repo_root = Path(__file__).resolve().parents[1] codex_schema = (repo_root / "plugins/codex/schemas/coding-session.md").read_text( @@ -157,18 +173,24 @@ def test_bm_checkpoint_tells_a_story_and_uses_graph_semantics() -> None: assert "Do not invent intent, impact, verification, decisions, or drama" in writing -def test_codex_checkpoint_applies_accumulated_redaction_before_write() -> None: +def test_codex_checkpoint_has_no_plugin_redaction_gate() -> None: repo_root = Path(__file__).resolve().parents[1] - skill = (repo_root / "plugins/codex/skills/bm-checkpoint/SKILL.md").read_text(encoding="utf-8") + plugin_files = ( + repo_root / "plugins/codex/README.md", + repo_root / "plugins/codex/skills/bm-checkpoint/SKILL.md", + repo_root / "plugins/codex/skills/bm-setup/SKILL.md", + repo_root / "plugins/codex/skills/bm-status/SKILL.md", + ) + + for path in plugin_files: + text = path.read_text(encoding="utf-8") + assert "checkpointPrivacyReview" not in text + assert "redactKeys" not in text + assert "redactPaths" not in text - assert "## Privacy Gate" in skill - assert "`redactKeys` and `redactPaths` accumulate" in skill - assert "Scrub **every string** passed to `write_note`" in skill - assert "Do not omit schema-required path fields; use the marker" in skill - assert "[REDACTED_PATH]" in skill - assert "[REDACTED]" in skill - assert "fail closed: skip the checkpoint" in skill - assert "Do not fall back to an unredacted note" in skill + checkpoint = plugin_files[1].read_text(encoding="utf-8") + assert "## Optional Privacy Review" not in checkpoint + assert "Scrub **every string** passed to `write_note`" not in checkpoint def test_infographics_skill_keeps_weekly_contract_and_bm_style_pool() -> None: