Preserve complete Stop-hook stdout through lifecycle wrappers, wait for queued output to flush before exiting, bound child output with a larger explicit buffer, and add end-to-end regressions for large, multibyte, dry-run, and failure cases.
The existing OpenAI pattern sk-[a-zA-Z0-9]{20,} never matches real
Anthropic keys: their sk-ant-api03-... format contains hyphens, which
break the character class before reaching the 20-char threshold. Keys
from the fastest-growing Claude Code user base slipped through the scan.
Adds a dedicated sk-ant-[a-zA-Z0-9_-]{20,} pattern (checked before the
OpenAI one) and extends the staged-secrets test with a realistic
Anthropic key fixture.
On a case-insensitive filesystem (macOS APFS/HFS+, Windows NTFS) a write to
`.ESLINTRC.JS` lands on the exact same inode as `.eslintrc.js`, but the guard
looked the basename up in PROTECTED_FILES with a case-sensitive `Set.has`.
Every entry in that Set is lowercase, so any case-variant path missed the
branch entirely and returned exit 0 — a single Write silently overwrote a
live config while the hook reported success.
Reproduced on macOS APFS: `.eslintrc.js` and `.ESLINTRC.JS` share one inode,
yet the hook returned exit 2 for the former and exit 0 for the latter, and the
uppercase write replaced the real config's contents.
This is a one-step bypass of the whole guard and needs no shell access, unlike
the known delete-then-recreate route.
Fix: also test `basename.toLowerCase()`. All 32 PROTECTED_FILES entries are
already lowercase, so the fallback is exact. On a genuinely case-sensitive
filesystem this costs at most a false positive on a distinct file whose name
differs from a protected one by case alone.
Behaviour deliberately unchanged: first-time creation is still allowed (the
bootstrap affordance), non-config paths still pass through, and the existing
lstat/ENOENT fail-closed semantics are untouched.
Test: adds a case-variant case that asserts exit 2. It guards itself with an
inode comparison and skips on case-sensitive filesystems rather than asserting
something untrue there. Verified in both directions — it FAILS against the
unpatched hook (`Got 0; 0 !== 2`) and passes with the fix. Suite: 9/9.
* fix(hooks): remove stray '?' that made every 'yarn <anything>' trigger tmux reminder
The tmux-reminder matcher uses one alternation per package manager. Each
branch requires a subcommand (install|test) — except yarn, whose subcommand
group carried a trailing `?`:
yarn (install|test)?
That made the subcommand optional, so the branch degraded to "yarn " plus
anything: `yarn add foo`, `yarn build`, `yarn dev`, even `yarn --version`
all matched and spammed the "Consider running in tmux" hint into the
additional-context channel.
Drop the `?` so yarn matches parity with npm/pnpm/bun. Verified locally
against 14 cases (yarn install/test still fire; yarn add/build/dev/… no
longer do; npm/pnpm/bun/pytest behavior unchanged).
Fixes#2514
* test(hooks): add pre-bash-tmux-reminder regression tests
Add coverage for the tmux-reminder matcher following the auto-tmux-dev.test.js
structure — the regex-first hook now has direct regression tests for the yarn
branch fix in this PR (and for the sibling package managers, other matched
tools, TMUX bypass, and malformed input).
16 assertions total:
- fires for: yarn install, yarn test, npm install, pnpm test, bun install,
pytest tests/, cargo build
- does NOT fire for: yarn add react, yarn build, yarn dev, yarn --version,
bare `yarn`, npm run dev
- respects TMUX env var
- tolerates invalid JSON and missing command field
Verified the tests actually catch the bug: reintroducing the buggy
`yarn (install|test)?` fails 4 of the 5 yarn non-match cases (the fifth,
bare `yarn`, stays passing because even the buggy branch requires a trailing
space after yarn).
Addresses CodeRabbit review on #2517.
* test(hooks): fail loudly on spawn errors, use destructuring, split runTests
Address three CodeRabbit review notes on tests/hooks/pre-bash-tmux-reminder.test.js:
- Fail loudly on spawnSync errors: raise instead of coercing
`result.status || 0`, which would mask spawn errors, timeouts, or signal
termination as a successful exit 0 (masks legitimate test failures).
- Use destructuring (`const { TMUX, ...env } = process.env`) instead of
copy-then-`delete` so the base env is built immutably.
- Split `runTests` (was 66 lines) into small per-group helpers
(runYarnTests, runSiblingPackageManagerTests, runOtherToolTests,
runTmuxBypassTests, runEdgeCaseTests). `runTests` is now 18 lines and
purely orchestrates.
16 assertions still pass; no coverage changes.
The 4th CodeRabbit note (avoid console.log in test files) is intentionally
not adopted here — every sibling hook test in this repo
(auto-tmux-dev.test.js, bash-hook-dispatcher.test.js, block-no-verify.test.js,
etc.) writes to console.log because the project's own test runner
(tests/run-all.js) is console-log based and there is no Jest/Mocha
dependency. Diverging from the established convention in a bugfix PR is
out of scope.
* test(hooks): trim tmux reminder regression coverage
---------
Co-authored-by: Haley Chen <2022hachen@gmail.com>
* refactor(hooks): consolidate PostToolUse hooks into sync/async dispatchers
Replace 10 individual PostToolUse entries in hooks.json with two
consolidated dispatcher entries (post:dispatcher:sync /
post:dispatcher:async). The dispatcher's internal registry preserves
every hook ID, matcher, and profile, so ECC_DISABLED_HOOKS and
ECC_HOOK_PROFILE gating behave exactly as before.
Performance (Edit event, actual hooks.json commands spawned in
parallel like the harness does, median of 7 runs):
- Blocking hook latency: 81ms -> 49ms (~40% faster; 7 blocking
processes -> 1 sync dispatcher)
- Node processes per tool call: 10 -> 2 (7 blocking + 3 async
-> 1 sync + 1 async)
- observe-runner now runs in-process (~370ms) inside the async
dispatcher, which stays backgrounded (async: true, timeout 45s),
so it adds no user-facing latency.
Also:
- dashboard-web lists dispatcher-managed child hooks so the hook
inventory stays complete
- post-edit-console-warn refactored to export run() for in-process
dispatch while keeping standalone stdin behavior
- dispatcher stdin reading is multi-byte safe (StringDecoder) and
child hook exit codes propagate to the dispatcher exit code
* test(hooks): replace emoji literal with unicode escape for CI unicode safety check
* fix(hooks): adopt explicit cli() entrypoint and merge multi-hook stdout
Address Greptile review on #2494:
- Replace the non-standard 'require.main === undefined' guard with an
explicit exported cli(). The hooks.json bootstraps now call
require(s).cli(), so merely requiring the module (dashboard-web,
test runners, Jest, worker threads) can never trigger dispatch,
attach stdin listeners, or set process.exitCode.
- Replace last-writer-wins stdout with mergeHookStdout(): when several
hooks emit additionalContext envelopes they merge into a single
PostToolUse envelope; non-mergeable raw stdout keeps the last hook's
output and emits a stderr warning naming the dropped hook IDs, so
nothing is lost silently.
Also includes local formatter reformatting of the dispatcher and its
test file (no behavioral changes beyond the above).
* fix(hooks): keep post:bash:dispatcher phase reachable in minimal profile
The Greptile P1 premise was partially incorrect: sub-hooks without
explicit profiles default to standard,strict via parseProfiles()
(scripts/lib/hook-flags.js), so audit/cost logs never ran under the
minimal profile on main either — there is no user-visible regression.
However, main did spawn the bash dispatcher phase unconditionally and
let each sub-hook gate itself. Restore that semantic by opening the
outer registry gate to minimal,standard,strict so a future sub-hook
that opts into minimal is not silently blocked at the phase level.
Adds the previously missing minimal-profile async dry-run test.
* test(hooks): assert failing hook exit code propagates to real process status
Spawns the actual dispatcher subprocess with an injected failing hook
and asserts the OS-level exit status, stderr diagnostic, and suppressed
pass-through — closing the E2E gap CodeRabbit flagged on #2494.
* chore: retrigger CI (flaky windows powershell bootstrap test)
* feat: add Plan Canvas - browser annotate-and-approve review for plan artifacts
- scripts/plan-canvas.js CLI (open/await/end/stop/server; bin ecc-plan-canvas)
- loopback server + ECC-styled chrome + annotation SDK + zero-dep markdown renderer
- Approve/Request-changes verdicts wired to the /plan confirmation gate
- plan-canvas skill, /plan-canvas command, SessionStart hook surfacing open reviews
- shared scripts/lib/loopback-guard.js extracted from control-pane (API re-exported)
- 121 new tests incl. full-workflow E2E; registered in manifests, catalog, registry
Inspired by lavish-axi (https://github.com/kunchenguid/lavish-axi) by @kunchenguid;
original ECC-native implementation, not a port.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(plan-canvas): invoke via ecc-plan-canvas bin so the skill works from any project
Skill/command referenced a cwd-relative `node scripts/plan-canvas.js`, unusable
outside the ECC root. Switch to the ecc-plan-canvas bin (and $CLAUDE_PLUGIN_ROOT
fallback) and align CLI next_step hints so an agent can run it as a skill in any repo.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(plan-canvas): render Mermaid diagrams + ship Codex cross-harness surface
- markdown renderer emits <pre class="mermaid"> for ```mermaid blocks (source
entity-escaped so the browser decodes it for the renderer while blocking injection)
- artifact template loads a pinned Mermaid build only when a diagram is present,
themed to ECC dark, securityLevel strict, graceful offline fallback to source
(ECC_PLAN_CANVAS_MERMAID_URL overrides for a local mirror)
- skill teaches Mermaid-for-diagrams and states the CLI+JSON loop is harness-agnostic
- add .agents/skills/plan-canvas (Codex) with agents/openai.yaml interface manifest
- register in install-modules workflow-quality paths; docs updated
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(plan-canvas): add demo screenshot
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(ci): sync yarn.lock with new bin; add contributor checklist
- yarn.lock records the ecc-plan-canvas bin so Yarn hardened-mode install no
longer wants to modify the lockfile on public PRs
- PR template + CONTRIBUTING gain a pre-push checklist covering the lockfile
trap and the full skill/command/CLI registration surfaces
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Haley Chen <2022hachen@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The fact-forcing gate fires once per first-touched file per session. In
build-heavy sessions this costs a deny->retry round-trip on every new file,
including trees where the gate's questions ("who imports this? what schema?")
carry no signal: test files, generated artifacts, scratch dirs.
Add an opt-in, comma-separated glob allowlist read from GATEGUARD_EXEMPT_GLOBS.
A matching Edit/Write/MultiEdit target skips the first-touch gate; destructive-
Bash and routine-Bash gates are untouched. Default-off (unset => identical prior
behavior), fail-open (a malformed glob is dropped, never throws), and memoized on
the env value, matching the existing getExtraDestructiveRegex idiom.
"env": { "GATEGUARD_EXEMPT_GLOBS": "**/tests/**,**/scratchpad/**" }
Adds 4 tests; all 144 gateguard tests pass.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(session-start): make instinct injection count and confidence threshold configurable
Expose ECC_MAX_INJECTED_INSTINCTS and ECC_INSTINCT_CONFIDENCE_THRESHOLD so
operators can tune SessionStart instinct injection without editing source.
Defaults are unchanged (6 instincts, 0.7 confidence floor).
The two previously hardcoded constants become DEFAULT_-prefixed fallbacks,
resolved through getMaxInjectedInstincts() and getInstinctConfidenceThreshold(),
mirroring the existing getSessionRetentionDays() /
getSessionStartMaxContextChars() env-override pattern already in this file.
Invalid or out-of-range values fall back to the defaults.
Adds subprocess coverage in tests/hooks/hooks.test.js and documents both
variables in the README Hook Runtime Controls section.
Implements part (a) of #2371.
* fix(session-start): reject partial env values for instinct injection knobs
Parse ECC_MAX_INJECTED_INSTINCTS and ECC_INSTINCT_CONFIDENCE_THRESHOLD with
Number() (after trim) instead of parseInt/parseFloat, so malformed values
like "3.9", "6abc", or "0.7x" fall back to the default rather than silently
accepting the numeric prefix (parseInt("3.9")=3, parseFloat("0.7x")=0.7).
Adds a regression assertion that a non-integer count falls back to 6.
* fix(session-start): validate decimal grammar for instinct injection env vars
Number() still accepts non-decimal numeric syntax, so
ECC_INSTINCT_CONFIDENCE_THRESHOLD=0x1 resolved to 1 and
ECC_MAX_INJECTED_INSTINCTS=1e2 to 100. Gate each value on a strict format
(/^\d+(\.\d+)?$/ for the 0-1 threshold, /^\d+$/ for the positive-integer
count) before converting, so hex/exponent/partial values fall back to the
default. Adds regression assertions for 1e2 and 0x1.
2026-07-03 20:40:17 -07:00
devin-ai-integration[bot]GitHubDevin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>affaanDevin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(ci): resync lockfiles with package.json (eslint 10) + migrate yarn.lock to Yarn 4 format
package.json requires eslint@^10.6.0 but the committed locks pinned 9.39.2, so
npm ci aborted and Yarn 4 hardened mode rejected the stale v1-classic yarn.lock
(YN0028). Regenerate package-lock.json and rewrite yarn.lock in Yarn 4 (berry)
format so npm ci and immutable yarn installs both pass.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(ci): require clean probe exit for Windows shell/bash detection; add pyyaml dev dep
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* refactor: consolidate duplicated hook-root resolver into shared resolveEccRoot() (#2368)
The inline node -e resolver blob was duplicated ~60x across hooks.json,
command docs, and translations. Each copy inlined the full ~700-char
plugin-root search using a spread over nested array literals
(p.join(d,'plugins',...s) over [['ecc'],...]), which breaks Windows hook
execution due to shell quoting (#2368).
Collapse every copy to a 250-char locator that loads the committed
resolve-ecc-root module and delegates to resolveEccRoot() — no spread, no
nested array literals, no escaped double quotes. The real search logic now
lives in one tested module. Also route session-start-bootstrap.js through
resolveEccRoot() instead of its own duplicated reimplementation, and fix
the auto-update.md 'marketplace' (singular) typo along the way.
Guard tests updated: discovery behavior is asserted against resolveEccRoot();
the inline is asserted to delegate and to contain no Windows-fragile
constructs.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(resolve-ecc-root): restore full env-unset discovery in inline resolver
Address Greptile review on #2410: when CLAUDE_PLUGIN_ROOT is unset the
delegating inline could only load the resolver module from ~/.claude,
returning ~/.claude without ever reaching the plugin/cache search. Restore
the old inline's discovery breadth (exact plugin roots + versioned cache)
Windows-safely (no spread, nested arrays, or escaped quotes), then delegate
the authoritative decision to resolveEccRoot(). Add regression tests for
plugin-subdir and versioned-cache bootstrap with env unset.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: affaan <affaan@itomarkets.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-07-03 20:01:17 -07:00
devin-ai-integration[bot]GitHubDevin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>affaanDevin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(plan-orchestrate): detect ecc@ecc marketplace + emit ecc: agent prefix (#2316)
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(ci): resync lockfiles with package.json (eslint 10) + migrate yarn.lock to Yarn 4 format
package.json requires eslint@^10.6.0 but the committed locks pinned 9.39.2, so
npm ci aborted and Yarn 4 hardened mode rejected the stale v1-classic yarn.lock
(YN0028). Regenerate package-lock.json and rewrite yarn.lock in Yarn 4 (berry)
format so npm ci and immutable yarn installs both pass.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(ci): require clean probe exit for Windows shell/bash detection; add pyyaml dev dep
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: affaan <affaan@itomarkets.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
b3268fef (#2272) made the write-gate "confirm no existing file" item
tool-agnostic in the JS hook, but the rest of the checklist surface still
names Glob/Grep. On hosts without those tools the agent still hits a dead
tool call on:
- the edit-gate "list importers" item in the hook (scripts/hooks/gateguard-fact-force.js)
- both checklist items in all three SKILL.md copies (en, ja-JP, zh-CN)
Apply the same wording b3268fef introduced — "(search the tree — Glob/Grep,
or find/grep via Bash)" — to those five remaining spots so the whole gate is
consistent. Prose-only; no logic change.
Follow-up to #2272 / b3268fef.
Replace mechanical text extraction in session-end.js and pre-compact.js
with LLM-generated summaries using `claude -p`. Summaries now capture
design decisions, resolved bugs, changed files, and carry-over context
rather than just truncated user message snippets.
- Add scripts/lib/llm-summary.js: generateSessionSummary, extractConversationText,
getContextRemainingPct, getContextThreshold, getLLMModel
- Update scripts/hooks/session-end.js: trigger LLM when context < 20% or
every 50 messages (env-configurable via ECC_LLM_SUMMARY_*)
- Update scripts/hooks/pre-compact.js: generate LLM summary right before
compaction and write it to the active session .tmp file
- Add tests/lib/llm-summary.test.js: 18 unit tests
- Update tests/hooks/hooks.test.js: 3 integration tests for new behaviour
Recursion guard: sets ECC_SKIP_LLM_SUMMARY=1 in subprocess env so Stop
hooks fired by the claude -p subprocess do not re-enter summarisation.
Requires no ANTHROPIC_API_KEY — reuses Claude Code's own authentication.
Co-authored-by: Hiroshi Tanaka <hiroshi_tanaka@MBAM3.local>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(windows): prefer PowerShell over bash to prevent zombie process accumulation
On Windows, ECC hook scripts were spawning bash.exe (MSYS2/Git Bash) on
every tool use via findShellBinary(). These processes were not reaped by
Windows, causing 40+ zombie bash.exe/conhost.exe processes per session with
noticeable system lag.
Changes to scripts/hooks/plugin-hook-bootstrap.js:
- Add isPowerShellBin(bin) helper: basename-based detection so full paths
like C:\Windows\...\powershell.exe are handled correctly
- findShellBinary(): check BASH env var first (preserves escape hatch),
then on win32 probe pwsh.exe -> powershell.exe -> bash.exe -> bash;
use correct probe args per shell type; cache result in _cachedShell
- findBashBinary(): separate cached bash-only finder used by spawnShell
.sh fallback; skips PowerShell binaries even if BASH points to one
- spawnShell(): use isPowerShellBin() to select -NoProfile -NonInteractive
-File args for PowerShell; .sh scripts fall back to findBashBinary()
with a skip-warning if no bash found on Windows
observe-runner.js is intentionally unchanged: it always invokes observe.sh
which is bash-only; routing it through PowerShell would silently break it.
The observe.sh -> observe.js migration is tracked separately.
Fixes#2345
* fix(windows): address CodeRabbit and Greptile review comments
- Add timeout: 30000 to all spawnSync probe calls in findShellBinary and
findBashBinary to prevent hangs on broken/stalled shell candidates
- Add -ExecutionPolicy Bypass to PowerShell -File invocation to fix
execution on machines with the default Restricted policy (Win10/11)
- Add PowerShell availability skip guard to PS selection test (mirrors
existing bash skip guard)
- Fix no-bash test to keep PowerShell on PATH so the .sh fallback branch
is actually exercised rather than hitting shell-unavailable early exit
* test: add timeout to spawnSync probes in Windows test skip guards
---------
Co-authored-by: Christopher J Diamond <diamondcj@leidos.com>
* fix(hooks): guard doc-file-warning stdin listeners behind require.main
doc-file-warning.js registered process.stdin data/end listeners at module
scope while also exporting run(). run-with-flags.js require()s any hook that
exports run() for its in-process fast path, so importing this hook attached
stray stdin listeners to the dispatcher process, corrupting the PreToolUse
stdout JSON contract. This is the exact failure run-with-flags' own SAFETY
comment warns about, and 24 sibling hooks already guard against it.
- Move the stdin entrypoint into main() and gate it behind require.main === module
- pre-write-doc-warn.js now calls main() explicitly instead of relying on the
import side effect
- Add regression tests: require() attaches no stdin listeners, run()/main()
stay exported, and the pre-write-doc-warn shim still warns
* docs(hooks): add JSDoc for doc-file-warning main() entrypoint
Satisfies the docstring-coverage pre-merge check; documents the stdin
entrypoint and why it must not run on require().
On Windows, when a bare-name MCP server command (e.g. codesys-mcp-sp21-plus)
falls back to the .cmd candidate, the probe sets shell:true to work around
Node 18.20+ CVE-2024-27980. However, passing an args array alongside
shell:true causes Node to concatenate the tokens without quoting (DEP0190),
so an arg containing a space (e.g. --codesys-path "C:\Program Files\...") is
re-split by cmd.exe at every space boundary. The child process receives a
truncated path, fails to launch, and the probe declares the server unavailable,
falsely blocking every MCP tool call to that server.
Fix: add a quoteWin() helper that double-quotes any token containing whitespace
or cmd metacharacters. In the useShell branch, build a single properly-quoted
command line string and pass it as the sole argument to spawn() with no separate
args array. The else branch (shell:false, all non-.cmd commands) is unchanged.
Regression test added: on Windows, creates a .cmd shim that echoes its first
positional argument to stderr, probes it with a space-containing path arg, and
asserts the probe succeeds and the arg was not split at the space boundary.
Co-authored-by: Karstein Phobic Nyvold Kvistad <karstein.kvistad@maritimerobotics.com>
- gateguard (GHSA-4v57-ph3x-gf55): add a quote-aware detection pass that
dequotes command words and splits on UNQUOTED separators incl. newlines, so
newline-separated commands, quoted command words ('rm'/"rm"), quoted
find -exec, and sh/bash -c wrappers are all classified destructive. Additive —
existing 133 cases still pass; +7 bypass regressions + a false-positive guard
(rm inside a quoted echo arg stays allowed). 140/140.
- Windows CI: format-code.ts emitted backslash paths via path.normalize, breaking
forward-slash assertions on all Windows matrix cells — force forward slashes.
- claw.js (CodeQL #1 js/polynomial-redos): bound parseTurns input so the lazy
[\s\S]*? body can't drive O(n^2) scanning on adversarial history files.
Full suite 2852/2852; lint green.
- #2290 suggest-compact: honor ECC_CONTEXT_WINDOW_TOKENS / CLAUDE_CODE_AUTO_COMPACT_WINDOW
so 400k-window models (Opus 4.x) no longer report ~double context usage; add
override + isolation tests in transcript-context.test.js.
- #2282 install: bare-language syntax is legacy-only by design, but the error
now distinguishes a supported-but-wrong-mode target (gemini/codex/…) from a
genuinely unknown one and points to --profile/--modules/--skills.
- #2276 cost-report: the command + cost-tracking skill targeted a SQLite DB no
tracker writes. Repoint both at the real ~/.claude/metrics/costs.jsonl (JSONL,
estimated_cost_usd), reduce cumulative-per-session snapshots to latest-per-session,
and use node instead of sqlite3 for cross-platform support.
- #2272 gateguard: make the 'confirm no existing file' checklist item
tool-agnostic (Glob/Grep or find/grep via Bash) so hosts without a Glob tool
don't get a dead tool call.
Full suite 2839/2839; lint green.
* fix(gateguard): check isDestructiveFindExec on each command segment
`isDestructiveBash` called `isDestructiveFindExec` only on the raw full
command string. When the raw string starts with a non-find command (e.g.
`echo x && find . -exec rm {} \;`), `isDestructiveFindExec` checks
tokens[0] and returns false — then the per-segment loop never calls it
again, letting the destructive `find -exec rm` segment through silently.
Fix: call `isDestructiveFindExec(segment)` inside the per-segment loop so
compound commands (`&&`, `;`, `|`) cannot be used to prepend a harmless
command and bypass the find-exec destructive check.
Adds three regression tests covering `&&`, `;`, and `|` bypass patterns.
* fix(gateguard): use raw body segments for isDestructiveFindExec to close quoted-binary gap
The previous per-segment call passed quote-stripped output from
splitCommandSegments to isDestructiveFindExec, so a quoted exec binary
like find . -exec 'rm' {} \; would arrive as find . -exec {} \; and
the check would silently miss it.
Switch to splitting collectExecutableBodies output on [;|&]+ without
quote-stripping first, so the find-exec binary name is always intact
when isDestructiveFindExec inspects it. This also covers || and
background & separators that the original tests did not exercise.
Adds a regression test for the || OR-chain bypass pattern.
Addresses Greptile review comments on PR #2292.
---------
Co-authored-by: kapilvus <kapilvus@gmail.com>
- README: add a visible ## Security section (official sources, vuln reporting via SECURITY.md, GateGuard/IOC/AgentShield guardrails, security guide); make stats line a plain paragraph to clear MD028
- eslint: empty catch comment in run-with-flags.js; drop unneeded escape in github-coordination/parsing.js; remove unused execFileSync import in its test (#2236 follow-ups)
- markdownlint: wrap bare URLs in rules/vue/*.md (#2250 follow-up)
npm run lint green; full suite 2836/2836.
runHooks() returned the unmodified raw stdin (the PreToolUse/PostToolUse
input event) on stdout whenever no sub-hook produced additionalContext.
Claude Code parses a hook's stdout as JSON and validates it against the
hook-output schema, so echoing the input object
({session_id, hook_event_name, tool_name, tool_input, ...}) fails with
"Hook JSON output validation failed — (root): Invalid input" on nearly
every Bash command.
Track whether a sub-hook deliberately set stdout (string / {stdout}, e.g.
GateGuard) via a rawModified flag and emit '' in the pass-through case
instead of the echoed input. Preserves GateGuard pass-through and
block-no-verify's exit-2 blocking.
Update the three dispatcher tests that codified the buggy echo behavior to
expect empty stdout, and add a regression test for a plain pass-through
command.
Fixes#2239
Co-authored-by: WOZCODE <contact@withwoz.com>
- suggest-compact hook now reads the latest usage record from the session
transcript and suggests /compact at a window-scaled token threshold
(160k/200k window, 250k/1M window; COMPACT_CONTEXT_THRESHOLD and
COMPACT_CONTEXT_INTERVAL overridable), re-firing per 60k-token growth
bucket; tool-call count stays as the secondary signal (#2155)
- Codex repo marketplace now points at ./plugins/ecc instead of ./ — Codex
never discovers plugins whose local marketplace source.path is the
marketplace root (verified on Codex CLI 0.137.0); plugins/ecc is a thin
folder referencing root skills/.mcp.json per maintainer direction on
#2097; docs flag plugin mode as experimental with the upstream blocker
openai/codex#26037 linked (#2128)
- README badges for installs/stars/forks now use shields endpoint badges
backed by api.ecc.tools (live install count 3,712 vs the stale static
150), which also eliminates shields' 'Unable to select next GitHub token
from pool' render in the stars badge
Closes#2155Closes#2128
* fix(hooks): fail open on oversized stdin instead of echoing truncated JSON (#2222)
run-with-flags.js capped stdin at 1MB but every fallthrough path still
echoed the truncated string to stdout. The harness parses hook stdout as
JSON, got a document cut mid-stream, and blocked the tool call — so any
Edit/Write with a >1MB hook payload was permanently blocked by every
registered pre-write hook, before ECC_HOOK_PROFILE / ECC_DISABLED_HOOKS
gating could run.
- Exit 0 with empty stdout (no opinion) when the stdin cap trips, before
any echo or gating logic.
- Flush stdout via write callback before process.exit: exiting right
after stdout.write() dropped everything past the ~64KB pipe buffer,
cutting even sub-cap pass-through payloads mid-JSON.
Regression tests cover the enabled, disabled, and missing-arg paths for
oversized payloads plus full echo of sub-cap >64KB payloads.
* fix(codex): stop emitting invalid exa url entry, align merge with connector policy (#2224)
The Codex MCP merge declared exa with a url key, but Codex's
[mcp_servers.*] TOML schema is stdio-only — the url key makes the
entire config.toml fail to load, bricking both the codex CLI and the
desktop app. Every install/update re-injected the line because the
urlEntry branch treated the broken entry as present.
- ECC_SERVERS now emits only the current default set per
docs/MCP-CONNECTOR-POLICY.md: chrome-devtools (stdio, command/args).
Retired servers (supabase, playwright, context7, exa, github, memory,
sequential-thinking) are never re-emitted; existing user-managed
entries are untouched.
- The merge now repairs the exact ECC-emitted broken form (url-only
exa entry) on every run so re-running the installer fixes broken
configs instead of preserving them. User stdio exa entries
(command + mcp-remote) are left alone.
- check-codex-global-state.sh requires chrome-devtools instead of the
retired set, and flags url-only exa entries with a repair hint.
Tests cover repair, re-run idempotence, stdio-entry preservation, and
no-retired-server emission in add, update, dry-run, and disabled modes.
* fix(hooks): never echo truncated stdin from Stop hooks (#2090)
Stop hooks follow the ECC pass-through convention (echo stdin on
stdout), but every echoing Stop hook capped stdin and echoed the capped
string. The Stop payload carries last_assistant_message, so a long
final assistant message produced a JSON document cut mid-stream on
stdout, which the harness reports as 'Stop hook error: JSON validation
failed' across the whole Stop chain.
Reproduced: a Stop payload with a >64KB last_assistant_message run
through run-with-flags + cost-tracker emitted exactly 65536 bytes of
invalid JSON (cost-tracker capped stdin at 64KB — far below realistic
Stop payloads).
- cost-tracker: raise the cap to 1MB (matching all other hooks) and
suppress the pass-through echo when stdin was truncated.
- check-console-log, stop-format-typecheck, desktop-notify: suppress
the echo when stdin was truncated; flush stdout before process.exit
so sub-cap payloads are not cut at the ~64KB pipe buffer.
- All hooks keep exiting 0 (fail-open); diagnostics go to stderr.
New stop-hooks-stdout test asserts the contract for every registered
Stop hook: stdout is empty or valid JSON, exit code 0 — for realistic
100KB payloads and oversized >1MB payloads, via the production runner
and via direct invocation. Updated the old hooks.test.js case that
codified the truncated-echo behavior.
* fix(hooks): dampen GateGuard fact-force repetition in long sessions (#2142)
In long autonomous sessions the fact-force gate produced 10+
near-identical 'state facts -> blocked -> restate -> retry' blocks in
one context window, which measurably raises the odds of the model
collapsing into a degenerate single-token repetition loop.
- Track a per-session fact_force_denials counter in GateGuard state
(merged max across concurrent writers, reset with the session, robust
to malformed on-disk values).
- The first GATEGUARD_FACT_FORCE_FULL_DENIALS denials (default 3) keep
the full four-fact block; later denials emit a condensed single-line
message that carries the denial ordinal, so consecutive denials are
structurally different and never textually identical.
- True retries of the same target remain allowed without re-prompting
(unchanged). Destructive-Bash and routine-Bash gates are unchanged,
as are the ECC_GATEGUARD=off / ECC_DISABLED_HOOKS escape hatches.
Eight new tests cover budget counting, condensed format, ordinal
advancement, retry pass-through, env tuning, malformed state, MultiEdit
dampening, and destructive-gate exemption.
* fix(hooks): keep security hooks able to block on oversized stdin (#2222)
Refine the truncation fail-open: instead of skipping the hook entirely,
the runner now suppresses only its own raw-echo when stdin was
truncated. The hook still executes and receives the truncated flag
(run() context / ECC_HOOK_INPUT_TRUNCATED), so config-protection keeps
blocking truncated protected-config payloads (its test requires exit 2)
while pass-through hooks fail open with empty stdout as before.
* style: apply repo formatter to touched hook files
ROOT CAUSE: hooks load plugin-hook-bootstrap.js via
`node -e "...; process.argv.splice(1,0,s); require(s)"`. On Node 21+,
require.main is `undefined` under --eval, so the `if (require.main === module)`
guard was false and main() never ran — every plugin hook silently no-op'd
(e.g. the MCP-health PreToolUse hook stopped blocking). CI (Node 18/20) hid
this; it only surfaces on Node 21+. Fix: also run main() when require.main is
undefined (the eval-bootstrap case), while staying dormant on real imports.
Also clears pre-existing main debt the full local suite enforces:
- catalog:sync — README/docs agent+skill counts drifted after recent merges
- tests/ci/supply-chain-watch-workflow: update checkout SHA to the merged v6.0.3 (#2183)
- markdownlint + check-unicode-safety --write across docs/skills
Suite: 2683/2683 green under Node v25; lint + unicode clean.
Co-authored-by: ECC Test <ecc@example.test>
* feat: auto-isolate ECC memory data for Cursor via ECC_AGENT_DATA_HOME
Add ECC_AGENT_DATA_HOME (defaults to ~/.claude) with Cursor-aware resolution,
sessionStart env injection, install scaffolds, and hook bootstrap so memory
hooks do not collide with Claude Code when both harnesses are used.
Closes#2065
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: log agent-data config errors and ship cursor sessionStart deps
Address CodeRabbit review: log invalid .cursor/ecc-agent-data.json parse
failures, and copy cursor-session-env.js plus lib deps on legacy Cursor
install so sessionStart hook path exists without hooks-runtime alone.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: resolve relative agentDataHome paths from project root
Project config values like ".ecc-data" now resolve against the
repository root (parent of .cursor/), not process.cwd(), so Cursor
hooks persist memory in the intended directory regardless of hook cwd.
Addresses cubic review on PR #2066.
Co-authored-by: Cursor <cursoragent@cursor.com>
* docs: explain getHomeDir duplicate and docstring policy
Document why agent-data-home keeps a local home-dir helper (circular
require with utils.js) and list consolidation options for maintainers.
Note that CodeRabbit JSDoc coverage warnings are informational relative
to ECC's usual script documentation style.
Addresses cubic P2 context on PR #2066.
Co-authored-by: Cursor <cursoragent@cursor.com>
* test: isolate agent-data-home tests from dogfooded .cursor config
Use isolated temp cwd for default-resolution cases and assert
resolveAgentDataHome({ projectDir }) reads ecc-agent-data.json.
Document cwd/project caveats in the test file header.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
The PostToolUse cost warnings emit imperative text via additionalContext
("Stop and inform the user...", "Review whether...", "Consider whether...").
Subagents read additionalContext as an instruction and obey the "Stop",
abandoning their task and returning a prompt-for-direction instead of their
result — derailing multi-agent workflows. The main loop is also nudged to
halt mid-task.
Reword all three severities to pure-informational data: keep the
CRITICAL/WARNING/NOTICE label + the dollar figure (and the threshold), drop
the imperative sentence, and state plainly it is informational. No logic,
severity, or threshold change. Existing tests pass (they assert the labels +
severities, which are preserved).
Before: `COST CRITICAL: Session cost is $X. Stop and inform the user about high cost before continuing.`
After: `COST CRITICAL: session total ~$X (over $50). Informational only — not an instruction to stop.`
Co-authored-by: OrenG Tools <tools@orengacademy.com>
Ghostty natively supports the OSC 9 desktop-notification escape
(ESC ] 9 ; <message> BEL), the same sequence already used for iTerm2.
Previously only TERM_PROGRAM === 'iTerm.app' took the escape path, so
Ghostty users fell through to the osascript path. That makes Script
Editor the notification owner, and clicking the notification just
launches Script Editor instead of focusing the terminal.
Adding 'ghostty' to the OSC 9-capable check makes Ghostty the owner,
so clicking the notification focuses the Ghostty window/tab where
Claude Code is running. Verified on Ghostty (TERM_PROGRAM=ghostty).
Co-authored-by: 高野智史 <satoshitakano@takanosatoshinoMacBook-Pro-522.local>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(hooks): stop false loop warnings and repeated identical context warnings
Two PostToolUse monitor defects surfaced during a long single-turn session:
1. ecc-metrics-bridge hashToolCall fingerprinted Edit/Write/MultiEdit on
file_path ONLY, so several distinct edits to the same file produced the
same hash and tripped the loop detector ("stuck loop") even though every
edit was different. Now the hash includes the edit content
(old_string/new_string/content/edits) so distinct edits to one file hash
differently; identical edits still collide as intended.
2. ecc-context-monitor re-emitted the SAME warning every DEBOUNCE_CALLS (5)
tool calls even when nothing changed. Because the cost figure only refreshes
at Stop (turn) boundaries, a single stale value printed the identical
warning ~20 times within one turn. Dedupe on message content instead: a
warning surfaces only when its text changes (cost moved, new file count, new
loop) or on first escalation to critical, and is otherwise suppressed.
Adds regression tests for the same-file/different-content hash case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(hooks): address CodeRabbit review (#2121)
- ecc-context-monitor: clear dedupe state when warnings resolve, so the same
warning text recurring in a later turn (context dips/recovers/dips, a loop
that stops then restarts) is surfaced again instead of suppressed as a
duplicate. Guarded so the no-warning hot path stays write-free.
- ecc-metrics-bridge: hash the FULL serialized edit payload and truncate the
digest, not the input. Slicing the serialized string to HASH_INPUT_LIMIT
first could collapse large edits sharing their first 2048 chars, reviving the
false-loop collision for big Write/edit payloads.
- Add regression test for >2048-char edit divergence.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(gateguard): gate force/path git checkout as destructive
The destructive-command gate's `checkout` handler only flagged
`git checkout -- <path>`. It missed `git checkout --force` / `-f <branch>`
and `git checkout .`, all of which discard uncommitted working-tree changes,
so they bypassed the gate (once the once-per-session routine-Bash gate is
satisfied, they ran with no challenge). The sibling `switch` handler already
covers these force forms; mirror it for `checkout`.
* test(gateguard): document Test 7b force-checkout case
---------
Co-authored-by: bymle <229636660+bymle@users.noreply.github.com>
`DEV_PATTERN`'s trailing `\b` treats a hyphen as a word boundary, so
`dev\b` matched the `dev` prefix of distinct npm scripts like
`dev-setup` / `dev-docs` / `dev-build` and blocked them with exit 2.
Replace the trailing `\b` with `(?![\w-])` so the dev server still
matches (`dev`, `dev;`, `dev:ssr`) but `dev-<suffix>` scripts pass.
Adds regression tests for dev-setup/dev-docs/dev-build (allowed) and
dev:ssr (still blocked).
Co-authored-by: bymle <229636660+bymle@users.noreply.github.com>
The regenerated summary block embeds raw user-message text and was passed
as the *replacement* argument to String.prototype.replace, where $-sequences
($&, $$, $`, $') are special. A user message containing $& re-injected the
entire matched block (duplicating the summary markers) and $$ collapsed to $,
silently corrupting the persisted session summary. buildSummarySection only
escapes newlines and backticks, not $.
Fix: use function replacers (() => summaryBlock) at both rewrite sites so the
replacement text is treated literally. Adds an end-to-end regression test.
Co-authored-by: bymle <229636660+bymle@users.noreply.github.com>
* fix(session-start): support ECC_SESSION_RETENTION_DAYS opt-out + document env var
The retention pass for *-session.tmp files (issue #2151) landed previously,
but the env var that controls it was undocumented in the README and rejected
falsy values (0, off, disabled), silently falling back to the 30-day default.
Users who want to keep all sessions for forensic or research workflows had no
way to opt out.
This patch:
- Extends getSessionRetentionDays() so 0|off|false|disabled|never|none disables
pruning entirely (returns null sentinel; default behavior unchanged).
- Updates the call site in main() to skip pruneExpiredSessions when retention
is null and emits a clear "[SessionStart] Pruning disabled via
ECC_SESSION_RETENTION_DAYS" log line so the operator can tell pruning is off.
- Documents ECC_SESSION_RETENTION_DAYS in the README "Hook Runtime Controls"
section alongside the other ECC_SESSION_* knobs.
- Adds three regression tests in tests/hooks/hooks.test.js covering opt-out
via 0, opt-out via off, and garbage-value fallback to default 30.
Verification:
- node tests/hooks/hooks.test.js — 240/240 green (incl. 3 new retention tests)
- node tests/run-all.js — 2622/2622 green
- npx eslint scripts/hooks/session-start.js tests/hooks/hooks.test.js — clean
- node scripts/ci/validate-no-personal-paths.js — clean
- node scripts/ci/check-unicode-safety.js — clean
- node scripts/ci/validate-hooks.js — 28 matchers validated
- node scripts/ci/validate-rules.js — 115 files validated
Fixes#2151
* docs(readme): list all ECC_SESSION_RETENTION_DAYS opt-out values + add Windows example
Address reviewer feedback on PR #2163:
- CodeRabbit and cubic both flagged that the README docs only listed 3 of 6
opt-out values accepted by getSessionRetentionDays() (0, off, disabled),
while the implementation also accepts false, never, none.
- cubic also flagged the missing Windows PowerShell example for the new
variable, breaking the parallel structure of the existing
ECC_CONTEXT_MONITOR_COST_WARNINGS example block.
Updated the README to:
- Spell out all six opt-out values (0, off, false, disabled, never, none)
and clarify they "keep all sessions (disable pruning)".
- Add an ECC_SESSION_RETENTION_DAYS line to the Windows PowerShell example.
No behavior change. README only.
Verification:
- npx markdownlint README.md — clean
- npx eslint scripts/hooks/session-start.js tests/hooks/hooks.test.js — clean
* feat(gateguard): add env knobs for routine bash gate + extra destructive patterns
The JS port of gateguard-fact-force has two bash gates: a destructive
gate (rm -rf, drop table, git push --force, etc.) that operators want
to keep, and a once-per-session routine gate that fires on the very
first bash invocation regardless of intent. Operators on hosts where
the routine gate is friction without signal (Cursor, OpenCode, etc.)
have been maintaining local patches that get clobbered on every plugin
update; the Python upstream gateguard-ai already exposes equivalent
config via .gateguard.yml.
Adds two env vars, both off-by-default so existing behavior is
preserved:
- GATEGUARD_BASH_ROUTINE_DISABLED — truthy values (1, true, on, yes,
enabled) skip the routine bash gate. Destructive gate is unaffected.
- GATEGUARD_BASH_EXTRA_DESTRUCTIVE — regex source string for additional
destructive patterns. Matches against the same quote-stripped,
subshell-flattened command the built-in DESTRUCTIVE_SQL_DD regex sees,
so a custom phrase inside $(...) or backticks is also caught. A
malformed regex is logged once to stderr and treated as not configured
rather than crashing the hook (hooks must never block tool execution
unexpectedly).
Twelve new tests pin both env vars (truthy aliases, falsy values, unset
baseline, destructive-gate-still-fires, alternation members, malformed
regex degrades safely, custom phrase inside command substitution).
Existing 2619/2619 tests still pass; eslint clean.
Fixes#2078
* fix(gateguard): reset extra-destructive warn-once gate when env value changes
Both reviewers (CodeRabbit + cubic) flagged that
extraDestructiveWarnLogged was never reset when GATEGUARD_BASH_EXTRA_DESTRUCTIVE
flipped from one invalid regex to a different invalid regex. The
sticky boolean meant a long-running process saw bad-pattern-a's
warning then silently swallowed bad-pattern-b's parse failure.
Fix: clear extraDestructiveWarnLogged whenever the cache key changes
(i.e. before the regex compile attempt). The warn-once-per-distinct-
pattern invariant now matches the per-key cache invariant.
Adds a same-process regression test via loadDirectHook() that spies on
process.stderr.write and asserts: same bad pattern warns once across
multiple invocations; switching to a different bad pattern emits a
second warning; switching to a valid regex emits zero warnings.
* fix(suggest-compact): clean up old counter temp files
claude-tool-count-<sessionId> files were written into the OS temp dir
on every hook run and never removed, accumulating one orphan per
session indefinitely.
Sweep stale counter files at the top of main() before opening the
active counter. Retention is env-tunable via COMPACT_STATE_TTL_DAYS
(default 14 days); invalid values fall back to the default. The
active session's counter file is preserved unconditionally even if
its mtime is past the cutoff. Failures during the sweep are swallowed
to preserve the always-exit-0 hook contract.
Adds 7 regression tests covering the sweep, env-var validation, and
the always-exit-0 invariant under a populated temp dir.
Fixes#2156
* fix(suggest-compact): preserve counter files at the TTL cutoff boundary
The cleanup sweep used `mtimeMs > cutoffMs` to short-circuit, which
matched files whose mtime sits exactly on the cutoff boundary and
deleted them. The cleanupOldCounters docstring promises only files
*older than* retentionDays are removed; a file at age == retentionDays
is not older than retentionDays, so it must survive.
Switch the comparison to `>=` so only strictly older files fall
through to deletion. Add a regression test that pins boundary-aged
files (mtimeMs sitting just past the projected cutoff) are preserved.
Refs #2156
Mirror the previous commit's Windows-EPERM retry on the companion
`writeWarnState` in `scripts/hooks/ecc-context-monitor.js`. Same
race: two PostToolUse subprocesses writing concurrent debounce
state racing on `MoveFileExW`, target-in-use throwing EPERM on
Windows even though each writer's tmp path is now unique.
Implementation: import `renameWithRetry` from `scripts/lib/session-bridge.js`
(exported in the previous commit) instead of duplicating the helper.
The retry policy, backoff schedule, and main-thread `Atomics.wait`
strategy stay identical to `writeBridgeAtomic`.
Three writers in the repo now share the same atomic-write contract:
- `writeBridgeAtomic` (scripts/lib/session-bridge.js) — round 1 +
this round's retry
- `writeWarnState` (this file) — round 1 + this round's retry via shared helper
- `writeCostWarningIfChanged` (scripts/hooks/ecc-metrics-bridge.js) —
out of scope for this PR (already uses unique tmp suffix; a future
consolidation could move it to the shared helper too).
Local: `yarn test` green, `yarn lint` clean. The companion test
suite for `ecc-context-monitor.js` does not currently exercise
concurrent `writeWarnState` writes, but the helper it now uses is
covered by the `tests/lib/session-bridge.test.js` concurrent-write
regression added in round 1's last commit.
Mirror the previous commit's `writeBridgeAtomic` fix on the
companion `writeWarnState` in `ecc-context-monitor.js`. Same shape:
fixed `${target}.tmp` → `${target}.${process.pid}.${randomNonce}.tmp`,
plus best-effort cleanup of the tmp file on `renameSync` failure
(throws original error after cleanup).
`writeWarnState` debounces the context-monitor's threshold alarms
(`COST_NOTICE_USD`, `COST_WARNING_USD`, `COST_CRITICAL_USD`, plus the
context-remaining and loop-detection ones). Without unique suffixes,
two PostToolUse subprocesses racing on the warn-state file produce
either a corrupted JSON debounce-state on disk or an ENOENT throw
that the hook catches and swallows — either way the next warn-state
read returns the default `{callsSinceWarn: 0, lastSeverity: null}`
and the threshold alarms re-fire or stop firing erratically. Users
see warning messages flicker or vanish; debounce no longer works.
Three call sites in this repo now share the same atomic-write
contract:
- `writeBridgeAtomic` (scripts/lib/session-bridge.js) — primary
- `writeCostWarningIfChanged` (scripts/hooks/ecc-metrics-bridge.js) — cost cache
- `writeWarnState` (this file) — debounce state
`yarn lint` clean. Regression test covering both `writeBridgeAtomic`
and `writeWarnState` under concurrent load lands in the next commit.
greptile P2 nitpick: the previous commit's docblock said "on every
PreToolUse hook" but the module header (and the actual hook wiring
in `hooks/hooks.json`) identifies this script as a PostToolUse
hook — it runs *after* each tool invocation to update the running
session aggregate. One-word typo, no behavior change.
coderabbitai flagged: the two `catch` blocks in `readSessionCost`
silently swallowed every failure mode. A malformed `costs.jsonl`
row, a permission error opening the file, or any other unexpected
I/O failure would silently return zero cost — masking real
problems and feeding stale or zero numbers into
`ecc-context-monitor.js` (which then injects them as
`additionalContext` into the live model turn).
Fix two things, both fail-open-preserving:
1. **Inner JSON.parse catch** — count malformed lines and write
one aggregated breadcrumb per call:
[ecc-metrics-bridge] skipped N malformed line(s) in <path>
Aggregating (rather than per-line) keeps a log-flooded
`costs.jsonl` diagnosable without overwhelming stderr.
2. **Outer fs.readFileSync catch** — write a breadcrumb on real
errors, but stay silent on `ENOENT`. The "no costs.jsonl yet"
case is genuinely normal (no Stop event has fired this session)
and producing noise on every PreToolUse before the first Stop
would be reviewer-visible spam. All other error codes
(`EACCES`, `EISDIR`, `EMFILE`, …) get:
[ecc-metrics-bridge] failing open after <name> reading <path>: <msg>
In both cases the function still returns the zero-cost fallback
so the bridge never breaks tool execution — only the
diagnosability changes.
Two new regression tests in
`tests/hooks/ecc-metrics-bridge.test.js`:
✓ readSessionCost writes a stderr breadcrumb when malformed
lines are skipped — feeds 4 rows (2 valid, 2 malformed),
asserts the last valid row still wins AND captured stderr
contains "skipped 2 malformed line(s)".
✓ readSessionCost stays silent when costs.jsonl does not exist
(ENOENT) — uses a fresh tmp HOME with no metrics dir, asserts
zero return AND empty stderr.
Test count: 16 → 18; `npm test` green; `yarn lint` clean.