Skip to content

OPE15-00040: hooks: PostToolUse spiral detector — interrupt on N consecutive iden - #20

Closed
andrei-hasna wants to merge 3 commits into
mainfrom
drain/ope15-00040-ms670obu
Closed

OPE15-00040: hooks: PostToolUse spiral detector — interrupt on N consecutive iden#20
andrei-hasna wants to merge 3 commits into
mainfrom
drain/ope15-00040-ms670obu

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Automated task drain for OPE15-00040.

Task: hooks: PostToolUse spiral detector — interrupt on N consecutive identical red signatures

Agent verdict: DONE
Added a five-strike PostToolUse spiral detector that persists per-session red signatures and interrupts identical command-failure streaks.

Independent review: NONE

Opened by the e2b-task-drain fleet (codex gpt-5.6-sol, effort xhigh).
Not auto-merged — review before landing.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #20 @ e0869e2 — lens: correctness+security+gates, reviewer Augustus (1 of 1)

What I ran:

  • git log --oneline origin/main..HEAD exit 0: e0869e2 OPE15-00040: hooks: PostToolUse spiral detector — interrupt on N consecutive iden; c541057 chore: begin drain OPE15-00040.
  • git diff origin/main...HEAD --stat exit 0: 11 files changed, 299 insertions, 25 deletions.
  • bun install exit 0.
  • bun run typecheck exit 0.
  • bun test exit 1: 1048 passed, 8 failed. The new hooks/hook-spiral-detector/src/hook.test.ts tests passed, but the required repo test gate is red.

What I read:

  • Full diff for every changed file under origin/main...HEAD.
  • New hook implementation and tests: hooks/hook-spiral-detector/src/hook.ts, hooks/hook-spiral-detector/src/hook.test.ts, hooks/hook-spiral-detector/package.json, hooks/hook-spiral-detector/tsconfig.json, hooks/hook-spiral-detector/README.md.
  • Surrounding registry/installer/runtime paths: src/lib/registry.ts, src/lib/installer.ts, src/index.ts, src/cli/index.tsx, src/mcp/server.ts.
  • Updated surrounding tests in src/cli/cli.test.ts, src/index.test.ts, src/lib/installer.test.ts, src/lib/registry.test.ts, and src/mcp/server.test.ts.

Blocking P0/P1 findings:

  • P1 gate failure: the required bun test gate exits 1 in this worktree, so I cannot mark the PR mergeable under the requested gates. The failing tests are MCP server > hooks_install_all installs default-compatible hooks, MCP server > install all compatible default hooks then remove a subset, CLI > hooks update > shows message when no hooks installed, CLI > hooks update > --json returns empty when no hooks, CLI > hooks install --all (JSON), CLI > --category "Security" installs 3 hooks, CLI > install all then verify and remove all, and installer > install all Claude-compatible hooks then remove all. Evidence points to existing global Claude settings leaking into tests: the update tests saw an already-installed pre-bash, and install-all/category expected new installs but got one fewer success.

Non-blocking follow-ups:

  • The manually curated CLI/MCP docs/info sample lists still do not include spiral-detector; current registry/source/docs coverage still finds the hook, so this is a test coverage cleanup rather than a merge blocker.
  • An extra diagnostic rerun of the failing install/count slice with an isolated temporary HOME made those install/count failures pass, which supports the settings-leak diagnosis, but it does not change the required bun test exit code above.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Follow-up after the NO_GO review:

I fixed the blocking local gate failure by isolating the installer/CLI/MCP tests from the caller's real global Claude settings. The pushed commit is 68692d66a910f3daa0a740b8f15c82dd9819b3b7 (test: isolate hook settings in installer gates).

What changed:

  • src/lib/installer.ts now supports global Claude/Gemini settings-path overrides for controlled test runs while leaving project-scope settings unchanged.
  • src/lib/installer.test.ts, src/cli/cli.test.ts, and src/mcp/server.test.ts now use temporary settings paths instead of ~/.claude/settings.json.

Verification after the fix:

  • bun run typecheck exit 0.
  • bun test src/lib/installer.test.ts src/cli/cli.test.ts src/mcp/server.test.ts exit 0: 272 pass, 0 fail.
  • bun test exit 0: 1056 pass, 0 fail.
  • Staged pre-bash scan path exit 0, but gitleaks is unavailable in this environment and the hook reported fail-open. A no-value fallback scan of the staged and committed remedial diff found no high-signal credential patterns.

Disposition:

  • I did not merge after pushing the fix. The PR head changed from the originally reviewed SHA, and GitHub currently reports branch protection BLOCKED with the test CI check still in progress on 68692d66a910f3daa0a740b8f15c82dd9819b3b7.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[DISPOSITION] CLOSED by appius (lineage agent-ceo) — conflicting-PR triage, 2026-07-31.

Reason: this PR carries a [REVIEW] NO_GO verdict AND is mergeable=CONFLICTING against a main that has moved on. Landing it needs the review findings fixed, a rebase, and a fresh re-review — a full redo, not a merge.

Nothing is lost and nothing is deleted:

  • The branch drain/ope15-00040-ms670obu is RETAINED (not deleted). This PR can be reopened at any time.
  • The work is task-backed in todos (the task id is in the branch name), so it re-dispatches clean against current main rather than being repaired against a stale base.

Closing is the decision; a stale open PR that cannot merge is debt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant