Commit Graph

155 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
  • 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: 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: 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 #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: 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>
  • 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
  • fix: extract inline SessionStart bootstrap to separate file (#1035)
    Inline `node -e "..."` in hooks.json contained `!` characters (e.g.
    `!org.isDirectory()`) that bash history expansion in certain shell
    environments would misinterpret, producing syntax errors and the
    "SessionStart:startup hook error" banner in the Claude Code CLI header.
    
    Extract the bootstrap logic to `scripts/hooks/session-start-bootstrap.js`
    so the shell never sees the JS source. Behaviour is identical: the script
    reads stdin, resolves the ECC plugin root via CLAUDE_PLUGIN_ROOT or a set
    of well-known fallback paths, then delegates to run-with-flags.js.
    
    Update the test that asserted the old inline pattern to verify the new
    file-based approach instead.
    
    Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
  • fix: harden Trae install ownership (#1013)
    * fix: harden trae install ownership
    
    * fix: unblock unicode safety CI lint
    
    * fix: unblock shared CI regressions
    
    * test: isolate package-manager dependent hooks and formatter tests
  • Merge pull request #926 from xingzihai/feature/pre-commit-quality-hook
    feat(hooks): add pre-commit quality check hook