Skip to content

fix: isolate Claude safety hooks per run - #812

Merged
tomdps merged 30 commits into
mainfrom
codex/migrate-pr-806-to-main
Jul 29, 2026
Merged

fix: isolate Claude safety hooks per run#812
tomdps merged 30 commits into
mainfrom
codex/migrate-pr-806-to-main

Conversation

@tomdps

@tomdps tomdps commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Main-trunk migration

Replaces #806 with the same three isolated commits rebased onto the new main. Commit authorship is preserved; the implementation diff is unchanged apart from conflict resolution against the trunk cutover.

Original PR body

Summary

  • stop installing Zeroshot safety hooks into the user-level Claude settings
  • create a mode-0600 temporary settings overlay for each Claude task and load it through the supported --settings flag
  • preserve normal user, project, local, credential, plugin, skill, and MCP discovery
  • add the dangerous-git hook only for worktree tasks and clean the overlay after the provider exits
  • route the overlay path through the provider command contract and cover the isolation behavior with regression tests

Root cause

The prior project-overlay helper returned null when a repository had no Claude config. The hook installers treated that null as permission to fall back to CLAUDE_CONFIG_DIR or ~/.claude, so the first Zeroshot run mutated global Claude settings.

Validation

  • npm run check:agent-cli-provider:ci — 135 passing
  • focused hook/config suite — 52 passing
  • npm run check — pass (0 errors; existing warnings only)
  • full npm test — 1,697 passing, 18 pending; 7 unrelated PTY tests fail in this worktree because node-pty reports posix_spawnp failed
  • Python hooks compile successfully with python3 -m py_compile
  • Opcore targeted validation is degraded because this repository has no configured Python type-checker authority for the two comment-only hook files; direct syntax and hook behavior checks pass

Migration note

This prevents future global settings mutations. It deliberately does not rewrite existing user-level Claude settings to remove hook entries installed by an older Zeroshot version, because an automatic cleanup would itself mutate user-owned global configuration.

Fixes #43

@greptile-apps

greptile-apps Bot commented Jul 27, 2026

Copy link
Copy Markdown

Greptile Summary

This PR isolates Claude safety configuration and strengthens detached-task cleanup ownership.

  • Creates per-run Claude settings overlays and forwards them through the provider command contract.
  • Preserves repository MCP configuration while validating required Claude CLI capabilities.
  • Persists cleanup and spawn-ownership metadata across watcher, cancellation, termination, and recovery paths.
  • Adds regression coverage for overlay isolation, cleanup safety, task launch ambiguity, and watcher recovery.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/worktree-claude-config.js Creates and validates private per-run Claude settings overlays and restricts recursive cleanup to canonical owned directories.
src/agent-cli-provider/adapters/claude.ts Adds Claude settings and MCP arguments with capability checks and deliberate variadic-argument termination.
src/agent/agent-task-executor.js Routes overlay ownership through local and isolated task launches and retains task handles when termination cannot be confirmed.
src/task-spawn-cleanup-ownership.js Defines the durable spawn-token boundary used to transfer command cleanup from the caller to the detached task lifecycle.
task-lib/command-spec-cleanup.js Validates closed cleanup receipts and limits deletion to canonical Zeroshot-owned overlay directories or schema files.
task-lib/watcher-output-runtime.js Coordinates provider termination, output persistence, and cleanup receipt retention or clearing.
task-lib/store.js Persists unique spawn-ownership tokens, cancellation intent, termination boundaries, and retryable cleanup metadata.

Sequence Diagram

sequenceDiagram
    participant Agent
    participant Wrapper as Task wrapper
    participant Store as Task store
    participant Watcher
    participant Claude

    Agent->>Agent: Create settings overlay and spawn token
    Agent->>Wrapper: Start task with overlay path and token
    Wrapper->>Store: Persist task, token, and cleanup receipt
    Wrapper->>Watcher: Start detached watcher
    Watcher->>Claude: Launch with --settings and --mcp-config
    Wrapper-->>Agent: Return durable task ID
    Claude-->>Watcher: Exit
    Watcher->>Watcher: Confirm provider boundary is terminal
    Watcher->>Watcher: Remove owned overlay
    Watcher->>Store: Persist terminal status and clear cleanup receipt
Loading

Reviews (23): Last reviewed commit: "fix(test): avoid cleanup sentinel proces..." | Re-trigger Greptile

@tomdps
tomdps force-pushed the codex/migrate-pr-806-to-main branch from 013005d to 0cce889 Compare July 27, 2026 15:09
@tomdps
tomdps force-pushed the codex/migrate-pr-806-to-main branch from 0cce889 to c8df481 Compare July 27, 2026 15:51
@tomdps
tomdps added this pull request to the merge queue Jul 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 29, 2026
@tomdps
tomdps added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit a04552e Jul 29, 2026
13 of 16 checks passed
@tomdps
tomdps deleted the codex/migrate-pr-806-to-main branch July 29, 2026 02:15
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 6.10.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Move hook configuration to project path

1 participant