Skip to content

fix(ci): bypass delegation-guard during merge conflict resolution - #133

Merged
aram-devdocs merged 3 commits into
mainfrom
fix/delegation-guard-merge-bypass
Apr 27, 2026
Merged

fix(ci): bypass delegation-guard during merge conflict resolution#133
aram-devdocs merged 3 commits into
mainfrom
fix/delegation-guard-merge-bypass

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Summary

  • Allow .claude/hooks/delegation-guard.sh to permit root Rust source edits while the worktree has unmerged paths.
  • Add a regression test that creates a temporary conflicted Git repository and verifies root Rust edits are allowed only in that merge-conflict state.

Why

Merge conflict marker arbitration is mechanical cleanup, not new Rust implementation work, so forcing it through 01-implementer conflicts with that subagent's TDD-oriented contract.

Validation

  • bash -n .claude/hooks/delegation-guard.sh .claude/hooks/tests/test_delegation_guard_subagent.sh
  • bash .claude/hooks/tests/test_delegation_guard_subagent.sh
  • git diff --check
  • cargo fmt --check passed after installing the pinned Rust 1.95.0 toolchain under /workspace.
  • cargo clippy --workspace --all-targets --all-features -- -D warnings could not complete in this container because no C linker (cc, gcc, or clang) is installed. A bundled rust-lld retry also failed due missing libc development linker names/startup environment, so cargo test was not run here for the same environment reason.

@aram-devdocs

Copy link
Copy Markdown
Owner Author

Updated delegation-guard.sh so root Rust source edits are allowed while the worktree has unmerged paths, and added a regression test that creates a temporary merge conflict to verify that behavior. Local shell validation passed (bash -n, delegation guard regression test, git diff --check) and cargo fmt --check passed using the pinned Rust 1.95.0 toolchain installed under /workspace; cargo clippy and cargo test need a human or CI rerun because this container lacks a usable C linker/libc development environment (cc/gcc/clang unavailable, rust-lld could not link libc-backed build scripts).

@github-actions

github-actions Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

I have a full picture. Here is the review.


PR #133 — Delegation Guard: Merge-Conflict Bypass

Buckets touched: ci (hooks), gitignore, docs (crates/plumb-mcp/AGENTS.md)
Rust code changed: none — all five blocker categories (unsafe, unwrap, println!, SystemTime, HashMap-in-output) are N/A.


Punch list

.claude/hooks/delegation-guard.sh:49–56 — conflict-bypass logic

Correct and narrowly scoped:

  • --diff-filter=U lists only files with unresolved merge conflicts; it drops staged/modified files, so the exception disappears the moment Claude resolves and git adds the file. This is verified by test case 3.
  • grep -qxF "$rel_path" is exact-line matching — a partial path like lib.rs cannot match crates/plumb-core/src/lib.rs. No false-positive risk.
  • || true prevents set -euo pipefail from aborting when the hook runs outside a git repo.
  • Placement is correct: the check runs after subagent and infra-path early-exits, so only root-context Rust files under crates/*/src/ reach this branch.
  • Security surface is minimal: engineering a fake merge conflict to exploit this bypass requires controlling the git state of the worktree, which is a much stronger position than what this hook defends against.

.claude/hooks/tests/test_delegation_guard_subagent.sh:121–174 (committed version) — three new test cases

  • Case 1 (allow conflicted file): correct.
  • Case 2 (block unrelated file during conflict): correct — crates/plumb-cli/src/main.rs is not in conflicted_paths, so falls through to the hook_block role check.
  • Case 3 (block after git add): correct — once staged, --diff-filter=U stops listing the file, so the bypass no longer fires.

The test repo setup (git merge feature producing a real conflict) is the right approach — it exercises the actual git plumbing rather than mocking it.

crates/plumb-mcp/AGENTS.md

Removing the 09-mcp-tool-author one-liner is fine; the agent is still registered at .claude/agents/09-mcp-tool-author.md and referenced in the subagent-driven-development and gh-issue skills. No dead-reference problem.

The phrase "for the handoff path" is slightly opaque but not wrong. Non-blocking.

.gitignore.claude-pr/ entry

Clean, no issues.

Working-tree state (not a PR defect)

The review context shows staged/unstaged reverts of the bypass code in the working tree. These are not part of the PR commits and appear to be artifacts of a prior review run. The PR commits at 39ed3d7 are correct.

HOOK_CWD / rel_path interaction

HOOK_CWD is set from $CLAUDE_PROJECT_DIR (or git rev-parse --show-toplevel) in _lib.sh. The test sets CLAUDE_PROJECT_DIR="$merge_repo" so git -C "$HOOK_CWD" runs against the scratch repo with the live conflict. Works as intended. Pre-existing edge case (subdirectory CLAUDE_PROJECT_DIR) is not introduced by this PR.


Verdict: APPROVE

@aram-devdocs aram-devdocs changed the title fix(hooks): bypass delegation-guard during merge conflict resolution fix(ci): bypass delegation-guard during merge conflict resolution Apr 27, 2026
@aram-devdocs

aram-devdocs commented Apr 27, 2026

Copy link
Copy Markdown
Owner Author

PR #133 is ready to merge: the PR title now uses the allowed ci scope, crates/plumb-mcp/AGENTS.md is back under the 60-line preflight cap, .claude-pr/ is ignored, and Claude review punch-list items were addressed by narrowing the merge-conflict bypass to only the unresolved Rust file plus adding negative regression coverage. GitHub CI is green after rerunning one transient setup-layer Determinism failure, and the latest Claude review verdict is APPROVE.

@aram-devdocs
aram-devdocs merged commit 3c951d0 into main Apr 27, 2026
23 of 24 checks passed
aram-devdocs added a commit that referenced this pull request Apr 27, 2026
Resolves the conflicts that arose after PR #131 (lint_url over real
http(s)://) and PR #133 (delegation-guard merge bypass) merged into
main while this branch carried PR #132 (get_config tool).

- crates/plumb-mcp/src/lib.rs: keep both new use lines —
  plumb_cdp::{BrowserDriver, ChromiumDriver, ChromiumOptions, Target,
  is_fake_url} from main and plumb_config::ConfigError from this
  branch. Bring in ViewportKey alongside Config/PlumbSnapshot/etc.
- Cargo.lock: union plumb-cdp and plumb-config under plumb-mcp.

Note: local cargo check is blocked by a disk-space failure in the
sandbox toolchain ('No space left on device' while syncing the
1.95.0 channel). Verification deferred to CI.
This was referenced Apr 27, 2026
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