Commit Graph

138 Commits

  • 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>
  • fix: MultiEdit gate bypass — handle edits[].file_path correctly
    P1 bug reported by greptile-apps: MultiEdit uses toolInput.edits[].file_path,
    not toolInput.file_path. The gate was silently allowing all MultiEdit calls.
    
    Fix: separate MultiEdit into its own branch that iterates edits array
    and gates on the first unchecked file_path.
    
    9/9 tests pass.
    
    Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  • fix: session-scoped state to prevent cross-session race
    Addresses reviewer feedback from @affaan-m:
    
    1. State keyed by CLAUDE_SESSION_ID / ECC_SESSION_ID
       - Falls back to pid-based isolation when env vars absent
       - State file: state-{sessionId}.json (was .session_state.json)
    
    2. Atomic write+rename semantics
       - Write to temp file, then fs.renameSync to final path
       - Prevents partial reads from concurrent hooks
    
    3. Bounded checked list (MAX_CHECKED_ENTRIES = 500)
       - Prunes to last 500 entries when cap exceeded
       - Stale session files auto-deleted after 1 hour
    
    9/9 tests pass.
    
    Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  • Merge pull request #1384 from KeWang0622/fix/lint-md028-eqeqeq
    fix: resolve markdownlint MD028 + ESLint eqeqeq lint failures
  • Merge pull request #1385 from KeWang0622/fix/block-no-verify-hook
    fix: route block-no-verify hook through run-with-flags.js
  • fix: detach ecc2 background session runners (#1387)
    * fix: detach ecc2 background session runners
    
    * fix: stabilize windows ci portability
    
    * fix: persist detached runner startup stderr
    
    * fix: prefer repo-relative hook file paths
    
    * fix: make npm pack test shell-safe on windows
  • fix: address PR review comments on block-no-verify hook
    - Add `minimal` profile so the security hook runs in all profiles
    - Scope -n/--no-verify flag check to the detected subcommand region,
      preventing false positives on chained commands (e.g. `git log -n 10`)
    - Guard stdin listeners with `require.main === module` so require()
      from run-with-flags.js does not register unnecessary listeners
    - Verify subcommand token is preceded only by flags/flag-args after
      "git", preventing misclassification of argument values as subcommands
    - Add integration tests for block-no-verify hook
    
    Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  • fix: route block-no-verify hook through run-with-flags.js
    Replace inline `npx block-no-verify@1.1.2` with a standalone Node.js
    script routed through `run-with-flags.js`, matching every other hook.
    
    Fixes two bugs:
    1. npx inherits the project cwd and triggers EBADDEVENGINES in
       pnpm-only projects that set devEngines.packageManager.onFail=error.
    2. The hook bypassed run-with-flags.js so ECC_DISABLED_HOOKS had no
       effect — the isHookEnabled() check never ran.
    
    The new script replicates the full block-no-verify@1.1.2 detection
    logic (--no-verify, -n shorthand for commit, core.hooksPath override)
    with zero external dependencies.
    
    Closes #1378
  • fix: resolve markdownlint MD028 and ESLint eqeqeq warnings
    Fix two lint issues that cause `npm run lint` to exit non-zero:
    
    1. README.md (MD028): Two consecutive blockquotes separated by a bare
       blank line. Markdownlint treats this as one blockquote with an
       illegal blank line inside. Replace the blank line with a `>`
       continuation so both paragraphs stay in the same blockquote.
    
    2. session-activity-tracker.js (eqeqeq): Three instances of `== null`
       replaced with explicit `=== null || === undefined` guards to satisfy
       the repo's `eqeqeq: warn` ESLint rule.
    
    Closes #1366
  • fix: gate MultiEdit tool alongside Edit/Write
    MultiEdit was bypassing the fact-forcing gate because only Edit and
    Write were checked. Now MultiEdit triggers the same edit gate (list
    importers, public API, data schemas) before allowing file modifications.
    
    Updated both the hook logic and hooks.json matcher pattern.
    
    Addresses coderabbit/greptile/cubic-dev: "MultiEdit bypasses gate"
    
    Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
  • fix: allow destructive bash retry after facts presented
    Destructive bash gate previously denied every invocation with no
    isChecked call, creating an infinite deny loop. Now gates per-command
    on first attempt and allows retry after the model presents the required
    facts (targets, rollback plan, user instruction).
    
    Addresses greptile P1: "Destructive bash gate permanently blocks"
    
    Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
  • fix: address P2 review feedback (coderabbitai, cubic-dev-ai)
    - GATEGUARD_STATE_DIR env var for test isolation (hook + tests)
    - Exit code assertions on all 9 tests (no vacuous passes)
    - Non-vacuous allow-path assertions (verify pass-through preserves input)
    - Robust newline-injection assertion
    - clearState() now reports errors instead of swallowing
    
    Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
  • fix: address P1 review feedback from greptile bot
    1. Use run-with-flags.js wrapper (supports ECC_HOOK_PROFILE, ECC_DISABLED_HOOKS)
    2. Add session timeout (30min inactivity = state reset, fixes "once ever" bug)
    3. Add 9 integration tests (deny/allow/timeout/sanitize/disable)
    
    Refactored hook to module.exports.run() pattern for direct require() by
    run-with-flags.js (~50-100ms faster per invocation).
    
    Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
  • feat(hooks,skills): add gateguard fact-forcing pre-action gate
    A PreToolUse hook that forces Claude to investigate before editing.
    Instead of self-evaluation ("are you sure?"), it demands concrete facts:
    importers, public API, data schemas, user instruction.
    
    A/B tested: +2.25 quality points (9.0 vs 6.75) across two independent tasks.
    
    - scripts/hooks/gateguard-fact-force.js — standalone Node.js hook
    - skills/gateguard/SKILL.md — skill documentation
    - hooks/hooks.json — PreToolUse entries for Edit|Write and Bash
    
    Full package with config: pip install gateguard-ai
    Repo: https://github.com/zunoworks/gateguard
    
    Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
  • perf(hooks): batch format+typecheck at Stop instead of per Edit (#746)
    * perf(hooks): batch format+typecheck at Stop instead of per Edit
    
    Fixes #735. The per-edit post:edit:format and post:edit:typecheck hooks
    ran synchronously after every Edit call, adding 15-30s of latency per
    file — up to 7.5 minutes for a 10-file refactor.
    
    New approach:
    - post-edit-accumulator.js (PostToolUse/Edit): lightweight hook that
      records each edited JS/TS path to a session-scoped temp file in
      os.tmpdir(). No formatters, no tsc — exits in microseconds.
    - stop-format-typecheck.js (Stop): reads the accumulator once per
      response, groups files by project root and runs the formatter in
      one batched invocation per root, then groups .ts/.tsx files by
      tsconfig dir and runs tsc once per tsconfig. Clears the accumulator
      immediately on read so repeated Stop calls don't double-process.
    
    For a 10-file refactor: was 10 × (15s + 30s) = 7.5 min overhead,
    now 1 × (batch format + batch tsc) = ~5-30s total.
    
    * fix(hooks): address race condition, spawn timeout, and Windows path guard
    
    Three issues raised in code review:
    
    1. Race condition: switched accumulator from non-atomic JSON
       read-modify-write to appendFileSync (one path per line). Concurrent
       Edit hook processes each append independently without clobbering each
       other. Deduplication moved to the Stop hook at read time.
    
    2. Effective timeout: added run() export to stop-format-typecheck.js so
       run-with-flags.js uses the direct require() path instead of falling
       through to spawnSync (which has a hardcoded 30s cap). The 120s
       timeout in hooks.json now governs the full batch as intended.
    
    3. Windows path guard: added spaces and parentheses to UNSAFE_PATH_CHARS
       so paths like "C:\Users\John Doe\project\file.ts" are caught before
       being passed to cmd.exe with shell: true.
    
    * fix(hooks): fix session fallback, stale comment, trim verbose comments
    
    - Replace 'default' session ID fallback with a cwd-based sha1 hash so
      concurrent sessions in different projects don't share the same
      accumulator file when CLAUDE_SESSION_ID is unset
    - Remove stale "JSON file" reference in accumulator header (format is
      now newline-delimited plain text)
    - Remove redundant/verbose inline comments throughout both files
    
    * fix(hooks): sanitize session ID, fix Windows tsc, proportional timeouts
    
    - Sanitize CLAUDE_SESSION_ID with /[^a-zA-Z0-9_-]/g before embedding in
      the temp filename so crafted separators or '..' sequences cannot escape
      os.tmpdir() (cubic P1)
    - Fix typecheckBatch on Windows: npx.cmd requires shell:true like
      formatBatch already does; use spawnSync and extract stdout/stderr from
      the result object (coderabbit P1)
    - Proportional per-batch timeouts: divide 270s budget across all format
      and typecheck batches so sequential runs in monorepos stay within the
      Stop hook wall-clock limit (greptile P2)
    - Raise Stop hook timeout from 120s to 300s to give large monorepos
      adequate headroom (cubic P2)
    
    * fix(hooks): extend accumulator to Write|MultiEdit, fix tests
    
    - Extend matcher from Edit to Edit|Write|MultiEdit so files created with
      Write and all files in a MultiEdit batch are included in the Stop-time
      format+typecheck pass (cubic P1)
    - Handle tool_input.edits[] array in accumulator for MultiEdit support
    - Rename misleading 'concurrent writes' test to clarify it tests append
      preservation, not true concurrency (cubic P2)
    - Add Stop hook dedup test: writes duplicate paths to accumulator and
      verifies the hook clears it cleanly (cubic P2)
    - Add Write and MultiEdit accumulation tests
    
    * fix(hooks): move timeout to command level, add dedup unit tests
    
    - Move timeout: 300 from the matcher object to the hook command object
      where it is actually enforced; the previous position was a no-op
      (cubic P2)
    - Extract parseAccumulator() and export it so tests can assert dedup
      behavior directly without relying only on side effects (cubic P2)
    - Add two unit tests for parseAccumulator: deduplication and blank-line
      handling; rename the integration test to match its scope
    
    * fix(hooks): replace removed format/typecheck hooks with accumulator in cursor adapter