19 Commits
Author SHA1 Message Date
JongHyeok ParkandGitHub 28b922dee3 fix(hooks): preserve Stop output through lifecycle wrappers (#2493)
Preserve complete Stop-hook stdout through lifecycle wrappers, wait for queued output to flush before exiting, bound child output with a larger explicit buffer, and add end-to-end regressions for large, multibyte, dry-run, and failure cases.
2026-07-26 00:01:57 -07:00
JongHyeok ParkandGitHub 0071fa5c3c refactor(hooks): consolidate PostToolUse hooks into sync/async dispatchers (#2494)
* refactor(hooks): consolidate PostToolUse hooks into sync/async dispatchers

Replace 10 individual PostToolUse entries in hooks.json with two
consolidated dispatcher entries (post:dispatcher:sync /
post:dispatcher:async). The dispatcher's internal registry preserves
every hook ID, matcher, and profile, so ECC_DISABLED_HOOKS and
ECC_HOOK_PROFILE gating behave exactly as before.

Performance (Edit event, actual hooks.json commands spawned in
parallel like the harness does, median of 7 runs):
- Blocking hook latency: 81ms -> 49ms (~40% faster; 7 blocking
  processes -> 1 sync dispatcher)
- Node processes per tool call: 10 -> 2 (7 blocking + 3 async
  -> 1 sync + 1 async)
- observe-runner now runs in-process (~370ms) inside the async
  dispatcher, which stays backgrounded (async: true, timeout 45s),
  so it adds no user-facing latency.

Also:
- dashboard-web lists dispatcher-managed child hooks so the hook
  inventory stays complete
- post-edit-console-warn refactored to export run() for in-process
  dispatch while keeping standalone stdin behavior
- dispatcher stdin reading is multi-byte safe (StringDecoder) and
  child hook exit codes propagate to the dispatcher exit code

* test(hooks): replace emoji literal with unicode escape for CI unicode safety check

* fix(hooks): adopt explicit cli() entrypoint and merge multi-hook stdout

Address Greptile review on #2494:

- Replace the non-standard 'require.main === undefined' guard with an
  explicit exported cli(). The hooks.json bootstraps now call
  require(s).cli(), so merely requiring the module (dashboard-web,
  test runners, Jest, worker threads) can never trigger dispatch,
  attach stdin listeners, or set process.exitCode.
- Replace last-writer-wins stdout with mergeHookStdout(): when several
  hooks emit additionalContext envelopes they merge into a single
  PostToolUse envelope; non-mergeable raw stdout keeps the last hook's
  output and emits a stderr warning naming the dropped hook IDs, so
  nothing is lost silently.

Also includes local formatter reformatting of the dispatcher and its
test file (no behavioral changes beyond the above).

* fix(hooks): keep post:bash:dispatcher phase reachable in minimal profile

The Greptile P1 premise was partially incorrect: sub-hooks without
explicit profiles default to standard,strict via parseProfiles()
(scripts/lib/hook-flags.js), so audit/cost logs never ran under the
minimal profile on main either — there is no user-visible regression.

However, main did spawn the bash dispatcher phase unconditionally and
let each sub-hook gate itself. Restore that semantic by opening the
outer registry gate to minimal,standard,strict so a future sub-hook
that opts into minimal is not silently blocked at the phase level.
Adds the previously missing minimal-profile async dry-run test.

* test(hooks): assert failing hook exit code propagates to real process status

Spawns the actual dispatcher subprocess with an injected failing hook
and asserts the OS-level exit status, stderr diagnostic, and suppressed
pass-through — closing the E2E gap CodeRabbit flagged on #2494.

* chore: retrigger CI (flaky windows powershell bootstrap test)
2026-07-19 15:47:10 -04:00
JongHyeok ParkandGitHub ed38744605 fix(hooks): require successful shell probe in observe runner (#2403)
Require observe-runner shell detection to accept only candidates whose probe exits successfully, avoiding Windows WSL launcher false positives.
2026-07-13 21:31:12 -04:00
JongHyeok ParkGitHubAffaan Mustafadevin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>affaanBoube
914a58a716 feat(workflows): re-land orch-review workflow + add /orch-review command (#2400)
* feat(workflows): re-land orch-review workflow + add /orch-review command

Re-lands #2363 (reverted by #2393 to unbreak main's lint) and fixes the
root cause so it stays green:

- Restore workflows/orch-review.workflow.js + workflows/README.md.
- eslint.config.js: ignore 'workflows/**/*.workflow.*' and '.claude/workflows/**'
  per the maintainer's note in #2393. Workflow DSL scripts use both top-level
  export (ESM) and top-level return (the runtime wraps them in an async fn),
  which no single eslint sourceType can parse — they must be excluded, not
  lint-fixed. 'npx eslint .' is green with this ignore.
- Add commands/orch-review.md (the /orch-review surface) + regenerate
  docs/COMMAND-REGISTRY.json.

Supersedes #2397 (command-only), which referenced the reverted workflow.

* fix(workflows): address orch-review bot review findings

- Verifier uncertainty no longer demotes blockers (Greptile P1 + CodeRabbit):
  isReal=false only refutes when confidence >= 0.8; low-confidence 'false'
  is treated as uncertain and kept blocking (fail closed).
- Treat the diff (and finding text) as untrusted input in both review and
  verify prompts; ignore embedded directives (prompt-injection hardening).
- Validate changedFiles entries are strings, not just that it is an array.
- Enforce proof for HIGH/CRITICAL in FINDINGS_SCHEMA, not only in the prompt.
- Remove in-place mutation in dimension build + dedup merge (immutable).
- /orch-review: extract & validate a numeric PR id before shelling out to gh.
- Docs: complete the stats example, soften wording, refresh follow-up list.

* style(workflows): apply formatter to orch-review assembly

* fix(plan-orchestrate): detect ecc@ecc marketplace + emit ecc: agent prefix (#2316) (#2409)

* fix(plan-orchestrate): detect ecc@ecc marketplace + emit ecc: agent prefix (#2316)

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(ci): resync lockfiles with package.json (eslint 10) + migrate yarn.lock to Yarn 4 format

package.json requires eslint@^10.6.0 but the committed locks pinned 9.39.2, so
npm ci aborted and Yarn 4 hardened mode rejected the stale v1-classic yarn.lock
(YN0028). Regenerate package-lock.json and rewrite yarn.lock in Yarn 4 (berry)
format so npm ci and immutable yarn installs both pass.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(ci): require clean probe exit for Windows shell/bash detection; add pyyaml dev dep

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: affaan <affaan@itomarkets.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* refactor: consolidate duplicated hook-root resolver into shared resolveEccRoot() (#2368) (#2410)

* fix(ci): resync lockfiles with package.json (eslint 10) + migrate yarn.lock to Yarn 4 format

package.json requires eslint@^10.6.0 but the committed locks pinned 9.39.2, so
npm ci aborted and Yarn 4 hardened mode rejected the stale v1-classic yarn.lock
(YN0028). Regenerate package-lock.json and rewrite yarn.lock in Yarn 4 (berry)
format so npm ci and immutable yarn installs both pass.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(ci): require clean probe exit for Windows shell/bash detection; add pyyaml dev dep

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* refactor: consolidate duplicated hook-root resolver into shared resolveEccRoot() (#2368)

The inline node -e resolver blob was duplicated ~60x across hooks.json,
command docs, and translations. Each copy inlined the full ~700-char
plugin-root search using a spread over nested array literals
(p.join(d,'plugins',...s) over [['ecc'],...]), which breaks Windows hook
execution due to shell quoting (#2368).

Collapse every copy to a 250-char locator that loads the committed
resolve-ecc-root module and delegates to resolveEccRoot() — no spread, no
nested array literals, no escaped double quotes. The real search logic now
lives in one tested module. Also route session-start-bootstrap.js through
resolveEccRoot() instead of its own duplicated reimplementation, and fix
the auto-update.md 'marketplace' (singular) typo along the way.

Guard tests updated: discovery behavior is asserted against resolveEccRoot();
the inline is asserted to delegate and to contain no Windows-fragile
constructs.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(resolve-ecc-root): restore full env-unset discovery in inline resolver

Address Greptile review on #2410: when CLAUDE_PLUGIN_ROOT is unset the
delegating inline could only load the resolver module from ~/.claude,
returning ~/.claude without ever reaching the plugin/cache search. Restore
the old inline's discovery breadth (exact plugin roots + versioned cache)
Windows-safely (no spread, nested arrays, or escaped quotes), then delegate
the authoritative decision to resolveEccRoot(). Add regression tests for
plugin-subdir and versioned-cache bootstrap with env unset.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: affaan <affaan@itomarkets.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: docs/COMMAND-REGISTRY.json check fails on fresh Windows clone (missing .gitattributes) (#2437)

* fix: add .gitattributes to force LF line endings for text files

npm run command-registry:check (part of npm test) fails on a fresh clone
on Windows with the common core.autocrlf=true setting: git checks out
docs/COMMAND-REGISTRY.json with CRLF, but generate-command-registry.js
always writes LF, so the strict string comparison in checkRegistry()
never matches. Forcing LF via .gitattributes makes checkouts consistent
across platforms regardless of a contributor's local autocrlf setting.

* fix: normalize CRLF line endings to LF per .gitattributes

pyproject.toml, src/llm/__init__.py, src/llm/prompt/builder.py,
src/llm/providers/claude.py, and tests/test_builder.py had CRLF line
endings committed to the repo, inconsistent with the rest of the
codebase. Renormalized via 'git add --renormalize .' now that
.gitattributes enforces eol=lf.

---------

Co-authored-by: Affaan Mustafa <me@affaanmustafa.com>

* chore(catalog): sync command counts (92->93) + register orch-review in agent.yaml surface

---------

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: affaan <affaan@itomarkets.com>
Co-authored-by: Boube <109886533+Cb2i@users.noreply.github.com>
Co-authored-by: Affaan Mustafa <me@affaanmustafa.com>
2026-07-03 20:26:40 -07:00
JongHyeok ParkandGitHub 1c3a989ea6 refactor(commands): remove duplicated content in skill-create and learn-eval (#2348)
skill-create: drop the "Example Output" section (53 lines) — it re-rendered
the same skeleton already defined by the Step 3 output template, just with
filled-in `my-app` values.

learn-eval: drop the "Next Action" column from the 5b verdict table — it
duplicated Step 6's "Verdict-specific confirmation flow". The table now
carries Verdict + Meaning, and a pointer to Step 6 as the single source for
each verdict's action.

No behavior, frontmatter, or design-rationale changes.
2026-06-29 18:38:36 -07:00
JongHyeok ParkandGitHub 7976e6faf2 feat(skills): make tdd-workflow test-runner aware (npm/pnpm/yarn/bun) (#2347)
* feat(skills): make tdd-workflow test-runner aware (npm/pnpm/yarn/bun)

Add "Step 0: Detect the Test Runner" so the RED/GREEN cycle no longer
hardcodes `npm test`. Distinguishes the package manager from the test
runner (a project can install with Bun yet run Jest/Vitest), adds a runner
command matrix, and warns about `bun test` (native bun:test runner) vs
`bun run test` (runs the package.json script) — a common ESM failure mode.
Adds a Bun native test pattern section and links the bun-runtime skill.

Applied to both the canonical skills/ copy and the .agents/skills/ Codex
subset (manual sync per CONTRIBUTING).

* docs(skills): apply <test>/<coverage> placeholders in tdd-workflow steps

Address review feedback on PR #2347: Step 0 instructs the agent to substitute
the detected runner command, but Steps 3/5/7, Run Coverage Report, Watch Mode,
Pre-Commit, and CI/CD still showed literal `npm test` / `npm run test:coverage`
— so an agent reaching those blocks could run npm test on a pnpm/bun project.
Replace them with the <test> / <test-watch> / <coverage> placeholders from
Step 0. Left untouched: the plan-handoff allowlist example and the Step 8
evidence-table samples (illustrative, not run-this instructions). Applied to
both the canonical and Codex-subset copies.

* docs(skills): make pre-commit lint runner-agnostic via <lint> placeholder

Follow-up to PR #2347 review (CodeRabbit): the pre-commit example still used
`npm run lint`, coupling it to npm after test/coverage were made runner-aware.
Add a `<lint>` column to the Step 0 runner matrix (npm run lint / pnpm lint /
yarn lint / bun run lint) and change the Pre-Commit Hook example to
`<test> && <lint>`. Applied to both the canonical and Codex-subset copies.

* chore: re-trigger CI (flaky windows/node20 npm cell)
2026-06-29 18:38:33 -07:00
JongHyeok ParkandGitHub 1031d312cc feat(workflows): add orch-review native Workflow pilot (#2363)
* feat(workflows): add orch-review native Workflow pilot

Port orch-pipeline Phase 5 (Review) to a native Claude Code Workflow
script. The gated outer loop stays in the main conversation; this script
owns only the autonomous review+verify segment between the two human
gates:

1. Review — reviewers fan out in parallel: ecc:code-reviewer always,
   ecc:<language>-reviewer when args.language maps, ecc:security-reviewer
   when the orch-pipeline security trigger matches the diff/paths.
2. Dedup — merge findings across dimensions keyed on the normalized
   evidence snippet, since independent reviewers flag the same line.
3. Verify — each unique CRITICAL/HIGH finding goes to an independent
   adversarial verifier; MEDIUM/LOW pass through as advisory.

The Review->Verify barrier is deliberate: deduping before verification
stops the verifier running N times on the same bug (local testing: 11
raw findings collapsed to 4 unique, ~halving verifier cost).

Existing ECC reviewer subagents are reused via agentType; reviewer
output is validated by JSON schema. args is accepted as an object or a
JSON-encoded string.

- workflows/orch-review.workflow.js — the workflow script
- workflows/README.md — invocation contract, returns shape, follow-ups

CI lint is scoped to scripts/ and tests/, so the script (validated with
node --check) and the README (passes markdownlint) are untouched.

* fix(workflows): fail closed on invalid args and lost review dimensions

Addresses the two safety findings from the PR bot review:

1. Lost review dimension (Greptile P1 / CodeRabbit Major): a reviewer
   agent that returns null or rejects was silently dropped by
   filter(Boolean), so an unreviewed security dimension could still
   return APPROVE. Each dimension's outcome is now captured; failures
   land in failedDimensions and force CHANGES_REQUESTED (incomplete).

2. Invalid args (CodeRabbit Major): an empty diff returned APPROVE and
   bad JSON / non-array changedFiles threw inconsistently. Input is now
   validated up front and rejected with a clear error — the gate fails
   closed instead of approving an unreviewed payload.

Docs (header contract + README) updated for the new return fields
(incomplete, failedDimensions, stats.failed). Remaining bot nits
(evidence minLength, verify-label collision, verified->confirmed
rename, contract drift) deferred as follow-ups.

* fix(workflows): address remaining orch-review review nits

Follow-up to the bot review (deferred items from the safety pass):

- evidence: require minLength 1 in the schema, and fall back to a
  title+line dedup key when evidence is empty, so empty-evidence
  findings in one file no longer collapse onto a single key and drop
  (CodeRabbit).
- verify label: include a slice of the normalized evidence so two
  CRITICAL/HIGH findings from the same file get distinct labels and do
  not alias under resumability (Greptile).
- stats.verified -> stats.confirmed to match the "confirmed" wording
  used in the log and avoid ambiguity vs the refuted count (Greptile);
  header contract and README updated to match.

Verified by running the workflow on a synthetic vulnerable diff:
dedup 12 raw -> 5 unique, stats.confirmed populated, fail-closed fields
(incomplete/failedDimensions) intact.

* fix(workflows): harden verify stage and diff-only verification

Addresses the second-round bot review:

- Verify stage now has the same failure guard as the review stage: a
  rejected verifier no longer nulls out its slot (which crashed the
  later filter). A null return is treated as unconfirmed; a rejection
  keeps the finding as blocking (fail closed) so an unverifiable
  CRITICAL is never silently demoted to advisory (CodeRabbit @221).
- verifyPrompt now instructs the skeptic to judge solely from the
  provided diff text and not to refute merely because the referenced
  file is absent from the working tree (the diff may be an unapplied
  PR). Fixes the false-refute seen when testing on a synthetic diff.

CodeRabbit @81 (evidence minLength) was already addressed in the prior
commit; this is a stale re-post on the unresolved thread.

* fix(workflows): keep unverifiable blockers blocking; stop leaking error text

Second-round bot review (CodeRabbit):

- @218 Treat a null/failed verifier as `unverified`, not refuted. A
  terminal verifier failure or skip no longer demotes a CRITICAL/HIGH
  to advisory; it stays in `blocking` tagged "could not be verified"
  (fail closed). Only a genuine isReal=false verdict is refuted. Adds
  stats.unverified.
- @189 Do not return raw subagent error text. Review/verify failures
  now log the raw message for operators and return only a bounded label
  (failedDimensions[].error = "review agent failed").

Stale re-posts this round (@81 evidence minLength, @224 verify guard)
were already fixed in prior commits.

* docs(workflows): enumerate bounded failedDimensions.error labels

CodeRabbit (trivial): the public contract implied callers get
human-readable error text, but the implementation returns only bounded
labels. Enumerate them in the README returns block.
2026-06-29 15:50:41 -07:00
Jonghyeok Park f6b10481f3 fix: add spawnSync error logging and restore 5s timeout
- Check spawnSync result and log warning on failure via stderr
- Restore osascript timeout to 5000ms, increase hook deadline to 10s
  for sufficient headroom
2026-03-25 16:03:21 +09:00
Jonghyeok Park d3699f9010 fix: use AppleScript-safe escaping and reduce spawnSync timeout
- Replace JSON.stringify with curly quote substitution for AppleScript
  compatibility (AppleScript does not support \" backslash escapes)
- Reduce spawnSync timeout from 5000ms to 3000ms to leave headroom
  within the 5s hook deadline
2026-03-25 16:03:21 +09:00
Jonghyeok Park 445ae5099d feat: add macOS desktop notification Stop hook
Add a new Stop hook that sends a native macOS notification with the
task summary (first line of last_assistant_message) when Claude finishes
responding. Uses osascript via spawnSync for shell injection safety.
Supports run-with-flags fast require() path. Only active on standard
and strict profiles; silently skips on non-macOS platforms.
2026-03-25 16:03:21 +09:00
JongHyeok ParkandGitHub 83d3279fd8 feat(skills): add Kysely migration patterns to database-migrations (#731)
* feat(skills): add Kysely migration patterns to database-migrations

Add Kysely section covering kysely-ctl CLI workflow, migration file
structure (up/down with Kysely<any>), and programmatic Migrator setup
with FileMigrationProvider and allowUnorderedMigrations option.

* fix(skills): address PR review feedback for Kysely migration patterns

- Replace redundant email index with avatar_url index (unique already creates index)
- Add ESM-compatible __dirname using import.meta.url
- Comment out allowUnorderedMigrations with production safety warning
- Add clarifying comment for db variable

* fix(skills): fix migration filename mismatch and clarify ESM-only pattern

- Rename migration file to create_user_profile to match actual content
- Restructure ESM import pattern to be clearly ESM-only with CJS note
2026-03-22 15:40:53 -07:00
Jonghyeok Park 67841042d6 refactor: deduplicate config lists and unify resolveFormatterBin branches
Extract BIOME_CONFIGS and PRETTIER_CONFIGS as shared constants to eliminate
duplication between PROJECT_ROOT_MARKERS and detectFormatter(). Unify the
biome/prettier branches in resolveFormatterBin() via a FORMATTER_PACKAGES
map. Remove redundant path.resolve() in quality-gate.js.
2026-03-11 10:45:28 +09:00
Jonghyeok Park 0a3afbe38f fix(hooks): add Windows .cmd support with shell injection guard
Handle Windows .cmd shim resolution via spawnSync with strict path
validation. Removes shell:true injection risk, uses strict equality,
and restores .cmd support with path injection guard.
2026-03-11 10:45:28 +09:00
Jonghyeok Park 66498ae9ac perf(hooks): use direct require() instead of spawning child process
Invoke hook scripts directly via require() when they export a
run(rawInput) function, eliminating one Node.js process spawn per
hook invocation (~50-100ms).

Includes path traversal guard, timeouts, error logging, PR review
feedback, legacy hooks guard, normalized filePath, and restored
findProjectRoot config detection with package manager support.
2026-03-11 10:45:27 +09:00
Jonghyeok Park e5d02000c3 perf(hooks): eliminate npx overhead and merge biome invocations
- Use local node_modules/.bin/biome binary instead of npx (~200-500ms savings)
- Change post-edit-format from `biome format --write` to `biome check --write`
  (format + lint in one pass)
- Skip redundant biome check in quality-gate for JS/TS files already
  handled by post-edit-format
- Fix quality-gate to use findProjectRoot instead of process.cwd()
- Export run() function from both hooks for direct invocation
- Update tests to match shared resolve-formatter module usage
2026-03-10 22:19:31 +09:00
Jonghyeok Park f331d3ecc9 feat(hooks): add shared resolve-formatter utility with caching
Extract project-root discovery, formatter detection, and binary
resolution into a reusable module. Caches results per-process to
avoid redundant filesystem lookups on every Edit hook invocation.

This is the foundation for eliminating npx overhead in format hooks.
2026-03-10 22:17:02 +09:00
Jonghyeok Park 3260c7449e fix(lint): remove unnecessary escape characters in regex patterns
- doc-file-warning.js: \/ → / inside character classes (4 occurrences)
- project-detect.js: \[ → [ inside character classes (2 occurrences)

These are pre-existing no-useless-escape errors on upstream main.
2026-03-03 22:11:51 +09:00
Jonghyeok Park 66143eaf74 chore: add .prettierrc for consistent code formatting
The post-edit-format hook runs Prettier on JS/TS files after edits,
but without a project-level config it applied default settings (double
quotes, etc.) that conflicted with the existing code style. Adding
.prettierrc ensures the hook respects the project conventions.

Settings derived from existing codebase analysis:
- singleQuote: true
- trailingComma: none
- arrowParens: avoid
- printWidth: 200
2026-03-03 22:06:09 +09:00
Jonghyeok Park 4eb6fbdd3f feat: auto-detect formatter in post-edit hook (Biome/Prettier)
The post-edit-format hook was hardcoded to use Prettier. Projects using
Biome had their code reformatted with Prettier defaults (e.g. double
quotes overwriting single quotes).

Now the hook walks up from the edited file to find the project root,
then checks for config files:
- biome.json / biome.jsonc → runs Biome
- .prettierrc / prettier.config.* → runs Prettier
- Neither found → skips formatting silently
2026-02-20 14:30:01 +09:00