fix(kickoff): bind-mount parent .git inside container + broaden detection / add kickoff.allowed_tools#592
Merged
dollspace-gay merged 1 commit intoMay 11, 2026
Conversation
…tion / add kickoff.allowed_tools (#733, GH#584) Two related bugs from GH#584 that together prevented kickoff agents from verifying their own work inside the container. ## Issue 1 — worktree `.git` pointer unresolvable in container `git worktree add` creates a `.git` *file* (not directory) inside the worktree containing `gitdir: <host>/.git/worktrees/<branch>/` -- an absolute host path. The container bind-mounts the worktree at `/workspaces/repo` but the parent repo's `.git/` directory was never mounted, so the gitdir pointer dangled and every git operation inside the container failed. Fix: `launch_container` takes a new `host_repo_root: &Path` parameter and adds a bind mount `-v <host>/.git:<host>/.git:rw` so the gitdir pointer resolves. `rw` is correct: the per-worktree subdir under `.git/worktrees/<branch>/` legitimately needs writes (HEAD, index, refs) for the agent to commit. Hook policy still blocks the genuinely destructive git ops regardless of mount mode. ## Issue 2 — `--allowedTools` missing language build tools `detect_conventions` already auto-added `Bash(cargo *)` etc. when a build manifest existed, but it only checked the repo root (plus the hardcoded special-case `crosslink/Cargo.toml`). Monorepos with manifests in any other subdir hit "Cannot run cargo (sandbox-denied)" even with `--skip-permissions`. Fix (chose "broaden detection + new config key"): - New `has_manifest` helper in `kickoff/helpers.rs` scans the repo root AND one directory level deep, skipping `SKIP_SCAN_DIRS` (`node_modules`, `target`, `vendor`, all hidden dirs, etc.) to avoid false positives from vendored builds. Detection now picks up `crates/foo/Cargo.toml`, `python-svc/pyproject.toml`, etc. The hardcoded `crosslink/Cargo.toml` special case is removed -- the generic scan subsumes it. `Bash(pytest *)` added to the Python toolset since GH#584 named it explicitly. - New `read_kickoff_allowed_tools(crosslink_dir)` helper reads a `kickoff.allowed_tools` array from `hook-config.json`. `run.rs` appends it to `conventions.allowed_tools` after detection. Purely additive -- the array adds patterns, never removes. Registered in `config_registry.rs` (Agents group, hot-swappable, StringArray) and documented in `docs_src/reference/hook-config.qmd`. The two fixes combine: detection catches the common 95% case; the config key handles the rest (manifests buried deeper than one level, custom build wrappers, MCP tool names). ## Verification - `cargo fmt --check` clean - `cargo clippy --lib --bin crosslink -- -D warnings` clean - `cargo test --bin crosslink` -- 2817 passed (2808 baseline + 9 new) 9 new unit tests: - `test_detect_conventions_rust_in_subdir` -- monorepo Cargo.toml one level deep enables `Bash(cargo *)` - `test_detect_conventions_rust_two_levels_deep_not_detected` -- asserts the contract limit, prevents future false positives - `test_detect_conventions_python_in_subdir_with_pytest` -- explicit GH#584 case - `test_detect_conventions_skips_node_modules` -- stray manifests in vendored dirs ignored - `test_detect_conventions_skips_hidden_dirs` -- same for `.cache/` and friends - 4 tests for `read_kickoff_allowed_tools` covering missing file, present array, absent key, and malformed JSON Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes GH#584. Two related bugs that together prevented kickoff agents from verifying their own work inside containers.
Issue 1 — worktree
.gitpointer unresolvable in containergit worktree addcreates a.gitfile (not directory) inside the worktree containinggitdir: <host>/.git/worktrees/<branch>/— an absolute host path. The container bind-mounted the worktree at/workspaces/repobut the parent repo's.git/directory was never mounted, so the gitdir pointer dangled and every git operation inside the container failed (git status,git diff,crosslink sync,crosslink commit, evencargoreading workspace metadata).Fix:
launch_containertakes a newhost_repo_root: &Pathparameter and adds a bind mount-v <host>/.git:<host>/.git:rwso the gitdir pointer resolves.rwis correct: the per-worktree subdir under.git/worktrees/<branch>/legitimately needs writes (HEAD, index, refs) for the agent to commit. Hook policy still blocks the genuinely destructive git ops regardless of mount mode.Issue 2 —
--allowedToolsmissing language build toolsdetect_conventionsalready auto-addedBash(cargo *)etc. when a build manifest existed, but it only checked the repo root (plus a hardcodedcrosslink/Cargo.tomlspecial case). Monorepos with manifests in any other subdir hit "Cannot run cargo (sandbox-denied)" even with--skip-permissions.Fix (chose broaden detection + new config key):
has_manifesthelper inkickoff/helpers.rsscans the repo root and one directory level deep, skippingSKIP_SCAN_DIRS(node_modules,target,vendor, all hidden dirs, etc.) to avoid false positives. The hardcodedcrosslink/Cargo.tomlspecial case is removed — the generic scan subsumes it.Bash(pytest *)added to the Python toolset since GH#584 named it explicitly.read_kickoff_allowed_tools(crosslink_dir)reads akickoff.allowed_toolsarray fromhook-config.json.run.rsappends it toconventions.allowed_toolsafter detection. Purely additive override knob. Registered inconfig_registry.rs(Agents group, hot-swappable, StringArray) and documented indocs_src/reference/hook-config.qmd.The two fixes combine: detection catches the common 95% case; the config key handles the rest (manifests buried deeper than one level, custom build wrappers, MCP tool names).
Test plan
cargo fmt --check— cleancargo clippy --lib --bin crosslink -- -D warnings— cleancargo test --bin crosslink— 2817 passed (2808 baseline + 9 new)read_kickoff_allowed_tools(missing file, present array, absent key, malformed JSON)crosslink kickoff run --container docker ...against santana-style monorepo, confirmgit statusworks inside the container and the agent successfully runscargo testcrosslink config set kickoff.allowed_tools '["Bash(make deploy *)"]', confirm the pattern appears in the agent's--allowedToolsafter a fresh kickoff🤖 Generated with Claude Code