feat(memory): Codex parity — register PreToolUse (Bash|apply_patch) and PreCompact, clamp the Codex envelope — release 0.28.0 (issue #95) - #152
Conversation
…nd PreCompact, clamp the Codex envelope — release 0.28.0 (issue #95)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 17 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
🔵 Needs a closer look
It is a versioned release whose correctness depends on cryptographic release-manifest hashes and on upstream Codex behavior claims (spill limit, matcher semantics, trust gate) that cannot be independently verified here, warranting human sign-off.
Pull request overview
This PR brings the Codex host to parity with Claude/ZCode by registering two previously-omitted coding-host hooks and adding a Codex-specific safety cap, then cutting release 0.28.0. It implements Workstream D-3 (issue #95): Codex now fires PreToolUse (pre-tool hazard recall) and PreCompact (delivery-ledger clear before compaction) through the already-wired pretool-recall/precompact launcher verbs, and clamps Codex hook envelopes below upstream's output-spill threshold so a full fence is never spilled to a file and hidden from the model. PostCompact is intentionally deferred to #118.
Changes:
- Register Codex
PreToolUse(matcherBash|apply_patch, from a live tool-name dump) andPreCompact(sharedprecompacthandler) inhooks/hooks.codex.json; MCP/write_stdinexcluded. - Add a Codex-only envelope clamp
CODEX_ENVELOPE_CAP_CHARS = 8000inhooks/zmem-launch.js, applied viaMath.min(resolveBudget, cap)so it binds even a large operator-setZMEM_CTX_BUDGET; Claude/ZCode unaffected. - Flip the former absence pins to registration assertions, add new adapter/clamp tests, update docs, and bump the version to 0.28.0 across all manifests and the regenerated release manifest.
File summaries
| File | Description |
|---|---|
| hooks/hooks.codex.json | Registers PreToolUse (matcher `Bash |
| hooks/zmem-launch.js | Adds CODEX_ENVELOPE_CAP_CHARS=8000 and clamps the resolved budget for the codex host. |
| hooks/zmem-pretool-recall.sh | Comment-only update reflecting the new Codex registration and matcher. |
| skills/memory/SKILL.md | Rewrites the Codex parity/timing text to the wired state; documents the cap and the hook re-approval requirement. |
| tests/test_codex_adapter.js | New sections [6] (clamp) and [7] (registered pre-tool/precompact drive) plus PostCompact-absence pin. |
| tests/test_pretool_inject.py | Inverts the Codex PreToolUse absence pin; asserts matcher and PreCompact registration; updated SKILL needle test. |
| tests/test_recall_hook_fence.py | Inverts the Codex PreCompact absence pin to a registration assertion. |
| release-manifest.json | Updated file hashes (5 files) and version → 0.28.0; new digest. |
| README.md | Documents the Codex 8000-char envelope cap for ZMEM_CTX_BUDGET. |
| CHANGELOG.md | Adds the dated 0.28.0 section. |
| hermes-plugin/plugin.yaml, marketplace.json, .claude-plugin/plugin.json, .claude-plugin/marketplace.json, .codex-plugin/plugin.json, .zcode-plugin/plugin.json, .agents/plugins/marketplace.json | Version bump 0.27.0 → 0.28.0. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // clamp the codex envelope to 8000 encoded chars (~2000 tokens at the | ||
| // plugin's 4-chars/token estimator) EVEN when the operator sets a huge | ||
| // ZMEM_CTX_BUDGET — and the clamp must be codex-specific. | ||
| function giantEnvelopeEncoded(hostVar, hostValue) { |
swarm-pr-review — PR #152 (
|
| ID | Severity | Summary |
|---|---|---|
sec-01 |
LOW | The "Codex treats Bash|apply_patch as exact alternation" claim is verified only by a JS reimplementation of that assumption inside the test itself (tests/test_codex_adapter.js:487-502), not against real codex-rs source — self-consistency, not an external-drift guard. |
C3 |
LOW | Same root cause from the compatibility lane — the matcher-exactness model has no vendored/pinned upstream reference; if upstream semantics differ (e.g. true regex vs. hardcoded exact-split), only tool names containing Bash/apply_patch as a substring would be affected. |
sec-02 |
INFO | CODEX_ENVELOPE_CAP_CHARS = 8000 and its calibration against upstream's DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT = 2_500 are documented only in prose comments, with no pinned constant or CI check to catch future upstream drift (fail-soft, not unsafe). |
C4 |
INFO | Minor 3-way doc drift: hooks/zmem-pretool-recall.sh:24-28's comment omits write_stdin from the excluded-tool list that SKILL.md, CHANGELOG.md, and the tests all mention. No behavioral effect. |
TF-01 |
INFO | No test feeds malformed/non-JSON stdin specifically through the new Codex PreToolUse/PreCompact registration (only well-formed payloads in tests/test_codex_adapter.js). Empirically verified fail-open still holds (printf 'not json' | node hooks/zmem-launch.js pretool-recall → {}, rc=0), so this is a coverage gap, not a live defect. |
Pre-existing, out of this PR's scope (surfaced for awareness only)
sec-03/ sibling tohooks/lib/zmem-recall-body.py:908-926: PreCompact's exclude-ID computation has no read-side carve-out (only the write side is guarded), predating this PR and untouched by its diff. Currently masked on Codex since upstream dropsadditionalContexton PreCompact anyway, so the PR doesn't newly expose it in a way that matters — but the same gap exists for Claude's PreCompact leg and may be worth its own issue.RP-01: the new CodexPreToolUseregistration adds measured ~1.4–2.2s warm latency perBash/apply_patchcall, but this reuses the exact code path and timeout (15s) already shipped for Claude's broaderEdit|Write|MultiEdit|NotebookEdit|Bashmatcher — not new exposure.C2: README's "optional" language for repo-local.codex/hooks.jsonis accurate (confirmed againstdoctor.py'soptional_codexvsrequired["codex_plugin"]split); the adjacent claim that "Claude/ZCode plugin surfaces are first-class now" (omitting Codex) is stale but predates this PR's diff.
Disproved candidates (listed for completeness, no action needed)
c1— the claimed "SessionStart re-injection needssource=compacthandling" was traced end-to-end and found to be a false comparison:zmem-session-start.shinjects unconditionally on every SessionStart using the delivery ledger's exclusion state (which PreCompact clears), with nosource-based branching required.c2— the hypothesized fail-open gap inzmem-recall-body.py's earlymain()setup doesn't hold; every cited statement is individually exception-guarded.TF-02— canary "failure-looking" stderr lines only print inside actual failure branches; cannot appear during a passing run (and the file isn't touched by this PR).RP-02— the "new concurrency exposure" premise is false: Claude already registers the identical sharedPreToolUse+PreCompacthandler pair this PR extends to Codex, and ledger writes are atomic (tmp-file +os.replace).
Recommendation: Approve after addressing the MEDIUM finding (README upgrade-path documentation gap) — it's docs-only to fix but has a real, currently-undetectable functional consequence for upgrading Codex users. Everything else is advisory/pre-existing and doesn't block merge.
Review conducted per swarm-pr-review protocol (4 explorer lanes / 2 independent reviewer batches / 1 critic challenge, ~30 subagent-minutes, models: sonnet explorers+reviewers, opus critic). Full candidate ledger available on request.
… warn on operator clamp (review PRR-001..005)
|
Both findings addressed in bb54467 (pushed to the PR):
Also fixed in the same pass (from this PR's own swarm review): the codex clamp now warns on stderr when an explicitly-set Suites: |
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/zmem-launch.js">
<violation number="1" location="hooks/zmem-launch.js:566">
P2: When a payload contains only `systemMessage`, this reservation can still emit an envelope larger than `budget` because the base envelope bytes are not checked. Check the complete system-only envelope and fail open when it cannot fit.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (sysMsg) { | ||
| const sysBytes = encodedSize(_withSystemMessage(makeEnvelope(host, hookName, ""), sysMsg)) | ||
| - encodedSize(makeEnvelope(host, hookName, "")); | ||
| if (sysBytes >= budget) { | ||
| sysMsg = null; | ||
| } else { | ||
| contentBudget = budget - sysBytes; | ||
| } | ||
| } |
There was a problem hiding this comment.
P2: When a payload contains only systemMessage, this reservation can still emit an envelope larger than budget because the base envelope bytes are not checked. Check the complete system-only envelope and fail open when it cannot fit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/zmem-launch.js, line 566:
<comment>When a payload contains only `systemMessage`, this reservation can still emit an envelope larger than `budget` because the base envelope bytes are not checked. Check the complete system-only envelope and fail open when it cannot fit.</comment>
<file context>
@@ -544,36 +544,63 @@ function fitEnvelope(host, hookName, content, budget) {
const hasContent = !(content === undefined || content === null || content === "");
if (!hasContent && !sysMsg) return {};
+ let contentBudget = budget;
+ if (sysMsg) {
+ const sysBytes = encodedSize(_withSystemMessage(makeEnvelope(host, hookName, ""), sysMsg))
+ - encodedSize(makeEnvelope(host, hookName, ""));
</file context>
| if (sysMsg) { | |
| const sysBytes = encodedSize(_withSystemMessage(makeEnvelope(host, hookName, ""), sysMsg)) | |
| - encodedSize(makeEnvelope(host, hookName, "")); | |
| if (sysBytes >= budget) { | |
| sysMsg = null; | |
| } else { | |
| contentBudget = budget - sysBytes; | |
| } | |
| } | |
| if (sysMsg) { | |
| const emptyEnvelope = makeEnvelope(host, hookName, ""); | |
| const emptyBytes = encodedSize(emptyEnvelope); | |
| const withSys = _withSystemMessage(emptyEnvelope, sysMsg); | |
| if (encodedSize(withSys) > budget) { | |
| if (!hasContent) return {}; | |
| sysMsg = null; | |
| } else { | |
| const sysBytes = encodedSize(withSys) - emptyBytes; | |
| contentBudget = budget - sysBytes; | |
| } | |
| } |
fix(memory): Codex parity — register PreToolUse and PreCompact, clamp the Codex envelope (0.28.0, issue #95)
Closes #95. Part of Workstream D (milestone "Workstream D — Right moment on every host").
What changed
hooks.codex.jsonwith the matcherBash|apply_patch— written from a live tool_name dump (codex-cli 0.153.0, Windows, 2026-09-09, capture preserved in the issue comment): shell operations emit the hook tool nameBash(the model-facing tool isexec_command), file patches emitapply_patch, and Codex treats an all-alphanumeric/pipe matcher as EXACT alternation. MCP tools (mcp__<server>__<tool>) andwrite_stdinare deliberately OUT (query derivation is shell/file-patch based).precompacthandler Claude uses. Upstream Codex dropsadditionalContexton PreCompact (decision control only, verified 2026-09-09 from codex-rs source at tag rust-v0.153.0 == main), so its functional payload on Codex is the delivery-ledger clear before compaction; post-compaction re-injection rides the registered SessionStart, which upstream fires withsource=compactafter every compaction.trigger: manual|auto— no compact_summary; [Workstream D] PR 2 of 8: Query-aware re-injection after compaction (and settle whether the PreCompact fence survives) #118 owns the shared compaction handlers). Pinned by a new absence test so it cannot silently appear.DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT = 2_500, above which Codex spills the text to a file and the model sees only a head/tail preview. The former 9000-char default sat within ~10% of the spill point. The cap binds even when an operator sets a largerZMEM_CTX_BUDGET; Claude/ZCode are untouched.test_recall_hook_fence.pyare inverted with failure messages naming [Workstream D] PR 3 of 8: Codex parity — register PreToolUse and PreCompact/PostCompact after the hook failure is attributed #95 and the re-probe convention. Deployment docs now state the per-entry trusted-hash re-approval requirement (new hook entries are silently skipped until re-approved in the TUI/hooksreview).release-manifest.json(release_gate.py --emit-manifest, 74 files, digest 775d9436).Live tool_name dump (codex-cli 0.153.0, Windows 11, 2026-09-09 — as the issue requires)
Probe method: isolated
CODEX_HOME(scratch dir, no contact with the user config), user-level capture hooks,codex exec --dangerously-bypass-hook-trust(documented flag for vetted automation), throwaway workdir. Captured PreToolUse payloads:Bash{"command": "echo zmem95-shell-probe"}apply_patch{"command": "*** Begin Patch\n*** Add File: …probe.txt\n+hello-from-patch\n*** End Patch"}Also captured: SessionStart (
source: "startup", plusmodel/permission_mode), UserPromptSubmit (prompt), PostToolUse (mirrors tool names +tool_response). MCP decision: OUT — MCP tools emitmcp__<server>__<tool>(source-verified) and their JSON arguments don't fit the query derivation. A-c model_context_window=1500run did not trigger live compaction, so compaction shapes were verified from source instead (the AC's compaction canary lane is exercised by the registered-path drive below; the force-/compact lane belongs to #118).Acceptance criteria → evidence
host_canary.py --host codex --self-testexit 0; pre-tool + precompact proven by the registered-path drive (matcher simulation + real launcher chain: PreToolUse fence carries the seeded row within the 8000-char cap; PreCompact drive clears the session ledger) — frozen checks 1-2, red pre-fix / green post-fixtest_pretool_registered_on_zcode_claude_and_codex,test_codex_json_registers_precompact— frozen checks 3-4test_memory_skill_pins_wired_codex_parity_state(needles: `BashCODEX_ENVELOPE_CAP_CHARS = 8000clamp + clamp e2e (codex ≤8000 withZMEM_CTX_BUDGET=50000; claude control unclamped) — frozen check 6release_gate.pygreen — frozen check 9Validation
node tests/test_codex_adapter.js: 94 passed, 0 failedpython -m unittest tests.test_pretool_inject tests.test_recall_hook_fence: 49 tests OKnode tests/test_launcher.js: 217 passed, 0 failed;tests.test_host_canary+tests.test_codex_manifest_contract: 25 tests OK--host codex|claude|zcode --self-test: all exit 0python scripts/release_gate.py: green at 0.28.0Deployment note
After upgrading, Codex users must re-approve the hook surface once (the two new entries carry new trusted hashes; untrusted entries are silently skipped). This is documented in SKILL.md and the CHANGELOG.
Merge status
MERGE_STATE: AWAITING_USER_APPROVAL — this PR is not merged by the agent; a separate, recorded human approval bound to the exact PR head SHA is required (issue-tracer Phase 5.1).
Waivers
None. No Full-Resolution Contract clauses waived. (The PostCompact half of the issue's registration item is dispositioned — not waived — as owned by open dependency #118, with an absence pin, SKILL pointer, and this disclosure; the compaction canary's force-/compact lane is likewise #118's scope item.)
Summary by cubic
Gives Codex parity with Claude and ZCode: Codex now fires
PreToolUseandPreCompacthooks for pre-tool hazard recall and compaction ledger-clear, and the Codex envelope is clamped below upstream's output-spill limit. Fixes issue #95.Changes
PreToolUsewith matcherBash|apply_patch, derived from a live tool-name dump (codex-cli 0.153.0); MCP tools andwrite_stdinstay excluded.PreCompactthrough the same sharedprecompacthandler Claude uses; since upstream Codex dropsadditionalContexton that event, the payload is the delivery-ledger clear before compaction, with post-compaction re-injection ridingSessionStart.PostCompactunregistered, deferred to issue [Workstream D] PR 2 of 8: Query-aware re-injection after compaction (and settle whether the PreCompact fence survives) #118, with an absence-pin test so it cannot silently reappear.ZMEM_CTX_BUDGETis larger, and an operator-set value above it warns on stderr before clamping. Claude/ZCode budgets are unchanged.systemMessage's exact encoded size from the envelope budget before content is fitted; asystemMessagethat alone cannot fit is dropped fail-open.Deployment
/hooksreview.Written for commit bb54467. Summary will update on new commits.