mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-08 07:37:48 +02:00
5064474d4d762dc9640234a41617cccb79185cec
28
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
afa0b35649 |
fix(continuous-learning-v2): warn when the observer never survives a hook invocation (#2489) (#2606)
* fix(continuous-learning-v2): warn when the observer never survives a hook invocation (#2489) The observer is lazy-started from a hook process that exits immediately afterwards. start-observer.sh's liveness check runs inside that still-living process tree, so it always sees a healthy observer and prints "Observer started (PID: N)". On native Windows (Git Bash/MSYS2) the reap happens later, when the hook's Job Object closes, so no self-check placed in start-observer.sh can ever observe the failure. The next hook invocation is the only place the death is visible, and _CHECK_OBSERVER_RUNNING already found it there -- then discarded it, deleting the stale PID file and restarting silently, once per tool call, forever. Users were left with an observer-start.log full of success lines and an observer that never completed a single analysis cycle. Record the "well-formed PID that is no longer alive" case, count consecutive non-survivals in ${PROJECT_DIR}/.observer-nosurvive-count, and log one explanatory warning when the streak reaches ECC_OBSERVER_NOSURVIVE_WARN_AFTER (default 3). Warning fires on equality so a persistent failure logs once per streak rather than once per tool call; finding the observer alive resets the streak. The Windows-specific explanation is gated on uname so Linux/macOS users are pointed at observer.log instead of a wrong diagnosis. Counting happens in the caller, not inside _CHECK_OBSERVER_RUNNING, because that function is invoked once per PID file and again under the start lock. The PowerShell backgrounding rewrite is deliberately not included: it cannot be exercised on a non-Windows machine, and untested process-spawning code is a worse outcome than an accurate diagnostic. * docs(continuous-learning-v2): state observer platform support and the new warn threshold The observer's Windows limitation was only discoverable by hitting it. Record it next to observer.enabled, where it is read before the flag is set, and document ECC_OBSERVER_NOSURVIVE_WARN_AFTER so the knob added alongside the warning does not repeat the undocumented-env-var problem tracked in #2573. zh-TW is intentionally left alone: translation parity is not enforced here and the repo rejects blind translation imports without translator review. * fix(continuous-learning-v2): serialize the non-survival streak under the lazy-start lock observe.sh runs on every tool call, so the streak read-modify-write could race between concurrent invocations -- losing an increment or logging the warning twice. That is the same class of bug the signal counter hit in #2296, and this repo's rule is to never fall back to an unlocked read-modify-write. Rather than add a second lock, move the increment into _START_OBSERVER_LOGGED. All three of its call sites already run inside the lazy-start lock (flock / lockfile / mkdir), so the update is serialized with no new machinery. Counting at the restart instead of at detection also means N racing hooks record one death rather than N. The reset stays in the caller: it is an idempotent unlink, not a read-modify-write, so it needs no lock. Adds a regression case pinning the increment inside _START_OBSERVER_LOGGED and asserting all three call sites remain locked. * fix(continuous-learning-v2): harden the non-survival threshold and warning output Three review findings on the #2489 diagnostic: - An all-zero threshold silently disabled it. `00` passes a digits-only check but compares as zero, and the streak only grows, so the warning could never fire. Normalize with base-10 arithmetic and fall back to the default for anything below 1. Base 10 is forced explicitly because a leading zero would otherwise be read as octal, and `08` is an arithmetic error that would abort the hook under `set -e`. The same normalization now guards the streak read. - An unwritable log silently swallowed the diagnostic. Build the message once and fall back to stderr when the append fails. This cannot spam: the block runs once per streak, not once per tool call. The counter write keeps its `|| true` -- observe.sh runs on every tool call and the repo rule is that hooks exit 0 on non-critical errors, so a full disk must not break tool use. - The live-PID test fixture used process.pid, which is 1 in a container and is deliberately rejected by _CHECK_OBSERVER_RUNNING; the reset case would then fail for the wrong reason. Use a spawned child and clean it up. Adds a regression case for the all-zero threshold. Verified on bash 3.2 (the macOS CI runner shell) as well as bash 5. * fix(continuous-learning-v2): warn only on a persisted streak increment If the counter write fails, the file stays below the threshold, so every later hook invocation rereads it, re-increments in memory, hits the equality check and warns again -- turning the once-per-streak diagnostic into once-per-tool- call spam. That is worse in exactly the case the stderr fallback added in the previous commit was meant to cover, since a disk that cannot take the log usually cannot take the counter either. Gate the warning on the write succeeding. The write stays non-fatal: it runs as an `if` condition, so `set -e` is satisfied and an unwritable counter costs a delayed diagnostic rather than a broken tool call. Tests: an unwritable counter must stay silent across repeated invocations while the hook still exits 0, and a leading-zero threshold ("08") must be read as decimal -- "00" alone did not exercise the base-10 conversion, since it is zero either way. --------- Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com> |
||
|
|
01e15490f0 |
fix(skill-evolution): wire Skill PostToolUse tracker so skill-health shows real runs (#2490)
* fix(skill-evolution): wire Skill PostToolUse tracker so skill-health shows real runs (#2463) recordSkillExecution() had no production callers, so ~/.claude/state/skill-runs.jsonl was never written and `scripts/skills-health.js --dashboard` always reported 0 runs. Adds scripts/hooks/skill-run-tracker.js and registers it as an async PostToolUse hook (matcher: Skill) in posttooluse-dispatcher.js, which is now the single PostToolUse entrypoint on main. Addresses the privacy and bounds review on #2555's sibling PR: - No prompt text is persisted. task_description is synthesized as "Skill invocation: <skill_id>"; tool_input.task_description/description/ prompt are never read. - Every persisted string is bounded and charset-restricted. A skill id is an identifier, so free text, newlines, or an over-long value are dropped rather than truncated and written through. - The JSONL sink is created 0600 and re-tightened on each append, repairing files written before this bound existed. - The sink is capped at MAX_RUN_RECORDS (5000), trimmed oldest-first, so the append-only file can no longer grow without limit. Tests cover the privacy guarantee (no prompt text reaches a record), the identifier bounds, the file mode on POSIX, and the retention cap. * fix(skill-evolution): re-register the tracker for PostToolUseFailure The rebase onto current main dropped the hooks.json entry, which silently resurrected the P1 from the earlier review round: deriveOutcome() still branches on hook_event_name === 'PostToolUseFailure', but the PostToolUse dispatcher does not fan that event out, so the branch was unreachable in production. Hard Skill failures were dropped from telemetry entirely, which inflates the dashboard success rate — the opposite of what #2463 asks for. Restores the dedicated PostToolUseFailure entry (matcher Skill, id post:skill:track, same run-with-flags wrapper and standard,strict gating as the dispatcher registration). Verified end-to-end: a PostToolUseFailure payload piped through run-with-flags now records outcome "failure". Adds a regression test asserting the registration so a future rebase cannot quietly drop it again. --------- Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com> |
||
|
|
5987bd4dc6 |
feat(session-start): rank injected instincts by project/stack relevance (#2466)
* feat(session-start): rank injected instincts by project/stack relevance Instinct selection at SessionStart ranked purely by confidence, so a high-confidence instinct about an unrelated stack could take an injection slot from a lower-confidence instinct that is actually relevant to the current project. Rank by confidence + location/stack relevance instead: project-scoped instincts, and instincts whose domain/trigger matches the detected stack (languages/frameworks via detectProjectType, plus terraform/dbt markers), get a small additive boost. The confidence>=threshold floor and the injection cap are unchanged, and ranking degrades to confidence-only when nothing matches or when ECC_INSTINCT_RELEVANCE_RANKING is set to off. The ranking helpers live in scripts/lib/instinct-relevance.js with unit coverage in tests/lib/, plus an end-to-end ordering test in tests/hooks/. Completes part (b) of #2371; part (a) (configurable count + threshold) shipped in #2413. Fixes #2371 * refactor(session-start): drop redundant confidence tiebreaker in instinct sort Greptile flagged that the secondary `right.confidence` comparison in summarizeActiveInstincts' sort was dead code when relevance ranking is disabled and, when enabled, was reached only on a floating-point tie of the combined score — where it skipped the intended scope-label tiebreaker. Remove it: the primary combined-score comparison already reduces to confidence-only ordering when relevance is off, so behavior there is unchanged; a genuine combined-score tie now falls through to the documented scope-first, then id, order. * test: isolate instinct relevance environment --------- Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com> |
||
|
Gaurav DubeyandGitHub
|
5debb798c8 |
fix(continuous-learning-v2): honor non-git CLAUDE_PROJECT_DIR (#2488)
Honor an explicit non-git CLAUDE_PROJECT_DIR as an isolated project scope, canonicalize and hash it consistently in the shell observer and Python CLI, and preserve the global fallback for arbitrary non-git working directories. |
||
|
Gaurav DubeyandGitHub
|
c714dc5654 |
fix(resolve-ecc-root): require ECC skills, not just scripts, before accepting a root (#2544) (#2577)
* fix(resolve-ecc-root): require ECC skills, not just scripts, before accepting a root (#2544) resolveEccRoot() accepted a candidate root on script-only evidence (scripts/lib/utils.js). A partial install that lands ECC's scripts into ~/.claude but not ECC's skills short-circuited at the standard-install branch, so skill-resolving callers built skills/... paths against a root where they do not exist and every command failed three layers away. For the default probe (skill consumers, reached via INLINE_RESOLVE) a candidate now qualifies only if it contains both the script tree and a sentinel ECC skill; the same stricter check guards the plugin-root and plugin-cache branches. An explicit caller probe is still honored exactly, so script consumers (e.g. session-start-bootstrap, which probes for the hook runner) are unaffected. Merely checking that skills/ exists is insufficient — a user's own ~/.claude/skills/ can be present with none of ECC's skills. Adds a regression test for the exact partial-install scenario and updates the resolver test fixtures to build complete roots. * test(resolve-ecc-root): cover partial exact-plugin and cache roots; DRY skill sentinel (#2544) Address CodeRabbit review on PR #2577: - Extend #2544 regression coverage to the exact-plugin and versioned plugin-cache branches, asserting the stricter both-sentinels predicate rejects a scripts-only root there too (not only for ~/.claude). - Extract the ECC_SKILL_SENTINEL constant in command-plugin-root.test.js and reuse it at both fixture setup sites instead of duplicating the literal. |
||
|
Gaurav Dubey
|
fafc832238 |
test(opencode): reject any namespace-scoped command agent id, not just the legacy prefix (#2477)
Address review: the frontmatter guard only rejected the `everything-claude-code:` prefix, so a hypothetical future `<other-plugin>:agent` value could slip through. opencode registers its inline agents unscoped, so assert the id carries no `:` at all — rejecting the whole scoped class — alongside the existing registered-agent-map membership check. |
||
|
Gaurav Dubey
|
9d1ecb0754 |
fix(opencode): resolve command agent ids to registered opencode agents (#2477)
The `.opencode/commands/*.md` frontmatter referenced agents with the Claude Code plugin namespace (`agent: everything-claude-code:<name>`), but ECC's opencode integration registers its agents unscoped in `opencode.json`'s `agent` map (`code-reviewer`, `planner`, ...), and that file's own `command` section already references them unscoped. The `everything-claude-code:` scope resolves under no opencode config (the opencode plugin package is `ecc-universal`, and inline-config agents are bare), so subtask commands like `/code-review` hard-fail with `Agent not found: everything-claude-code:code-reviewer`. Non-subtask commands fall back to the default agent and appear to work — which is why only some commands failed. Strip the `everything-claude-code:` prefix from all 30 command frontmatter agent ids so they match the registered agents, fix the MIGRATION.md example, and replace the test that enforced the broken scoped invariant with one that asserts each command agent id is a registered opencode agent (fails on the old scoped ids, passes on the fix). Fixes #2477 |
||
|
Gaurav DubeyandGitHub
|
8b6543929e |
chore(continuous-learning-v2): standardize shell shebangs to env bash (#2401)
* chore(continuous-learning-v2): standardize shell shebangs to env bash Three scripts under skills/continuous-learning-v2/ used the hardcoded `#!/bin/bash` shebang while the other four already used the portable `#!/usr/bin/env bash`: - hooks/observe.sh (runs on every hook invocation) - scripts/detect-project.sh - agents/start-observer.sh The hardcoded interpreter path fails to execute on systems where bash is not installed at /bin/bash (NixOS, some Homebrew layouts, FreeBSD). Standardize all three to `#!/usr/bin/env bash`, matching the repo-wide majority convention, and add a regression test that asserts shebang uniformity for every shell script in this skill so the inconsistency cannot reappear. Fixes #2303 * test(continuous-learning-v2): harden shebang test runner Address review feedback on the shebang-consistency regression test: - firstLine() now splits on /\r?\n/ so a script checked out with CRLF line endings does not leave a trailing carriage return that would break the shebang comparison on Windows. - The test() helper now surfaces the full error (stack trace, not just the message) on failure and writes pass/fail lines via process.stdout/stderr so diagnostics are preserved. * test(continuous-learning-v2): skip hidden dirs in shebang scan The recursive shell-script scan now skips hidden directories (e.g. the observer's runtime `.observer-tmp`). This keeps the shebang-consistency check deterministic: only committed skill scripts are examined, and an untracked local artifact left over from an observer run can no longer cause a false failure. |
||
|
Gaurav DubeyandGitHub
|
ff4a06dd91 |
feat(session-start): make instinct injection count and confidence threshold configurable (#2413)
* 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. |
||
|
Gaurav DubeyandGitHub
|
0a926b45ba |
fix(install): rewrite relative skill links for injected ecc namespace (#2399)
* fix(install): rewrite relative skill links for injected ecc namespace Skill and rule markdown is byte-copied during a claude install, but the home/project adapters inject an `ecc/` namespace segment (skills/<id> -> skills/ecc/<id>, rules/<x> -> rules/ecc/<x>). Source-relative links such as `../../rules/react/hooks.md` therefore broke after install: the extra level changed what `../..` resolved to, and the link target itself moved under rules/ecc/. Rewrite relative links in namespaced markdown so they resolve to the file's actual installed location, derived from the plan's own file placements (no hardcoded namespace literal, so the links cannot drift from where files land). Non-namespacing adapters and links to non-installed targets are left untouched; URLs, anchors, absolute paths, and fenced code blocks are never rewritten. Fixes #2340 * fix(install): keep non-namespaced markdown on the byte-for-byte copy path Address review feedback: the markdown branch in applyInstallPlan diverted every copy-file markdown operation through read+rewrite+write, so identity-mapped markdown (source path == install path, no namespace injected) lost byte-for-byte content and source mode bits even though no link rewrite was needed. Gate the rewrite on isNamespacedSource() so only files whose install path actually changed (e.g. skills/x -> skills/ecc/x) leave the copyFileSync path; everything else is copied verbatim as before. * test(install): emit failure stack in the link-rewrite test runner Address review feedback: the local test() harness logged only error.message, so a failing assertion lost its source line and diff. Print error.stack on stderr on failure so broken rewrite cases stay diagnosable. |
||
|
Gaurav DubeyandGitHub
|
c8c83ef428 |
fix(install): dedupe copy-file operations sharing a destination (#2429)
OpenCode ships override command files under .opencode/commands/ that shadow the generic commands/*.md sources. The manifest install plan recorded both writes against the same destination, so `ecc doctor` reported perpetual drift for the 29 shadowed command files and `ecc repair` "fixed" that phantom drift by copying the generic source over the correct override, corrupting the installed command while never clearing the warning. Dedupe copy-file operations by destination in createManifestInstallPlan, keeping the last writer to match the sequential apply order. install, repair, and doctor all consume this one builder, so a fresh install is clean and a single repair rewrites drifted state green. Fixes #2414 |
||
|
Gaurav DubeyandGitHub
|
f720885cea |
fix(clv2): archive observations only after successful analysis in observer-loop (#2386)
analyze_observations moved observations.jsonl into observations.archive/
unconditionally, even when the Claude analysis failed (timeout, non-zero
exit, rate limit). Because the analyzer only reads the live file, a failed
batch was archived and never re-analyzed, silently dropping the instincts
it would have produced.
Return early on a non-zero analysis exit so the archive mv runs only on
success, retaining observations for the next cycle to retry. Resolve the
script's own directory from ${BASH_SOURCE[0]} (SCRIPT_DIR) so sibling
scripts (session-guardian.sh) and relative helpers resolve correctly under
both execution and sourcing, and add a source-guard so observer-loop.sh can
be sourced without starting the loop. Add a regression test covering both
the failure (retain) and success (archive) paths.
Fixes #2370
|
||
|
Gaurav DubeyandGitHub
|
a36148fff9 |
test(clv2): add coverage for instinct-cli prune, projects ops, promote dry-run, normalize-url (#2374)
* test(clv2): cover instinct-cli prune, projects ops, promote dry-run, normalize-url Add pytest coverage for previously-untested functions in skills/continuous-learning-v2/scripts/instinct-cli.py: - _normalize_remote_url: scp/https/file forms, credential + .git stripping, network lowercasing, case-preserving local paths, idempotence - _promote_specific dry-run: returns 0 and writes no global file - projects delete/gc/merge: invalid-id, not-found, dry-run, and force paths over registry + storage, asserting destructive ops are gated - cmd_prune: dry-run keeps files; non-dry-run deletes only expired; quiet Test-only change; no production code modified. Fixes #2302 * test(clv2): assert dry-run storage no-op and quiet-mode stderr silence Address CodeRabbit review on #2374: - projects gc/merge dry-run tests now also assert on-disk storage is untouched (empty1 project dir survives; nothing copied into dest personal), closing the gap where a storage-mutating dry-run regression would still pass. - cmd_prune quiet test now asserts stderr is empty too, not just stdout. * test(clv2): cover merge missing-destination and prune empty-pending branches |
||
|
Gaurav DubeyandGitHub
|
a6d12ec21e |
fix(clv2): surface SIGALRM timeout drops in observe.sh (#2373)
* fix(clv2): surface SIGALRM timeout drops in observe.sh The inline-Python observation writers in observe.sh arm a signal.SIGALRM alarm (8s) so they self-terminate before the async hook's 10s timeout can orphan them (#2278). The handler _ecc_bail called sys.exit(0) with no logging, so when the alarm fired the in-flight observation was silently dropped: nothing was logged, no partial write occurred, and the shell saw a clean exit. There was no way to detect or count how many observations were being lost. Add a single stderr visibility line to both _ecc_bail handlers (the parse-error fallback path and the main observation-writing path) before sys.exit(0), using the repo's "[observe]" log prefix. Exit code stays 0: in a Claude Code hook a non-zero exit signals a block, so changing it would turn an internal timeout into a user-facing tool block. The warning goes to stderr (not stdout) because both blocks redirect stdout into the observations file. Add tests/hooks/observe-signal-timeout.test.js: a static regression guard that every _ecc_bail handler logs to stderr before exiting and keeps exit 0, plus a behavioral check that runs the real handler text extracted from observe.sh and confirms a fired alarm exits 0 and emits the [observe] warning on stderr only. Fixes #2300 * test(clv2): exercise both _ecc_bail handlers end-to-end The behavioral SIGALRM-fire test ran only handlers[0] (the parse-error fallback path); the main observation-write path (handlers[1]) was covered only by the static regex guard. The write path is the higher-value one to verify end-to-end since it carries valid, parseable data that would succeed given more time, so a silent drop there is the worst case. Loop the behavioral check over every extracted handler so a regression that silenced the second handler's stderr write is caught at runtime, not just by the static guard. * test(clv2): select timeout handlers by marker, not array index The behavioral check looped over all extracted _ecc_bail handlers by index. If an unrelated _ecc_bail were ever added to observe.sh, the loop would either test the wrong block or be diluted. Filter the handlers to those carrying the "[observe] SIGALRM timeout" marker so the live SIGALRM check stays pinned to the two #2300 timeout handlers regardless of array order or future additions. * test(clv2): fail fast when python is missing in SIGALRM check The behavioral test returned early when no python interpreter was found, which the test harness records as a PASS — so the SIGALRM contract could go entirely unverified yet still look green. Throw instead, matching the existing insaits-security-monitor convention of failing when a required Python runtime is absent, and drop the in-test console.log. |
||
|
Gaurav DubeyandGitHub
|
a89b32c2b5 |
fix(clv2): serialize observer signal-counter to stop dropped increments (#2372)
observe.sh bumps the SIGUSR1 throttle counter in
${PROJECT_DIR}/.observer-signal-counter with an unlocked read-modify-write.
The hook runs on every tool call, so concurrent invocations read the same
value, both increment, and lose a write, signaling the observer at
unpredictable intervals and defeating the #521 throttle.
Serialize the read-modify-write under a lock, and only ever bump the counter
while that lock is held:
- Prefer flock with a bounded -w wait (the OS auto-releases it when the fd
closes or the process dies, so there is no stale lock and no lost increment);
on a timeout the tick is skipped rather than bumped unlocked.
- Fall back to an atomic mkdir lock on platforms without flock, with a bounded
spin. An EXIT trap cleans up on normal completion; INT/TERM traps release the
lock and exit, so a signal cannot drop the lock and then continue the
read-modify-write without ownership. If the lock cannot be acquired in the
budget the tick is skipped rather than raced. No hand-rolled PID stale-reclaim
(which is racy and can delete a live re-acquirer's lock).
- Guard the counter read against a corrupt (non-integer) file that would abort
the hook under set -e.
Add tests/hooks/observe-signal-counter-race.test.js: 20 concurrent observe.sh
invocations must not lose increments (exact under flock; at most one dropped on
the best-effort mkdir fallback), the runner rejects on any hook execution
failure or hang, plus content guards for the lock and the corrupt-counter
handling.
Fixes #2296
|
||
|
|
f12b106c3c |
fix(clv2): align Python _update_registry schema with shell counterpart (#2369)
* fix(clv2): align Python _update_registry schema with shell counterpart The Python `_update_registry` in instinct-cli.py wrote registry entries without the `id` and `created_at` fields, while the shell counterpart in detect-project.sh writes both. A projects.json entry could therefore have a different shape depending on which path (Python CLI or shell hook) last touched it. Emit the same field set and order as the shell version: id, name, root, remote, created_at (preserved from any existing entry), last_seen. Add regression tests asserting field parity and created_at preservation. Fixes #2299 * fix(clv2): guard _update_registry against a non-dict registry entry A malformed projects.json (a non-dict value for the current project id, e.g. null) would make existing.get("created_at", ...) raise and crash the update, losing the old code's ability to self-heal a corrupt per-entry value. Normalize existing to {} when it is not a dict so the entry is healed by the rewrite. Add a regression test for the malformed-entry path. * test(clv2): assert the first-write created_at == last_seen contract The new _update_registry tests only checked both timestamps were truthy. On the initial write both derive from the same `now`, so created_at must equal last_seen; assert that explicitly so a later refactor that breaks the contract is caught. Split the compound assertions into single-expression checks. * fix(clv2): heal a non-dict top-level registry in _update_registry A projects.json that is valid JSON but not a mapping (e.g. `[]` or a string) previously crashed _update_registry on registry.get(), before the per-entry guard could run, so the corrupt file could not be healed. Guard the top-level shape right after the load and fall back to {} so the rewrite repairs the file — matching the per-entry healing already in place. Resolves the remaining CodeRabbit finding on #2299. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Gaurav DubeyandGitHub
|
e3f467989a |
fix(clv2): escape $HOME before pgrep -f in migrate-homunculus.sh (#2339)
* fix(clv2): escape $HOME before pgrep -f in migrate-homunculus.sh pgrep -f treats its argument as an extended regular expression, but the running-observer guard interpolated $HOME unescaped. Paths containing regex metacharacters (e.g. /home/user.name, /home/c++dev, /home/user (work)) made the match over-broad or invalid, causing either a false negative (live observer missed, migration proceeds and risks registry corruption) or a false positive (migration blocked unnecessarily). Escape the ERE metacharacters in $HOME via sed before building the pattern so the home prefix is matched literally while the trailing .*observer-loop\.sh regex is preserved. Portable across BSD and GNU sed. Fixes #2301 * test(clv2): add regression test for migrate-homunculus.sh $HOME escaping Guards the #2301 fix: extracts the script's sed escaping command and asserts the resulting pgrep -f pattern matches the literal home path while no longer over-matching a regex-expanded decoy (HOME=/home/user.name must not match /home/userXname). Also pins that the guard uses escaped_home rather than $HOME directly. Follows the existing clv2 shell-test convention in tests/hooks/observe-entrypoint-allowlist.test.js. Refs #2301 * test(clv2): skip migrate-homunculus escaping test on Windows The test relies on POSIX bash/sed/grep -E semantics, which differ on the Windows CI runners. Guard with the same process.platform === 'win32' early exit used by tests/hooks/observe-subdirectory-detection.test.js so the bash-dependent assertions only run on POSIX platforms. Refs #2301 |
||
|
Gaurav DubeyandGitHub
|
d8a84b5f7b |
fix(.cursor/hooks): route block-no-verify through local hook to fix message-body false positives (#2107) (#2177)
Cursor hooks still called `npx block-no-verify@1.1.2`, the broken external package whose matcher over-matches: it blocks legitimate `git commit` whenever `--no-verify` (or `no-verify`) appears anywhere in the command string, including inside the commit message body. The Claude Code surface already routes through the in-repo `scripts/hooks/block-no-verify.js`, which performs flag-position-aware tokenisation and passes 25 regression tests covering every false-positive case from #2107. Add a thin Cursor wrapper (`before-shell-execution-block-no-verify.js`) that reads Cursor stdin, transforms to the Claude Code `tool_input.command` shape, delegates to the local hook's exported `run()`, and forwards exit code and stderr. Update `.cursor/hooks.json` to call the wrapper instead of the npx package. New 14-case test file pins the false-positive cases from the issue plus the still-blocked real bypass attempts. Fixes #2107 |
||
|
Gaurav DubeyandGitHub
|
8dc43e5f60 |
fix(session-start): support ECC_SESSION_RETENTION_DAYS opt-out + document env var (#2151) (#2163)
* 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 |
||
|
Gaurav DubeyandGitHub
|
4afdb90800 |
feat(gateguard): add env knobs for routine bash gate + extra destructive patterns (#2161)
* 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. |
||
|
Gaurav DubeyandGitHub
|
a08445ad78 |
fix(suggest-compact): clean up old counter temp files (#2159)
* 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 |
||
|
Gaurav DubeyandGitHub
|
30ef079e7e |
fix(continuous-learning-v2): accept claude-vscode as valid entrypoint (#2134)
The observe.sh Layer 1 entrypoint guard short-circuits with exit 0 when
CLAUDE_CODE_ENTRYPOINT is not in {cli, sdk-ts, claude-desktop}. Claude
Code's VS Code extension sets CLAUDE_CODE_ENTRYPOINT=claude-vscode, so
VS Code users see no observations recorded — observations.jsonl never
gets created and the instinct pipeline stays empty.
Add claude-vscode to the allowlist, mirroring the precedent in #1522
which added claude-desktop the same way.
Add a regression test that spawns observe.sh under bash -x for each
allowed entrypoint (cli, sdk-ts, claude-desktop, claude-vscode) and
each denied entrypoint (unknown-host, claude-cody, mcp), asserting
that allowed entrypoints reach Layer 2's ECC_HOOK_PROFILE check while
denied entrypoints stop at Layer 1.
Fixes #2102
|
||
|
Gaurav DubeyandGitHub
|
ee9e5a19c4 |
fix(install-targets): validate compiled OpenCode plugin before install (#2041)
Fail fast when the OpenCode home install is attempted from a source checkout without the compiled .opencode/dist payload. PR had the full CI matrix green. |
||
|
Gaurav DubeyGitHubClaude Opus 4.7cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> |
e196f8a4cb |
fix(ci): flag SKILL.md frontmatter defects in validate-skills (#1669)
* fix(ci): flag SKILL.md frontmatter defects in validate-skills Issue #1663 reported two SKILL.md frontmatter defects (missing `name:` on skill-stocktake; literal block-scalar `description: |-` on openclaw-persona-forge) that PR #1664 addresses at the data level. This change is complementary: it extends `scripts/ci/validate-skills.js` to catch the same class of defect statically going forward, so the frontmatter-vs-renderer problems do not silently reappear as new skills land. ## Checks added - Frontmatter must declare a `name:` field. - Frontmatter `description:` must not use a literal block scalar (`|` / `|-` / `|+`) — these preserve internal newlines and break flat-table renderers keyed off `description`. Folded (`>`) and inline strings are accepted. ## Behavior - Frontmatter findings default to WARN (exit 0) so this PR does not break CI while the two known offenders are still on main. Pass `--strict` or set `CI_STRICT_SKILLS=1` to promote them to ERROR (exit 1). Structural findings (missing / empty SKILL.md) remain errors as before. - Today against main, the validator reports exactly two warnings — the same two files called out in #1663 — and exits 0. When #1664 lands, the validator reports zero warnings, at which point strict mode can be enabled in CI. ## Parser notes - Bespoke frontmatter parser mirrors the style of `validate-agents.js` (tolerant of UTF-8 BOM and CRLF; no new npm dependency). - Block-scalar continuation lines are skipped so keys inside a block scalar are not mistaken for top-level keys. - Hidden directories (`.something/`) under skills/ are now skipped. ## Tests Adds five focused tests to `tests/ci/validators.test.js`: - warns when frontmatter is missing `name` (default mode) - errors when frontmatter is missing `name` (--strict mode) - warns on literal block-scalar description (|-) - accepts folded (>) and inline descriptions under --strict - skips hidden directories under skills/ ## Docs Adds two bullets to the `Skill Checklist` in CONTRIBUTING.md covering the two rules now surfaced by the validator. Refs #1663. Complements (does not compete with) #1664. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): harden SKILL.md frontmatter checks after bot review Address findings from CodeRabbit, Greptile, and cubic on #1669: - Guard empty or whitespace-only `name:` values. Previously `name: ` silently passed because the presence check only tested key-set membership; now inspectFrontmatter captures trimmed values and validate flags an explicit 'name is empty' WARN/ERROR. - Broaden block-scalar detection to cover YAML 1.2 indent indicators (`|2`, `|-2`, `>2-`) and trailing comments (`|- # note`). The old regex required a bare `|`/`>` with optional `+`/`-`, which let valid-but-disallowed forms slip through. - Update CONTRIBUTING.md checklist to list `|+` alongside `|` and `|-` for parity with the validator. - Extend runSkillsValidator to accept env overrides and add four regression tests: empty name, |+ description, |-2 + comment, and CI_STRICT_SKILLS=1. * fix(ci): address round-2 review on validate-skills frontmatter - Tighten extractFrontmatter closing delimiter to require a newline or end-of-file after the closing `---`, so body lines beginning with `---text` are not parsed as frontmatter (CodeRabbit). - Strip both trailing and comment-only values in inspectFrontmatter, so `name: # todo` is surfaced as empty rather than silently passing (cubic P2). - Extract validateSkillDir helper so the per-directory validation block moves out of validateSkills, keeping both functions under the 50-line guideline (CodeRabbit nit). - Hoist runSkillsValidator to module scope in the test harness and share the spawnSync import with execFileSync so the helper stops re-requiring child_process on every invocation (CodeRabbit nit). - Add regression tests: comment-only `name:` values must fail strict mode; `---trailing` body lines must not be parsed as frontmatter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Update tests/ci/validators.test.js Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> |
||
|
Gaurav DubeyandGitHub
|
2bb88cff47 |
docs(strategic-compact): fix hook command path in zh-CN/zh-TW/ja-JP SKILL.md (#1701)
Extends the hook command path correction from PR #1682 (English source) to the zh-CN, zh-TW, and ja-JP translated mirrors so the PreToolUse hook example matches the actual script location at ~/.claude/scripts/hooks/suggest-compact.js. Changes per locale: - docs/zh-CN/skills/strategic-compact/SKILL.md: update both command strings from ~/.claude/skills/strategic-compact/suggest-compact.js to ~/.claude/scripts/hooks/suggest-compact.js. - docs/zh-TW/skills/strategic-compact/SKILL.md: replace the outdated suggest-compact.sh reference (the .sh variant was removed in merged PR #41) with the current node-invoked suggest-compact.js, and align the matcher block structure with the English canonical SKILL.md post-#1682. - docs/ja-JP/skills/strategic-compact/SKILL.md: same .sh -> .js migration and matcher alignment as zh-TW. The ko-KR mirror already uses the correct CLAUDE_PLUGIN_ROOT-based hook path and needs no change. Refs #1675 |
||
|
|
105b524c8f |
docs(strategic-compact): fix hook command path in SKILL.md (#1682)
The Hook Setup example pointed to `~/.claude/skills/strategic-compact/suggest-compact.js`, which does not exist in the current repo layout. The cross-platform Node.js hook ships at `scripts/hooks/suggest-compact.js` and is installed to `~/.claude/scripts/hooks/suggest-compact.js`. Anyone copy-pasting the documented config hit a broken hook command. Closes #1675 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
53a599fc03 |
docs: update README to reflect agents field removal from plugin manifest
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
|
c19fde229a |
fix: remove agents field from plugin.json manifest (#1459)
The Claude Code plugin validator rejects the "agents" field entirely. Remove it from the manifest, schema, and tests. Update schema notes to document this as a known constraint alongside the hooks field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> |