Compare commits

..

7 Commits

Author SHA1 Message Date
Jamkris
85d33748e0 test(hooks): regression coverage for round 1 review fixes
9 new cases locking in the behavior added by the previous two
commits. Each was verified to fail before the fix and pass after.

Greptile — quote-aware depth counting:
  - blocks $(echo ")"; (npm run dev))
  - blocks (echo ")"; npm run dev)
  - allows $(echo "(npm run dev)") — () inside double-quoted body is literal

Greptile — brace groups:
  - blocks { npm run dev; }
  - blocks echo hi && { npm run dev; }
  - allows {npm run dev} — bash brace-group syntax requires a space after {

CodeRabbit — missing package-manager variants:
  - blocks yarn run dev (yarn 1.x convention)
  - blocks bun dev (bun bare form)

CodeRabbit nitpick — symmetric quote test:
  - blocks echo "$(npm run dev)" — double-quoted substitution still substitutes

The `{npm run dev}` allow case is intentional: bash treats `{` as
a reserved word only when followed by whitespace. The pre-fix code
already passed this through, but until now we never asserted it,
so a future change to brace handling could silently start blocking
literal `{npm` tokens.
2026-05-14 12:24:45 +09:00
Jamkris
e2eaf4ac2f fix(hooks): cover brace groups + yarn-run/bun-bare dev variants
Two false-negatives surfaced in PR #1889 review:

1. Brace-group bypass (Greptile).
   `{ npm run dev; }` evaluates the dev command in the *current*
   shell — semantically distinct from `( ... )` but with the same
   effect for this hook. `splitShellSegments` correctly cleaves the
   group at `;` into `["{ npm run dev", "}"]`, but the first segment's
   leading token under `readToken` is the bare `{`, which was not in
   `DEV_COMMAND_WORDS`, so the dev-pattern check was skipped.

   Fix: treat `{` and `}` as no-op tokens in `getLeadingCommandWord`
   so we keep walking to the real command word. Matches how shell
   itself parses brace groups (the braces are reserved words, not
   commands). Bash requires a space after `{` and a terminator before
   `}` for an actual group, so `{npm run dev}` correctly remains
   allowed (single token `{npm`, not in `DEV_COMMAND_WORDS`).

2. Missing yarn-run / bun-bare variants (CodeRabbit).
   Both `yarn dev` *and* `yarn run dev` are valid (the latter is what
   `package.json` actually wires `dev` to under yarn 1.x). The same
   `(run )?` symmetry applies to bun. The previous `DEV_PATTERN` only
   matched `yarn\s+dev` and `bun\s+run\s+dev`, allowing the cross
   forms to pass through silently.

   Fix: `yarn(?:\s+run)?\s+dev` and `bun(?:\s+run)?\s+dev` — same
   shape `pnpm(?:\s+run)?\s+dev` was already using.

Verified after this commit (every form now exits 2):

  { npm run dev; }
  { npm run dev ; }
  echo hi && { npm run dev; }
  ({ npm run dev; })
  $( { npm run dev; } )
  yarn run dev
  bun dev

Verified still allowed (no regression):

  echo "{ npm run dev; }"   # literal inside double quotes
  {npm run dev}             # not a brace group per bash syntax
2026-05-14 12:23:55 +09:00
Jamkris
70b86d81c4 fix(lib): track quote state inside command-substitution depth counters
Greptile flagged a bypass in PR #1889: `$(echo ")"; (npm run dev))`
threaded the depth-counting loops in `extractCommandSubstitutions`
and `extractSubshellGroups` to terminate early, because a literal `)`
inside double quotes was treated as a real closing paren. The
truncated body then ended in a dangling `"` that toggled `inDouble`
in the outer scan, masking the subsequent `(npm run dev)` group from
extraction.

Reproduced (before this commit) by piping the synthetic PreToolUse
payload `{"tool_input":{"command":"$(echo \")\"; (npm run dev))"}}`
into `scripts/hooks/pre-bash-dev-server-block.js` and observing
exit 0 (allow) where the dev pattern is clearly present.

Fix: each `$(...)` and `(...)` body loop now tracks its own
single/double quote state and only treats `(` / `)` as depth
delimiters when outside quotes. The quoted `)` no longer closes
the group early, the body now extends to the real closing paren,
and the outer scan's quote state remains untouched.

After this commit:
  $ echo '{"tool_input":{"command":"$(echo \")\"; (npm run dev))"}}' \
      | node scripts/hooks/pre-bash-dev-server-block.js; echo $?
  2

The symmetric form `$(echo "(npm run dev)")` correctly remains
allowed (bash does not honor `(...)` inside double quotes).
2026-05-14 12:22:22 +09:00
Jamkris
4f4654bf21 test(hooks): regression coverage for dev-server-block subshell bypass
Lock in the behavior added by the previous commit. Each new case was
verified to fail before the fix and pass after.

Bypasses now blocked (exit 2):
- \$(npm run dev)              command substitution
- \`npm run dev\`              backtick substitution
- echo \$(npm run dev)         substitution inside an argument
- (npm run dev)               plain subshell group
- \$(echo a; npm run dev)      substitution containing a sequenced segment
- (pnpm dev)                  plain subshell group, alt package manager

Allow cases — explicitly proven NOT to regress so the fix doesn't
over-block legitimate uses:
- (tmux new-session -d -s dev "npm run dev")   tmux launcher inside ()
- git commit -m '(npm run dev)'                literal in single quotes
- echo "(npm run dev)"                         literal in double quotes
  (bash does NOT subshell () inside double quotes)
- git commit -m '\$(npm run dev) fix'          literal in single quotes

Single- and double-quote allow cases are important: they distinguish a
real subshell construct from one that's just text inside a string,
which is what `extractSubshellGroups` / `extractCommandSubstitutions`
quote-awareness is for.
2026-05-14 11:22:44 +09:00
Jamkris
a7e51e8046 fix(hooks): close subshell bypass in pre-bash-dev-server-block
Before this commit the dev-server-block hook ran the leading-command
and dev-pattern check only against the top-level segments returned by
`splitShellSegments`, which doesn't split on `$(...)`, backticks, or
plain `(...)`. That left the policy bypassable by wrapping a dev
command in any of those constructs:

  $(npm run dev)
  `npm run dev`
  echo $(npm run dev)
  (npm run dev)

Each verified by piping a synthetic PreToolUse payload into the hook
on this branch: every form above returned exit 0 (allow) where a plain
`npm run dev` correctly returned exit 2 (block).

Fix: expand the check space before running the leading-command rule.
A small BFS walks the raw command, harvesting bodies from
`extractCommandSubstitutions` (`$(...)` and backticks) and from
`extractSubshellGroups` (plain `(...)`), then splits each harvested
body through `splitShellSegments` and feeds the result into the
existing `isBlockedDevSegment` check.

This preserves every existing allow case (`tmux new-session -d -s dev
"npm run dev"`, quoted-string mentions like `git commit -m "npm run
dev fix"`, `echo hi`) because the leading-command rule is unchanged —
only the set of segments it runs against grew.

Known limitation, not fixed here: `eval "$(echo npm run dev)"` still
slips through because the substitution body's leading command is
`echo`, and statically modeling echo's output to recover the executed
command is out of scope. The same class affects `gateguard-fact-force`
(via `eval "$(echo rm -rf /)"` etc.) and is best addressed in both
hooks together as a follow-up rather than as a one-off here.
2026-05-14 11:21:41 +09:00
Jamkris
04dc03c3af feat(lib): add extractSubshellGroups for plain (...) subshells
`extractCommandSubstitutions` only walks `$(...)` and backticks — the two
shell constructs whose bodies are captured as strings. Bash also has
plain `(...)` subshells (e.g. `(npm run dev)`), where the body executes
in a child shell but is not value-captured. Our PreToolUse hooks need
to peer inside those too, because a `(...)` group bypasses the
top-level segment splitter just like `$(...)` does.

This commit adds a sibling extractor with the same conventions as
`extractCommandSubstitutions`:

- single quotes literal — `'(npm run dev)'` is a string, ignored
- double quotes literal for parens — `"(npm run dev)"` is a string
  (bash only honors `$(...)`, not bare `(...)`, inside double quotes)
- skips `$(...)` and backtick spans so we don't double-extract
  bodies the other helper already handles
- recurses into its own bodies for nested groups

No consumer yet; the next commit wires both extractors into
`scripts/hooks/pre-bash-dev-server-block.js` to close the subshell
bypass surface.
2026-05-14 11:17:46 +09:00
Jamkris
0a380c3e85 feat(lib): extract shell command-substitution parser to shared lib
Extract the `extractCommandSubstitutions` function originally
introduced in scripts/hooks/gateguard-fact-force.js (PR #1853
round 2) into scripts/lib/shell-substitution.js so other PreToolUse
hooks can reuse the same single-quote-aware, double-quote-aware,
nested-subshell-aware parser without duplicating it.

No behavior change in this commit — the function body is copied
verbatim and exposed via `module.exports`. The next commit wires it
into scripts/hooks/pre-bash-dev-server-block.js to close that hook's
own subshell-bypass holes.

gateguard-fact-force.js still defines its own private copy of the
function; consolidating both call sites onto this shared lib is a
follow-up worth doing once this PR lands, but is intentionally out
of scope here to keep the diff focused on the dev-server-block fix.
2026-05-14 11:10:40 +09:00
7 changed files with 410 additions and 422 deletions

View File

@@ -153,24 +153,6 @@ As of 2026-05-13:
`/ecc-tools analyze --job status` now reads the #65 latest-result cache for
the current PR head and posts a compact completed/blocked/not-run table with
the next hosted job command, without queueing work or billing usage.
- ECC-Tools PR #67 merged as `f20e6bec2b0bf49e4cc36e08b7285c795973b73d`
and made the hosted depth-plan check-run status-aware:
queued PR analysis now reads the #65/#66 latest-result cache when publishing
`ECC Tools / Hosted Depth Plan`, includes the latest hosted run status in
the plan table, and recommends the next unrun ready job before reruns.
- ECC-Tools PR #68 merged as `2cde524b5ef8f34ab7bb1af973248fe4be4359f8`
and added deterministic hosted promotion readiness:
opened/synchronized PRs now publish a non-blocking
`ECC Tools / Hosted Promotion Readiness` check-run that compares changed
files against the checked-in evaluator/RAG corpus, warns on missing
hosted-job promotion evidence, and can be disabled with
`PR_HOSTED_PROMOTION_READINESS_CHECK_MODE=off`.
- ECC-Tools PR #69 merged as `d0112dac7cef807ae27def41f057682ef0772cce`
and extended hosted promotion readiness with deterministic output scoring:
the check now reads cached completed hosted job results for the current PR
head, scores their artifacts and findings against evaluator/RAG corpus
expectations, and treats matching hosted artifacts as promotion evidence
before reporting a gap.
- Handoff `ecc-supply-chain-audit-20260513-0645.md` under
`~/.cluster-swarm/handoffs/`
records the May 13 supply-chain sweep: no active lockfile/manifest hit for
@@ -404,16 +386,6 @@ As of 2026-05-13:
`/ecc-tools analyze --job status`, summarizing completed, blocked, and
not-yet-run hosted jobs for the PR head and recommending the next hosted job
command.
- ECC-Tools PR #67 feeds those cached results back into the hosted depth-plan
check-run so queued analysis recommends the next unrun ready hosted job from
cache state instead of repeating the static readiness order.
- ECC-Tools PR #68 adds the first evaluator-backed hosted promotion gate:
opened/synchronized PRs get a non-blocking Hosted Promotion Readiness
check-run that turns the evaluator/RAG corpus into warnings when changed
files match fixture scenarios without their expected evidence artifacts.
- ECC-Tools PR #69 extends that gate to score cached completed hosted job
outputs for the current PR head, so hosted artifacts can satisfy corpus
evidence expectations before the check reports a promotion gap.
- ECC PR #1803 landed the contributor Quarkus handling branch after maintainer
cleanup, current-`main` alignment, full local validation, and preservation of
the author's removal of incomplete ja-JP and zh-CN Quarkus translations.
@@ -467,10 +439,10 @@ is not complete unless the evidence column exists and has been freshly verified.
| Claude and Codex plugin publication | Contact/submission path with required artifacts and status | Publication readiness, naming matrix, and May 12 dry-run evidence document plugin validation, clean-checkout Claude tag/install smoke, and Codex marketplace CLI shape | Needs explicit approval for real tag/push and marketplace submission |
| Articles, tweets, and announcements | X thread, LinkedIn copy, GitHub release copy, push checklist | Draft launch collateral exists under rc.1 release docs | Needs URL-backed refresh |
| AgentShield enterprise iteration | Policy gates, SARIF, packs, provenance, corpus, HTML reports, exception lifecycle audit, baseline drift Action/CLI surfaces, evidence-pack redaction, harness adapter registry, enterprise research roadmap, supply-chain hardened release path, CI-safe baseline fingerprints, corpus accuracy recommendations, remediation workflow phases, env proxy hijack corpus coverage | PRs #53, #55-#64, #67-#69, and #78-#82 landed with test evidence; native PDF export deferred in favor of self-contained HTML plus print-to-PDF until explicit enterprise demand appears; `docs/architecture/agentshield-enterprise-research-roadmap.md` now has baseline drift, evidence-pack bundle, redaction, adapter-registry, supply-chain hardening, hashed baseline fingerprints, corpus accuracy recommendation, remediation workflow, and env proxy hijack corpus slices landed | Next hosted evidence-pack workflow depth |
| ECC Tools next-level app | Billing audit, PR checks, deep analyzer, sync backlog, evaluator/RAG corpus, analysis-depth readiness, hosted execution planning, hosted CI diagnostics, hosted security evidence review, hosted harness compatibility audit, hosted reference-set evaluation, hosted AI routing/cost review, hosted team backlog routing, hosted depth-plan check-run, PR-comment hosted job dispatch, hosted job result history/check-runs, hosted result status command, status-aware depth-plan recommendations, hosted promotion readiness, hosted promotion output scoring | PRs #26-#43 plus #53-#69 landed with test evidence, including AgentShield evidence-pack gap routing, canonical bundle recognition, supply-chain signature gates, PR draft follow-up Linear tracking, evidence-backed/deep-ready repository classification, the `/api/analysis/depth-plan` hosted job plan, `/api/analysis/jobs/ci-diagnostics`, `/api/analysis/jobs/security-evidence-review`, `/api/analysis/jobs/harness-compatibility-audit`, `/api/analysis/jobs/reference-set-evaluation`, `/api/analysis/jobs/ai-routing-cost-review`, `/api/analysis/jobs/team-backlog-routing`, the `ECC Tools / Hosted Depth Plan` check-run, `/ecc-tools analyze --job ...` PR-comment dispatch, non-blocking per-hosted-job result check-runs backed by 30-day result cache records, `/ecc-tools analyze --job status` cache lookup, cache-aware next-job recommendations in the depth-plan check-run, the `ECC Tools / Hosted Promotion Readiness` corpus-backed PR check-run, and deterministic hosted-output scoring against cached completed job artifacts/findings | Next work is retrieval/model-backed hosted promotion after deterministic output scoring |
| ECC Tools next-level app | Billing audit, PR checks, deep analyzer, sync backlog, evaluator/RAG corpus, analysis-depth readiness, hosted execution planning, hosted CI diagnostics, hosted security evidence review, hosted harness compatibility audit, hosted reference-set evaluation, hosted AI routing/cost review, hosted team backlog routing, hosted depth-plan check-run, PR-comment hosted job dispatch, hosted job result history/check-runs, hosted result status command | PRs #26-#43 plus #53-#66 landed with test evidence, including AgentShield evidence-pack gap routing, canonical bundle recognition, supply-chain signature gates, PR draft follow-up Linear tracking, evidence-backed/deep-ready repository classification, the `/api/analysis/depth-plan` hosted job plan, `/api/analysis/jobs/ci-diagnostics`, `/api/analysis/jobs/security-evidence-review`, `/api/analysis/jobs/harness-compatibility-audit`, `/api/analysis/jobs/reference-set-evaluation`, `/api/analysis/jobs/ai-routing-cost-review`, `/api/analysis/jobs/team-backlog-routing`, the `ECC Tools / Hosted Depth Plan` check-run, `/ecc-tools analyze --job ...` PR-comment dispatch, non-blocking per-hosted-job result check-runs backed by 30-day result cache records, and `/ecc-tools analyze --job status` cache lookup | Next work is evaluator-backed hosted promotion and status-aware depth-plan recommendations |
| GitGuardian/Dependabot/CodeRabbit-style checks | Non-blocking taxonomy, deterministic follow-up checks, and local supply-chain gates | ECC-Tools risk taxonomy check plus follow-up signals landed, including Skill Quality, Deep Analyzer Evidence, Analyzer Corpus Evidence, RAG/Evaluator Evidence, PR Review/Salvage Evidence, and AgentShield evidence-pack evidence; #1846 added npm registry signature gates; #1848 added the supply-chain incident-response playbook and `pull_request_target` cache-poisoning validator guard; #1851 added the privileged checkout credential-persistence guard; AgentShield #78, JARVIS #13, and ECC-Tools #53 applied the same hardening outside trunk | Current supply-chain gate complete; deeper hosted review features remain future |
| Harness-agnostic learning system | Audit, adapter matrix, observability, traces, promotion loop | Audit/adapters/observability gates plus `docs/architecture/evaluator-rag-prototype.md`, `examples/evaluator-rag-prototype/`, and ECC-Tools PR #40 define read-only stale-salvage, billing-readiness, CI-failure-diagnosis, harness-config-quality, AgentShield policy-exception, skill-quality evidence, deep-analyzer evidence, and RAG/evaluator comparison scenarios with trace, report, playbook, verifier, and predictive-check artifacts; ECC-Tools PRs #68/#69 now turn that corpus into a deterministic PR check-run gate with cached hosted-output scoring | Deterministic hosted PR check and cached output scoring integrated; hosted retrieval remains future |
| Linear roadmap is detailed | Linear project status plus repo mirror | Repo mirror exists; issue creation was retried on 2026-05-12 and remains blocked by the workspace free issue limit; this May 13 sync adds ECC #1860, AgentShield #78-#82, JARVIS #13, ECC-Tools #53-#69, resolved queue/discussion counts, and Linear project status updates through ECC-Tools #69 | Needs recurring status updates after each merge batch |
| Harness-agnostic learning system | Audit, adapter matrix, observability, traces, promotion loop | Audit/adapters/observability gates plus `docs/architecture/evaluator-rag-prototype.md`, `examples/evaluator-rag-prototype/`, and ECC-Tools PR #40 define read-only stale-salvage, billing-readiness, CI-failure-diagnosis, harness-config-quality, AgentShield policy-exception, skill-quality evidence, deep-analyzer evidence, and RAG/evaluator comparison scenarios with trace, report, playbook, verifier, and predictive-check artifacts | Local corpus complete; hosted integration remains future |
| Linear roadmap is detailed | Linear project status plus repo mirror | Repo mirror exists; issue creation was retried on 2026-05-12 and remains blocked by the workspace free issue limit; this May 13 sync adds ECC #1860, AgentShield #78-#82, JARVIS #13, ECC-Tools #53-#66, resolved queue/discussion counts, and Linear project status updates through ECC-Tools #66 | Needs recurring status updates after each merge batch |
| Flow separation and progress tracking | Flow lanes with owner artifacts and update cadence | This roadmap defines lanes below and `docs/architecture/progress-sync-contract.md` makes GitHub/Linear/handoff/roadmap sync part of the readiness gate | Active |
| Realtime Linear sync | Project updates while issue limit is blocked; issues later | ECC-Tools #39 implements opt-in Linear API sync for deferred follow-up backlog items, and ECC-Tools #54 adds copy-ready PR drafts to that backlog when draft PR shells are not opened; `docs/architecture/progress-sync-contract.md` defines the local file-backed realtime boundary while issue capacity is blocked | Needs workspace capacity/config rollout |
| Observability for self-use | Local readiness gate, traces, status snapshots, HUD/status contract, risk ledger, progress-sync contract | `npm run observability:ready` reports 21/21 | Complete for local gate |
@@ -489,9 +461,9 @@ repo evidence and merge commits.
| Queue hygiene and salvage | GitHub PR/issue state, salvage ledger | Append ledger entries for any future stale closures | Every cleanup batch |
| Release and publication | rc.1 release docs, publication readiness doc | Naming matrix and plugin submission/contact checklist | Before any tag |
| Harness OS core | Audit, adapter matrix, observability docs, `ecc2/` | HUD/session-control acceptance spec | Weekly until GA |
| Evaluation and RAG | Reference-set validation, harness audit, traces, ECC-Tools corpus | Read-only evaluator/RAG prototype plus stale-salvage, billing-readiness, CI-failure-diagnosis, harness-config-quality, AgentShield policy-exception, skill-quality evidence, deep-analyzer evidence, and RAG/evaluator comparison fixtures; ECC-Tools #68 publishes the corpus as a hosted promotion readiness check-run, and #69 scores cached hosted job outputs against the same corpus | Hosted retrieval/model-backed promotion plan |
| Evaluation and RAG | Reference-set validation, harness audit, traces, ECC-Tools corpus | Read-only evaluator/RAG prototype plus stale-salvage, billing-readiness, CI-failure-diagnosis, harness-config-quality, AgentShield policy-exception, skill-quality evidence, deep-analyzer evidence, and RAG/evaluator comparison fixtures | Hosted retrieval/check-run automation plan |
| AgentShield enterprise | AgentShield PR evidence and roadmap notes | Remediation workflow depth or corpus expansion follow-up | Next implementation batch |
| ECC Tools app | ECC-Tools PR evidence, billing audit, risk taxonomy, evaluator/RAG corpus | ECC-Tools #53 published the supply-chain workflow hardening branch, #54 tracks copy-ready PR drafts in the Linear/project backlog, #55 classifies analysis-depth readiness, #56 exposes the hosted execution plan, #57 executes the first hosted CI diagnostics job, #58 executes the hosted security evidence review job, #59 executes the hosted harness compatibility audit, #60 executes the hosted reference-set evaluation, #61 executes the hosted AI routing/cost review, #62 executes hosted team backlog routing, #63 publishes the hosted depth-plan check-run, #64 dispatches hosted jobs from PR comments, #65 persists hosted result history/check-runs, #66 exposes hosted job status from PR comments, #67 makes depth-plan recommendations cache-aware, #68 publishes hosted promotion readiness from the evaluator/RAG corpus, and #69 scores cached hosted job outputs against that corpus; next work is retrieval/model-backed hosted promotion | Next implementation batch |
| ECC Tools app | ECC-Tools PR evidence, billing audit, risk taxonomy, evaluator/RAG corpus | ECC-Tools #53 published the supply-chain workflow hardening branch, #54 tracks copy-ready PR drafts in the Linear/project backlog, #55 classifies analysis-depth readiness, #56 exposes the hosted execution plan, #57 executes the first hosted CI diagnostics job, #58 executes the hosted security evidence review job, #59 executes the hosted harness compatibility audit, #60 executes the hosted reference-set evaluation, #61 executes the hosted AI routing/cost review, #62 executes hosted team backlog routing, #63 publishes the hosted depth-plan check-run, and #64 dispatches hosted jobs from PR comments; next work is hosted result history/check-run summaries | Next implementation batch |
| Linear progress | Linear project status updates, `docs/architecture/progress-sync-contract.md`, and this mirror | Status update with queue/evidence/missing gates | Every significant merge batch |
The project status update should always include:
@@ -708,9 +680,9 @@ Acceptance:
PR #82 expanded corpus coverage for env proxy hijacks and out-of-band
exfiltration; and ECC-Tools PRs #42/#43 now route and recognize evidence
packs. The next slice is hosted evidence-pack workflow depth.
2. Plan retrieval/model-backed hosted promotion on top of the #69 deterministic
hosted output scoring contract, keeping vector/model judgment behind fixture
evaluation until the retrieval contract is stable.
2. Feed the #66 status surface back into hosted depth-plan recommendations so
queued analysis can suggest the next unrun or newly blocked hosted job from
cached outcomes, not only static readiness.
3. Enable/configure the merged Linear backlog sync path after workspace issue
capacity clears or the Linear workspace is upgraded, then verify PR-draft
salvage items land in the expected project.

View File

@@ -199,8 +199,7 @@
"skills/database-migrations",
"skills/jpa-patterns",
"skills/mysql-patterns",
"skills/postgres-patterns",
"skills/prisma-patterns"
"skills/postgres-patterns"
],
"targets": [
"claude",

View File

@@ -4,6 +4,10 @@
const MAX_STDIN = 1024 * 1024;
const path = require('path');
const { splitShellSegments } = require('../lib/shell-split');
const {
extractCommandSubstitutions,
extractSubshellGroups
} = require('../lib/shell-substitution');
const DEV_COMMAND_WORDS = new Set([
'npm',
@@ -123,6 +127,8 @@ function getLeadingCommandWord(segment) {
continue;
}
if (token === '{' || token === '}') continue;
if (/^[A-Za-z_][A-Za-z0-9_]*=.*/.test(token)) continue;
const normalizedToken = normalizeCommandWord(token);
@@ -154,23 +160,55 @@ process.stdin.on('data', chunk => {
}
});
const TMUX_LAUNCHER = /^\s*tmux\s+(new|new-session|new-window|split-window)\b/;
const DEV_PATTERN = /\b(npm\s+run\s+dev|pnpm(?:\s+run)?\s+dev|yarn(?:\s+run)?\s+dev|bun(?:\s+run)?\s+dev)\b/;
/**
* Collect every command-line segment we should evaluate. Returns the top-level
* segments first, then segments harvested from `$(...)` / backtick command
* substitutions and plain `(...)` subshell groups, recursively.
*
* Without this expansion the leading-command and dev-pattern check below only
* sees the outermost command, so wrappers like `$(npm run dev)` and
* `(npm run dev)` (which still spawn a dev server) sneak past.
*/
function collectCheckSegments(cmd) {
const segments = [...splitShellSegments(cmd)];
const queue = [cmd];
const seen = new Set();
while (queue.length) {
const current = queue.shift();
if (seen.has(current)) continue;
seen.add(current);
for (const body of extractCommandSubstitutions(current)) {
for (const seg of splitShellSegments(body)) segments.push(seg);
queue.push(body);
}
for (const body of extractSubshellGroups(current)) {
for (const seg of splitShellSegments(body)) segments.push(seg);
queue.push(body);
}
}
return segments;
}
function isBlockedDevSegment(segment) {
const commandWord = getLeadingCommandWord(segment);
if (!commandWord || !DEV_COMMAND_WORDS.has(commandWord)) return false;
return DEV_PATTERN.test(segment) && !TMUX_LAUNCHER.test(segment);
}
process.stdin.on('end', () => {
try {
const input = JSON.parse(raw);
const cmd = String(input.tool_input?.command || '');
if (process.platform !== 'win32') {
const segments = splitShellSegments(cmd);
const tmuxLauncher = /^\s*tmux\s+(new|new-session|new-window|split-window)\b/;
const devPattern = /\b(npm\s+run\s+dev|pnpm(?:\s+run)?\s+dev|yarn\s+dev|bun\s+run\s+dev)\b/;
const hasBlockedDev = segments.some(segment => {
const commandWord = getLeadingCommandWord(segment);
if (!commandWord || !DEV_COMMAND_WORDS.has(commandWord)) {
return false;
}
return devPattern.test(segment) && !tmuxLauncher.test(segment);
});
const segments = collectCheckSegments(cmd);
const hasBlockedDev = segments.some(isBlockedDevSegment);
if (hasBlockedDev) {
console.error('[Hook] BLOCKED: Dev server must run in tmux for log access');

View File

@@ -0,0 +1,246 @@
'use strict';
/**
* Extract executable command-substitution bodies from a shell line.
*
* Single quotes are literal, so substitutions inside them are ignored;
* double quotes still permit substitutions, so those bodies are scanned
* before quoted text is stripped. Returns each substitution body plus
* any nested substitutions discovered recursively.
*
* Originally introduced in scripts/hooks/gateguard-fact-force.js
* (PR #1853 round 2). Extracted to a shared lib so other PreToolUse
* hooks that need the same "scan inside `$(...)` and backticks"
* behavior can reuse it without duplicating the parser.
*
* @param {string} input
* @returns {string[]}
*/
function extractCommandSubstitutions(input) {
const source = String(input || '');
const substitutions = [];
let inSingle = false;
let inDouble = false;
for (let i = 0; i < source.length; i++) {
const ch = source[i];
const prev = source[i - 1];
if (ch === '\\' && !inSingle) {
i += 1;
continue;
}
if (ch === "'" && !inDouble && prev !== '\\') {
inSingle = !inSingle;
continue;
}
if (ch === '"' && !inSingle && prev !== '\\') {
inDouble = !inDouble;
continue;
}
if (inSingle) {
continue;
}
if (ch === '`') {
let body = '';
i += 1;
while (i < source.length) {
const inner = source[i];
if (inner === '\\') {
body += inner;
if (i + 1 < source.length) {
body += source[i + 1];
i += 2;
continue;
}
}
if (inner === '`') {
break;
}
body += inner;
i += 1;
}
if (body.trim()) {
substitutions.push(body);
substitutions.push(...extractCommandSubstitutions(body));
}
continue;
}
if (ch === '$' && source[i + 1] === '(') {
let depth = 1;
let body = '';
let bodyInSingle = false;
let bodyInDouble = false;
i += 2;
while (i < source.length && depth > 0) {
const inner = source[i];
const innerPrev = source[i - 1];
if (inner === '\\' && !bodyInSingle) {
body += inner;
if (i + 1 < source.length) {
body += source[i + 1];
i += 2;
continue;
}
}
if (inner === "'" && !bodyInDouble && innerPrev !== '\\') {
bodyInSingle = !bodyInSingle;
} else if (inner === '"' && !bodyInSingle && innerPrev !== '\\') {
bodyInDouble = !bodyInDouble;
} else if (!bodyInSingle && !bodyInDouble) {
if (inner === '(') {
depth += 1;
} else if (inner === ')') {
depth -= 1;
if (depth === 0) {
break;
}
}
}
body += inner;
i += 1;
}
if (body.trim()) {
substitutions.push(body);
substitutions.push(...extractCommandSubstitutions(body));
}
}
}
return substitutions;
}
/**
* Extract bodies of plain `(...)` subshell groups.
*
* Bash treats `(npm run dev)` as a subshell that executes its contents, but
* the regex-light segment splitters used by our PreToolUse hooks don't peer
* inside those parens. This helper finds top-level `(...)` groups (skipping
* `$(...)` command substitutions and backticks, which `extractCommandSubstitutions`
* already covers) and returns each body, recursing for nested groups.
*
* Quote semantics:
* - Single quotes are literal: `'( ... )'` is a string, not a subshell.
* - Double quotes are literal *for parens*: `"( ... )"` is a string too —
* bash only honors `$( )` inside double quotes, not bare `( )`.
*
* @param {string} input
* @returns {string[]}
*/
function extractSubshellGroups(input) {
const source = String(input || '');
const groups = [];
let inSingle = false;
let inDouble = false;
for (let i = 0; i < source.length; i++) {
const ch = source[i];
const prev = source[i - 1];
if (ch === '\\' && !inSingle) {
i += 1;
continue;
}
if (ch === "'" && !inDouble && prev !== '\\') {
inSingle = !inSingle;
continue;
}
if (ch === '"' && !inSingle && prev !== '\\') {
inDouble = !inDouble;
continue;
}
if (inSingle || inDouble) {
continue;
}
if (ch === '$' && source[i + 1] === '(') {
let depth = 1;
let skipInSingle = false;
let skipInDouble = false;
i += 2;
while (i < source.length && depth > 0) {
const inner = source[i];
const innerPrev = source[i - 1];
if (inner === '\\' && !skipInSingle) {
i += 2;
continue;
}
if (inner === "'" && !skipInDouble && innerPrev !== '\\') {
skipInSingle = !skipInSingle;
} else if (inner === '"' && !skipInSingle && innerPrev !== '\\') {
skipInDouble = !skipInDouble;
} else if (!skipInSingle && !skipInDouble) {
if (inner === '(') depth += 1;
else if (inner === ')') depth -= 1;
}
i += 1;
}
i -= 1;
continue;
}
if (ch === '`') {
i += 1;
while (i < source.length && source[i] !== '`') {
if (source[i] === '\\' && i + 1 < source.length) {
i += 2;
continue;
}
i += 1;
}
continue;
}
if (ch === '(') {
let depth = 1;
let body = '';
let bodyInSingle = false;
let bodyInDouble = false;
i += 1;
while (i < source.length && depth > 0) {
const inner = source[i];
const innerPrev = source[i - 1];
if (inner === '\\' && !bodyInSingle) {
body += inner;
if (i + 1 < source.length) {
body += source[i + 1];
i += 2;
continue;
}
}
if (inner === "'" && !bodyInDouble && innerPrev !== '\\') {
bodyInSingle = !bodyInSingle;
} else if (inner === '"' && !bodyInSingle && innerPrev !== '\\') {
bodyInDouble = !bodyInDouble;
} else if (!bodyInSingle && !bodyInDouble) {
if (inner === '(') {
depth += 1;
} else if (inner === ')') {
depth -= 1;
if (depth === 0) {
break;
}
}
}
body += inner;
i += 1;
}
if (body.trim()) {
groups.push(body);
groups.push(...extractSubshellGroups(body));
}
}
}
return groups;
}
module.exports = { extractCommandSubstitutions, extractSubshellGroups };

View File

@@ -1,371 +0,0 @@
---
name: prisma-patterns
description: Prisma ORM patterns for TypeScript backends — schema design, query optimization, transactions, pagination, and critical traps like updateMany returning count not records, $transaction timeouts, migrate dev resetting the DB, @updatedAt skipped on bulk writes, and serverless connection exhaustion.
origin: ECC
---
# Prisma Patterns
Production patterns and non-obvious traps for Prisma ORM in TypeScript backends.
Tested against Prisma 5.x and 6.x. Some behaviors differ from Prisma 4.
Check the Prisma version before applying version-specific patterns:
```bash
npx prisma --version
```
Prisma 5 introduced `relationJoins`, which can load relations via JOIN rather than separate queries depending on query strategy and configuration. The `omit` field modifier and `prisma.$extends` Client Extensions API were also added. Note: `relationJoins` can cause row explosion on large 1:N relations or deep nested `include` — benchmark both approaches when relations may return many rows per parent.
## When to Activate
- Designing or modifying Prisma schema models and relations
- Writing queries, transactions, or pagination logic
- Using `updateMany`, `deleteMany`, or any bulk operation
- Running or planning database migrations
- Deploying to serverless environments (Vercel, Lambda, Cloudflare Workers)
- Implementing soft delete or multi-tenant row filtering
## Core Concepts
### ID Strategy
| Strategy | Use When | Avoid When |
|---|---|---|
| `@default(cuid())` | Default choice — URL-safe, sortable, no collisions | Sequential IDs needed for external systems |
| `@default(uuid())` | Interoperability with non-Prisma systems required | High-write tables (random UUIDs fragment B-tree indexes) |
| `@default(autoincrement())` | Internal join tables, audit logs | Public-facing IDs (exposes record count) |
### Schema Defaults
```prisma
model User {
id String @id @default(cuid())
email String @unique // @unique already creates an index — no @@index needed
name String
role Role @default(USER)
posts Post[]
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt
deletedAt DateTime?
@@index([createdAt])
@@index([deletedAt, createdAt]) // composite for soft-delete + sort queries
}
```
- Add `@@index` on every foreign key and column used in `WHERE` or `ORDER BY`.
- Declare `deletedAt DateTime?` upfront when soft delete is a foreseeable requirement — adding it later requires a migration on a live table.
- `updatedAt @updatedAt` is set automatically by Prisma on `update` and `upsert` only (see Anti-Patterns for bulk update trap).
### `include` vs `select`
| | `include` | `select` |
|---|---|---|
| Returns | All scalar fields + specified relations | Only specified fields |
| Use when | You need most fields plus a relation | Hot paths, large tables, avoiding over-fetch |
| Performance | May over-fetch on wide tables | Minimal payload, faster on large datasets |
| Prisma 5 note | Uses JOIN by default (`relationJoins`) | Same |
```ts
// include — all columns + relation
const user = await prisma.user.findUnique({
where: { id },
include: { posts: { select: { id: true, title: true } } },
});
// select — explicit allowlist
const user = await prisma.user.findUnique({
where: { id },
select: { id: true, email: true, name: true },
});
```
Never return raw Prisma entities from API responses — map to response DTOs to control exposed fields:
```ts
// BAD: leaks passwordHash, deletedAt, internal fields
return await prisma.user.findUniqueOrThrow({ where: { id } });
// GOOD: explicit DTO mapping
const user = await prisma.user.findUniqueOrThrow({ where: { id } });
return { id: user.id, name: user.name, email: user.email };
```
### Transaction Form Selection
| Situation | Use |
|---|---|
| Independent operations, no inter-dependency | Array form |
| Later step depends on earlier result | Interactive form |
| External calls (email, HTTP) involved | Outside transaction entirely |
```ts
// Array form — batched in one round trip
const [user, post] = await prisma.$transaction([
prisma.user.update({ where: { id }, data: { name } }),
prisma.post.create({ data: { title, authorId: id } }),
]);
// Interactive form — use tx client only, never the outer prisma client
const post = await prisma.$transaction(async (tx) => {
const user = await tx.user.findUniqueOrThrow({ where: { id } });
if (user.role !== 'ADMIN') throw new Error('Forbidden');
return tx.post.create({ data: { title, authorId: user.id } });
});
```
### PrismaClient Singleton
Each `PrismaClient` instance opens its own connection pool. Instantiate once.
```ts
// lib/prisma.ts
import { PrismaClient } from '@prisma/client';
const globalForPrisma = globalThis as unknown as { prisma?: PrismaClient };
export const prisma =
globalForPrisma.prisma ??
new PrismaClient({
log: process.env.NODE_ENV === 'development' ? ['query', 'error'] : ['error'],
});
if (process.env.NODE_ENV !== 'production') globalForPrisma.prisma = prisma;
```
The `globalThis` pattern prevents duplicate instances during hot reload (Next.js, nodemon, ts-node-dev).
### N+1 Problem
Loading relations inside a loop issues one query per row.
```ts
// BAD: N+1 — one extra query per user
const users = await prisma.user.findMany();
for (const user of users) {
const posts = await prisma.post.findMany({ where: { authorId: user.id } });
}
// GOOD: single query
const users = await prisma.user.findMany({ include: { posts: true } });
```
With Prisma 5+ `relationJoins`, the `include` form uses a single JOIN. On large 1:N sets this may increase result set size — benchmark both approaches if the relation can return many rows per parent.
## Code Examples
### Cursor Pagination (preferred for feeds and large datasets)
```ts
async function getPosts(cursor?: string, limit = 20) {
const items = await prisma.post.findMany({
where: { published: true },
orderBy: [
{ createdAt: 'desc' },
{ id: 'desc' }, // secondary sort prevents unstable pagination on duplicate timestamps
],
take: limit + 1,
...(cursor && { cursor: { id: cursor }, skip: 1 }),
});
const hasNextPage = items.length > limit;
if (hasNextPage) items.pop();
return { items, nextCursor: hasNextPage ? items[items.length - 1].id : null };
}
```
Fetch `limit + 1` and pop — canonical way to detect `hasNextPage` without an extra count query. Always include a unique field (e.g. `id`) as a secondary `orderBy` to prevent unstable pagination when multiple rows share the same timestamp. Use offset pagination only when users need to jump to arbitrary pages (admin tables).
### Soft Delete
```ts
// Always filter explicitly — do not rely on middleware (hides behavior, hard to debug)
const activeUsers = await prisma.user.findMany({ where: { deletedAt: null } });
await prisma.user.update({ where: { id }, data: { deletedAt: new Date() } });
await prisma.user.update({ where: { id }, data: { deletedAt: null } }); // restore
```
### Error Handling
```ts
import { Prisma } from '@prisma/client';
try {
await prisma.user.create({ data: { email } });
} catch (e) {
if (e instanceof Prisma.PrismaClientKnownRequestError) {
if (e.code === 'P2002') throw new ConflictError('Email already exists');
if (e.code === 'P2025') throw new NotFoundError('Record not found');
if (e.code === 'P2003') throw new BadRequestError('Referenced record does not exist');
}
throw e;
}
```
Common codes: `P2002` unique violation · `P2025` not found · `P2003` foreign key violation.
Catch at the service boundary and translate to domain errors. Never expose raw Prisma messages to API consumers.
### Connection Pool — Serverless
Embed connection params directly in `DATABASE_URL` — string concatenation breaks if the URL already has query parameters (e.g. `?schema=public`):
```bash
# .env — preferred: embed params in the URL
DATABASE_URL="postgresql://user:pass@host/db?connection_limit=1&pool_timeout=20"
# With an external pooler (PgBouncer, Supabase pooler)
DATABASE_URL="postgresql://user:pass@host/db?pgbouncer=true&connection_limit=1"
```
```ts
// Vercel, AWS Lambda, and similar serverless runtimes: cap pool to 1 per instance
// connection_limit and pool_timeout are controlled via DATABASE_URL
const prisma = new PrismaClient();
```
## Anti-Patterns
### `updateMany` returns a count, not records
```ts
// BAD: result is { count: 2 } — users[0] is undefined
const users = await prisma.user.updateMany({ where: { role: 'GUEST' }, data: { role: 'USER' } });
// GOOD: capture IDs first, then update, then fetch only the affected rows
const targets = await prisma.user.findMany({
where: { role: 'GUEST' },
select: { id: true },
});
const ids = targets.map((u) => u.id);
await prisma.user.updateMany({ where: { id: { in: ids } }, data: { role: 'USER' } });
const updated = await prisma.user.findMany({ where: { id: { in: ids } } });
```
Same applies to `deleteMany` — returns `{ count: n }`, never the deleted rows.
### `$transaction` interactive form times out after 5 seconds
```ts
// BAD: external call inside transaction exceeds 5s default → "Transaction already closed"
await prisma.$transaction(async (tx) => {
const user = await tx.user.findUniqueOrThrow({ where: { id } });
await sendWelcomeEmail(user.email); // external call
await tx.user.update({ where: { id }, data: { emailSent: true } });
});
// GOOD: external calls outside the transaction
const user = await prisma.user.findUniqueOrThrow({ where: { id } });
await sendWelcomeEmail(user.email);
await prisma.user.update({ where: { id }, data: { emailSent: true } });
// Only raise timeout when bulk processing genuinely needs it
await prisma.$transaction(async (tx) => { ... }, { timeout: 30_000 });
```
### `migrate dev` can reset the database
`migrate dev` detects schema drift and may prompt to reset the DB, dropping all data.
```bash
# NEVER on shared dev, staging, or production
npx prisma migrate dev --name add_column
# Safe everywhere except local solo dev
npx prisma migrate deploy
# Check drift without applying
npx prisma migrate diff \
--from-migrations ./prisma/migrations \
--to-schema-datamodel ./prisma/schema.prisma \
--shadow-database-url "$SHADOW_DATABASE_URL"
```
### Manually editing a migration file breaks future deploys
Prisma checksums every migration file. Editing after apply causes `P3006 checksum mismatch` on every environment where the original already ran. Create a new migration instead.
### Breaking schema changes require multi-step migration
Adding `NOT NULL` to an existing column or renaming a column in one migration will lock the table or drop data. Use expand-and-contract:
```bash
# Step 1: create migration locally, then deploy
npx prisma migrate dev --name add_new_column # local only
npx prisma migrate deploy # staging / production
```
```ts
// Step 2: backfill data (run in a script or migration job, not in the shell)
await prisma.user.updateMany({ data: { newColumn: derivedValue } });
```
```bash
# Step 3: create the NOT NULL constraint migration locally, then deploy
npx prisma migrate dev --name make_new_column_required # local only
npx prisma migrate deploy # staging / production
```
### `@updatedAt` does not fire on `updateMany`
`@updatedAt` is set automatically only on `update` and `upsert`. Bulk writes leave it stale.
```ts
// BAD: updatedAt stays at its old value
await prisma.post.updateMany({ where: { authorId }, data: { published: true } });
// GOOD
await prisma.post.updateMany({
where: { authorId },
data: { published: true, updatedAt: new Date() },
});
```
### Soft delete + `findUniqueOrThrow` leaks deleted records
`findUniqueOrThrow` throws `P2025` only when the row does not exist in the DB. Soft-deleted rows still exist and are returned without error.
`findUniqueOrThrow` requires a unique constraint field in `where` — adding `deletedAt: null` alongside `id` breaks the type because `{ id, deletedAt }` is not a compound unique constraint. Use `findFirstOrThrow` instead.
```ts
// BAD: returns soft-deleted user
const user = await prisma.user.findUniqueOrThrow({ where: { id } });
// BAD: Prisma type error — { id, deletedAt } is not a unique constraint
const user = await prisma.user.findUniqueOrThrow({ where: { id, deletedAt: null } });
// GOOD: findFirstOrThrow supports arbitrary where conditions
const user = await prisma.user.findFirstOrThrow({ where: { id, deletedAt: null } });
```
### `deleteMany` without `where` deletes every row
```ts
// BAD: silently wipes the table
await prisma.post.deleteMany();
// GOOD
await prisma.post.deleteMany({ where: { authorId: userId } });
```
## Best Practices
| Rule | Reason |
|---|---|
| `migrate deploy` in CI/CD, `migrate dev` only locally | `migrate dev` can reset the DB on drift |
| Map entities to response DTOs | Prevents leaking internal fields |
| Catch `PrismaClientKnownRequestError` at service boundary | Translate to domain errors |
| Prefer `*OrThrow` methods over manual null checks | Throws P2025 automatically; use `findFirstOrThrow` when filtering non-unique fields |
| `connection_limit=1` + external pooler in serverless | Prevents connection exhaustion |
| Always provide `where` on `deleteMany` | Prevents accidental table wipe |
| Set `updatedAt: new Date()` manually in `updateMany` | `@updatedAt` skips bulk writes |
## Related Skills
- `nestjs-patterns` — NestJS service layer that integrates Prisma
- `postgres-patterns` — PostgreSQL-level indexing and connection tuning
- `database-migrations` — multi-step migration planning for production
- `backend-patterns` — general API and service layer design

View File

@@ -130,12 +130,12 @@ test('candidate playbook preserves stale-salvage operating rules', () => {
}
});
test('roadmap points to the evaluator RAG prototype and hosted PR check', () => {
test('roadmap points to the evaluator RAG prototype and keeps hosted integration open', () => {
const roadmap = read('docs/ECC-2.0-GA-ROADMAP.md');
assert.ok(roadmap.includes('docs/architecture/evaluator-rag-prototype.md'));
assert.ok(roadmap.includes('examples/evaluator-rag-prototype/'));
assert.ok(roadmap.includes('Deterministic hosted PR check and cached output scoring integrated; hosted retrieval remains future'));
assert.ok(roadmap.includes('Local corpus complete; hosted integration remains future'));
});
test('billing readiness scenario rejects launch copy overclaims', () => {

View File

@@ -89,6 +89,110 @@ function runTests() {
assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`);
}) ? passed++ : failed++);
// --- Subshell bypass regression (issue: dev server slipped past via $(), ``, ()) ---
if (!isWindows) {
(test('blocks $(npm run dev) — command substitution', () => {
const result = runScript('$(npm run dev)');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
assert.ok(result.stderr.includes('BLOCKED'), 'expected BLOCKED in stderr');
}) ? passed++ : failed++);
(test('blocks `npm run dev` — backtick substitution', () => {
const result = runScript('`npm run dev`');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks echo $(npm run dev) — substitution nested in argument', () => {
const result = runScript('echo $(npm run dev)');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks (npm run dev) — plain subshell group', () => {
const result = runScript('(npm run dev)');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks $(echo a; npm run dev) — substitution with sequenced segments', () => {
const result = runScript('$(echo a; npm run dev)');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks (pnpm dev) — plain subshell group with pnpm', () => {
const result = runScript('(pnpm dev)');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('allows tmux launcher inside subshell wrapping (exit code 0)', () => {
const result = runScript('(tmux new-session -d -s dev "npm run dev")');
assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`);
}) ? passed++ : failed++);
(test('allows single-quoted "(npm run dev)" — literal string, not a subshell', () => {
const result = runScript("git commit -m '(npm run dev)'");
assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`);
}) ? passed++ : failed++);
(test('allows double-quoted "(npm run dev)" — literal in double quotes (bash does not subshell)', () => {
const result = runScript('echo "(npm run dev)"');
assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`);
}) ? passed++ : failed++);
(test("allows single-quoted '$(npm run dev)' — literal string, no substitution", () => {
const result = runScript("git commit -m '$(npm run dev) fix'");
assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`);
}) ? passed++ : failed++);
}
// --- Round 1 review fixes (Greptile + CodeRabbit on PR #1889) ---
if (!isWindows) {
(test('blocks $(echo ")"; (npm run dev)) — quoted ) does not terminate $() early', () => {
const result = runScript('$(echo ")"; (npm run dev))');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks (echo ")"; npm run dev) — quoted ) does not terminate (...) early', () => {
const result = runScript('(echo ")"; npm run dev)');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('allows $(echo "(npm run dev)") — () inside double-quoted substitution body is literal', () => {
const result = runScript('$(echo "(npm run dev)")');
assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks { npm run dev; } — brace group runs in current shell', () => {
const result = runScript('{ npm run dev; }');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks echo hi && { npm run dev; } — brace group after &&', () => {
const result = runScript('echo hi && { npm run dev; }');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('allows {npm run dev} — bash requires space after { to form a group', () => {
const result = runScript('{npm run dev}');
assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks yarn run dev — yarn 1.x convention', () => {
const result = runScript('yarn run dev');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks bun dev — bun bare form', () => {
const result = runScript('bun dev');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
(test('blocks "$(npm run dev)" — double-quoted substitution still substitutes', () => {
const result = runScript('echo "$(npm run dev)"');
assert.strictEqual(result.code, 2, `Expected exit code 2, got ${result.code}`);
}) ? passed++ : failed++);
}
// --- Edge cases ---
(test('empty/invalid input passes through (exit code 0)', () => {