Commit Graph

169 Commits

  • fix(hooks): scan full costs.jsonl when locating session row
    `readSessionCost` read only the trailing 8 KiB of
    `~/.claude/metrics/costs.jsonl` to "avoid scanning entire file".
    That ceiling is the opposite-sign sibling of the double-count bug
    fixed in the previous commit: once a session's most recent
    cumulative row gets pushed past the 8 KiB window by newer rows
    from other sessions, the bridge silently reports `totalCost: 0`,
    `totalIn: 0`, `totalOut: 0` for that session — same false signal
    to `ecc-context-monitor.js`, same wrong number injected into the
    live model turn as `additionalContext`.
    
    `cost-tracker.js` has no rotation policy, so on any non-trivial
    workstation costs.jsonl grows past 8 KiB within minutes of normal
    use. For users who keep multiple concurrent sessions, this means
    the second-and-later sessions silently report zero almost
    immediately.
    
    Reproduced before this commit:
    
      $ HOME=/tmp/eccc node -e '
          const fs = require("fs");
          const m = require("./scripts/hooks/ecc-metrics-bridge.js");
          // S1 row at file start, then 200 rows of OTHER-session noise (~16 KiB).
          // S1 is the row we want, but it sits past the 8 KiB tail.
          const s1 = `{"session_id":"S1","estimated_cost_usd":0.5,"input_tokens":500,"output_tokens":250}`;
          const other = `{"session_id":"OTHER","estimated_cost_usd":1,"input_tokens":100,"output_tokens":50}`;
          fs.mkdirSync("/tmp/eccc/.claude/metrics", { recursive: true });
          fs.writeFileSync("/tmp/eccc/.claude/metrics/costs.jsonl",
            [s1, ...Array(200).fill(other)].join("\\n") + "\\n");
          console.log(JSON.stringify(m.readSessionCost("S1")));'
      {"totalCost":0,"totalIn":0,"totalOut":0}
    
    Expected: `{"totalCost":0.5, "totalIn":500, "totalOut":250}` (the
    S1 row that exists in the file).
    Actual: zero — the row is past the 8 KiB tail.
    
    Fix: drop the `fs.openSync` + bounded `fs.readSync` + position
    arithmetic in favour of `fs.readFileSync(costsPath, 'utf8')` and
    iterate every line. Each row is ~150 bytes; even 100k rows is
    ~15 MB and a single sync read on PreToolUse is in the low ms.
    If file rotation lands in `cost-tracker.js` later, this scan
    becomes proportionally cheaper.
    
    After this commit the reproduction above returns
    `{"totalCost":0.5, "totalIn":500, "totalOut":250}`.
    
    Regression test in `tests/hooks/ecc-metrics-bridge.test.js`:
    `readSessionCost finds session row beyond the old 8 KiB tail
    boundary`. The test asserts the costs.jsonl fixture is > 8 KiB
    before reading so any reintroduction of a bounded tail would
    re-fail the test (i.e. the assertion is the contract, not the
    specific number 8192).
    
    Together with the previous commit, both directions of the
    metrics-bridge cost-reporting bug are closed.
  • fix(hooks): use last cumulative row for session cost in metrics bridge
    `ecc-metrics-bridge.js#readSessionCost` summed the
    `estimated_cost_usd`, `input_tokens`, and `output_tokens` of
    every matching row in `~/.claude/metrics/costs.jsonl`. That breaks
    the documented contract of `scripts/hooks/cost-tracker.js`, which
    explicitly states (in its module docblock):
    
      Cumulative behavior: Stop fires per assistant response, not
      per session. Each row therefore represents the cumulative
      session total up to that point. To get per-session cost, take
      the last row per session_id.
    
    Summing N cumulative rows over-counts by roughly (N+1)/2 ×. For a
    session with 3 rows at 0.01, 0.02, 0.03 USD (true running total
    0.03), the bridge today reports 0.06 USD. The over-counted value
    feeds `ecc-context-monitor.js`, which then trips its
    COST_NOTICE_USD / COST_WARNING_USD / COST_CRITICAL_USD thresholds
    on phantom spend AND injects the inflated number as
    `additionalContext` into the live model turn — so the agent
    itself is told a wrong cost.
    
    Reproduced on `main` before this commit:
    
      $ cat > /tmp/eccc/.claude/metrics/costs.jsonl <<EOF
      {"session_id":"S1","estimated_cost_usd":0.01,"input_tokens":333,"output_tokens":166}
      {"session_id":"S1","estimated_cost_usd":0.02,"input_tokens":666,"output_tokens":333}
      {"session_id":"S1","estimated_cost_usd":0.03,"input_tokens":1000,"output_tokens":500}
      EOF
    
      $ HOME=/tmp/eccc node -e 'const m = require("./scripts/hooks/ecc-metrics-bridge.js"); \
          console.log(JSON.stringify(m.readSessionCost("S1")))'
      {"totalCost":0.06,"totalIn":1999,"totalOut":999}
    
    Expected: `{"totalCost":0.03,"totalIn":1000,"totalOut":500}` (the
    last cumulative row).
    Actual: 2× over-count.
    
    Fix: replace `+=` with `=` in the matching branch so the assigned
    values reflect the most recent row encountered. The iteration
    order is file order, which is also event time order, so the last
    assignment wins — exactly the contract cost-tracker writes
    against.
    
    After this commit the reproduction above returns
    `{"totalCost":0.03,"totalIn":1000,"totalOut":500}`.
    
    Regression test in `tests/hooks/ecc-metrics-bridge.test.js`:
    `readSessionCost returns the LAST cumulative row, not the sum
    (cost-tracker contract)`. The existing
    `readSessionCost does not include unrelated default-session rows`
    test happened to pass even with the bug because it only had one
    target-session row — single-row sessions are coincidentally
    correct under both formulas. The new test uses three rows so the
    two formulas diverge.
    
    A second issue in the same function — the 8 KiB tail-only read
    silently drops older rows once a session's recent cumulative
    totals scroll past that window — is fixed in the next commit.
  • chore: update statusline ANSI color palette
    - Replace blinking red (5;31m) with bold red (1;31m) for critical context bar
    - Replace cyan metrics (36m) with sky blue (38;5;117m)
    - Replace plain bold task (1m) with bold bright white (1;97m)
    - Update test assertion to match new bold red code
  • fix(hooks): close grouped command bypasses in gateguard (#1912)
    Inspect executable bodies inside plain subshells and brace groups before applying destructive command classifiers.\n\nCo-authored-by: Jamkris <82251632+Jamkris@users.noreply.github.com>
  • fix: integrate recent hook and docs PRs (#1905)
    Integrates useful changes from #1882, #1884, #1889, #1893, #1898, #1899, and #1903:
    - fix rule install docs to preserve language directories
    - correct Ruby security command examples
    - harden dev-server hook command-substitution parsing
    - add Prisma patterns skill and catalog/package surfaces
    - allow first-time protected config creation while blocking existing configs
    - read cost metrics from Stop hook transcripts
    - emit suggest-compact additionalContext on stdout
    
    Co-authored-by: Jamkris <dltmdgus1412@gmail.com>
    Co-authored-by: Levi-Evan <levishantz@gmail.com>
    Co-authored-by: gaurav0107 <gauravdubey0107@gmail.com>
    Co-authored-by: richm-spp <richard.millar@salarypackagingplus.com.au>
    Co-authored-by: zomia <zomians@outlook.jp>
    Co-authored-by: donghyeun02 <donghyeun02@gmail.com>
  • fix: close block-no-verify bypass holes
    Backport Jamkris's fix for case-insensitive core.hooksPath overrides and the git commit -tn template-path false positive. Verified locally on current main with 25/25 block-no-verify tests and node tests/run-all.js passing 2369/2369.
  • feat: add ECC statusline observability hooks
    Salvages the useful statusline/context monitor work from stale PR #1504 while preserving the current continuous-learning hook runner wiring.
    
    Adds the metrics bridge, context monitor, statusline script, shared cost/session bridge utilities, and tests. Fixes the reviewed false loop-detection hash collision for non-file tools, avoids default-session cost inflation, sanitizes statusline task lookup, and records hook payload session IDs in cost-tracker.
  • fix: port continuous-learning observer fixes
    Ports continuous-learning observer signal, storage, remote normalization, and v1 deprecation fixes onto current main.
  • fix: port hook session and dashboard safety fixes
    Ports suggest-compact session_id isolation and dashboard terminal/document launch safety onto current main.
  • fix(hooks): resolve MCP health-check spawn ENOENT on Windows (#1456)
    * fix(hooks): resolve MCP health-check spawn ENOENT on Windows
    
    On Windows, commands like 'npx' are batch files (npx.cmd) that require
    shell expansion to resolve via PATH. Without shell: true, Node.js
    spawn() fails with ENOENT.
    
    However, absolute paths (e.g. C:\Program Files\nodejs\node.exe) must
    NOT use shell mode because cmd.exe misparses paths containing spaces.
    
    Fix: enable shell mode only for non-absolute commands on Windows, using
    path.isAbsolute() to distinguish. This matches how attemptReconnect()
    already handles the shell option.
    
    Fixes #1455
    
    * fix(hooks): harden Windows shell spawn — validate command for metacharacters
    
    Addresses bot review feedback on PR #1456:
    
    - Add UNSAFE_SHELL_CHARS regex to guard against shell injection when
      needsShell=true: cmd.exe operators (&, |, <, >, ^, %, !, (), ;,
      whitespace) are rejected before shell mode is enabled
    - Add typeof command === 'string' check so path.isAbsolute() cannot
      throw on malformed non-string command values
    - Rename test to 'via PATH resolution' (not Windows-only; runs all platforms)
    - Fix misleading test comment: 'node' resolves via PATH like npx.cmd but
      does not itself use .cmd; comment now accurately reflects the intent
    
    * fix(hooks): kill full process tree on Windows when shell mode is used
    
    When needsShell=true, the spawned child is cmd.exe. Calling child.kill()
    only terminates the shell, leaving the real server process orphaned.
    
    Use taskkill /PID <pid> /T /F on Windows+shell to kill the entire
    process tree rooted at cmd.exe. Fall back to SIGTERM+SIGKILL on all
    other platforms or when shell mode is not active.
    
    * fix(hooks): fall back to child.kill() when taskkill fails
    
    Windows taskkill can fail if it's not on PATH, the process already
    exited, or permissions are denied. Previously the failure was silently
    ignored and no kill signal reached the child.
    
    Now: capture the spawnSync result and fall back to child.kill('SIGKILL')
    on any taskkill error or non-zero status. This still may leak a
    detached server process but at least guarantees the cmd.exe shell is
    signaled.
  • fix: install native Cursor hook and MCP config (#1543)
    * fix: install native cursor hook and MCP config
    
    * fix: avoid false healthy stdio mcp probes
  • Merge pull request #1495 from ratorin/fix/session-end-transcript-path-isolation
    fix(hooks): isolate session-end.js filename using transcript_path UUID (#1494)
  • fix(hooks): wrap SessionStart summary with stale-replay guard (#1536)
    The SessionStart hook injects the most recent *-session.tmp as
    additionalContext labelled only with 'Previous session summary:'.
    After a /compact boundary, the model frequently re-executes stale
    slash-skill invocations it finds inside that summary, re-running
    ARGUMENTS-bearing skills (e.g. /fw-task-new, /fw-raise-pr) with the
    last ARGUMENTS they saw.
    
    Observed on claude-opus-4-7 with ECC v1.9.0 on a firmware project:
    after compaction resume, the model spontaneously re-enters the prior
    skill with stale ARGUMENTS, duplicating GitHub issues, Notion tasks,
    and branches for work that is already merged.
    
    ECC cannot fix Claude Code's skill-state replay across compactions,
    but it can stop amplifying it. Wrap the injected summary in an
    explicit HISTORICAL REFERENCE ONLY preamble with a STALE-BY-DEFAULT
    contract and delimit the block with BEGIN/END markers so the model
    treats everything inside as frozen reference material.
    
    Tests: update the two hooks.test.js cases that asserted on the old
    'Previous session summary' literal to assert on the new guard
    preamble, the STALE-BY-DEFAULT contract, and both delimiters. 219/219
    tests pass locally.
    
    Tracked at: #1534
  • fix(gateguard): rewrite routineBashMsg to use fact-presentation pattern (#1531)
    * fix(gateguard): rewrite routineBashMsg to use fact-presentation pattern
    
    The imperative 'Quote user's instruction verbatim. Then retry.' phrasing
    triggers Claude Code's runtime anti-prompt-injection filter, deadlocking
    the first Bash call of every session. The sibling gates (edit, write,
    destructive) use multi-point fact-list framing that the runtime accepts.
    
    Align routineBashMsg with that pattern to restore the gate's intended
    behavior without changing run(), state schema, or any public API.
    
    Closes #1530
    
    * docs(gateguard): sync SKILL.md routine gate spec with new message format
    
    CodeRabbit flagged that skills/gateguard/SKILL.md still described the
    pre-fix imperative message. Update the Routine Bash Gate section to
    match the numbered fact-list format used by the new routineBashMsg().
  • review: broaden CLAUDE_TRANSCRIPT_PATH fallback to cover missing/empty JSON fields
    Previously the env fallback ran only when JSON.parse threw. If stdin was valid
    JSON but omitted transcript_path or provided a non-string/empty value, the
    script dropped to the getSessionIdShort() fallback path, re-introducing the
    collision this PR targets.
    
    Validate the parsed transcript_path and apply the env-var fallback for any
    unusable value, not just malformed JSON. Matches coderabbit's outside-diff
    suggestion and keeps both input-source paths equivalent.
    
    Refs #1494
  • review: apply sanitizeSessionId to UUID shortId, fix test comment
    - Route the transcript-derived shortId through sanitizeSessionId so the
      fallback and transcript branches remain byte-for-byte equivalent for any
      non-UUID session IDs that still land in CLAUDE_SESSION_ID (greptile P1).
    - Clarify the inline comment in the first regression test: clearing
      CLAUDE_SESSION_ID exercises the transcript_path branch, not the
      getSessionIdShort() fallback (coderabbit P2).
    
    Refs #1494
  • review: address P1/P2 bot feedback on shortId derivation
    - Use last-8 chars of transcript UUID instead of first-8, matching
      getSessionIdShort()'s .slice(-8) convention. Same session now produces the
      same filename whether shortId comes from CLAUDE_SESSION_ID or transcript_path,
      so existing .tmp files are not orphaned on upgrade.
    - Normalize extracted hex prefix to lowercase to avoid case-driven filename
      divergence from sanitizeSessionId()'s lowercase output.
    - Explicitly clear CLAUDE_SESSION_ID in the first regression test so the env
      leak from parent test runs cannot hide the fallback path.
    - Add regression tests for the lowercase-normalization path and for the case
      where CLAUDE_SESSION_ID and transcript_path refer to the same UUID (backward
      compat guarantee).
    
    Refs #1494
  • fix(hooks): isolate session-end.js filename using transcript_path UUID
    When session-end.js runs and CLAUDE_SESSION_ID is unset, getSessionIdShort()
    falls back to the project/worktree name. If any other Stop-hook in the chain
    spawns a claude subprocess (e.g. an AI-summary generator using 'claude -p'),
    the subprocess also fires the full Stop chain and writes to the same project-
    name-based filename, clobbering the parent's valid session summary with a
    summary of the summarization prompt itself.
    
    Fix: when stdin JSON (or CLAUDE_TRANSCRIPT_PATH) provides a transcript_path,
    extract the first 8 hex chars of the session UUID from the filename and use
    that as shortId. Falls back to the original getSessionIdShort() when no
    transcript_path is available, so existing behavior is preserved for all
    callers that do not set it.
    
    Adds a regression test in tests/hooks/hooks.test.js.
    
    Refs #1494
  • Merge pull request #1445 from affaan-m/fix/plugin-installed-hook-root-resolution
    fix: resolve plugin-installed hook root on marketplace installs
  • Merge pull request #1367 from ozoz5/feat/gateguard
    feat(hooks,skills): add gateguard fact-forcing pre-action gate
  • fix: 5 bugs + 2 tests from 3-agent deep bughunt
    Bugs fixed:
    - B1: JS gate messages still said "cat one real record" -> redacted/synthetic
    - B2: Destructive bash key used 200-char truncation (collision bypass) -> SHA256 hash
    - B3: sanitizePath only stripped \n\r -> now strips null bytes, bidi overrides, all control chars
    - B4: Tool name matching was case-sensitive (latent bypass) -> lookup map normalization
    - B5: SKILL.md Gate Types missing MultiEdit -> added with explanation
    
    Tests added:
    - T1: MultiEdit gate denies first unchecked file (CRITICAL - was untested)
    - T2: MultiEdit allows after all files gated
    
    11/11 tests pass.
    
    Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  • fix: cubic-dev-ai round 2 — 3 issues across SKILL.md + pruning
    P1: Gate message asked for raw production data records — changed to
        "redacted or synthetic values" to prevent sensitive data exfiltration
    
    P2: SKILL.md description now includes MultiEdit (was missing after
        MultiEdit gate was added in previous commit)
    
    P2: Session key pruning now caps __prefixed keys at 50 to prevent
        unbounded growth even in theoretical edge cases
    
    9/9 tests pass.
    
    Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  • fix: remove unnecessary disk I/O + fix test cleanup
    - isChecked() no longer calls saveState() — read-only operation
      should not write to disk (was causing 3x writes per tool call)
    - Test cleanup uses fs.rmSync(recursive) instead of fs.rmdirSync
      which failed with ENOTEMPTY when .tmp files remained
    
    9/9 tests pass.
    
    Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  • fix: P1 test state-file PID mismatch + P2 session key eviction
    P1 (cubic-dev-ai): Test process PID differs from spawned hook PID,
    so test was seeding/clearing wrong state file. Fix: pass fixed
    CLAUDE_SESSION_ID='gateguard-test-session' to spawned hooks.
    
    P2 (cubic-dev-ai): Pruning checked array could evict __bash_session__
    and other session keys, causing gates to re-fire mid-session. Fix:
    preserve __prefixed keys during pruning, only evict file-path entries.
    
    9/9 tests pass.
    
    Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>