Skip to content

MUTATING_TOOLS case mismatch blocks pre-tool-use branch guard for OpenCode - #191

Merged
getappz merged 3 commits into
masterfrom
task/70
Jul 15, 2026
Merged

MUTATING_TOOLS case mismatch blocks pre-tool-use branch guard for OpenCode#191
getappz merged 3 commits into
masterfrom
task/70

Conversation

@getappz

@getappz getappz commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Auto-opened on item done for 019f6446-86b5-7240-a4d8-85ff8582458e.

Summary by CodeRabbit

  • Bug Fixes
    • Improved branch protection by recognizing additional lowercase tool names as potentially mutating operations.
    • Ensured redirect and blocking decisions are applied consistently across supported tool-name formats.
  • Tests
    • Added coverage to verify lowercase edit, write, and notebookedit are classified correctly.

OpenCode uses edit/write (lowercase). MUTATING_TOOLS had
only PascalCase variants -> branch guard never matched.
Add lowercase dups so the guard fires regardless of agent
tool naming convention.
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d96004d7-bb5c-4f3e-be0d-888d891437d8

📥 Commits

Reviewing files that changed from the base of the PR and between b802a9e and c6403a9.

📒 Files selected for processing (1)
  • src/hook_redirect.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/hook_redirect.rs

📝 Walkthrough

Walkthrough

Adds lowercase write, edit, and notebookedit variants to mutating-tool recognition and tests their branch-guard classification.

Changes

Redirect classification

Layer / File(s) Summary
Lowercase mutating tool matching and coverage
src/hook_redirect.rs
MUTATING_TOOLS recognizes lowercase tool names, and a unit test verifies their classification on master.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

  • getappz/agentflare#170: Both changes update tool-name-based redirect classification in src/hook_redirect.rs.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is off-topic and omits the required Summary, Test plan, and reviewer notes sections. Replace it with the repository template and add a summary, test plan, and reviewer notes for risk and compatibility.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the case-mismatch fix for OpenCode branch-guard tool matching.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task/70

Comment @coderabbitai help to get the list of available commands.

@getappz

getappz commented Jul 15, 2026

Copy link
Copy Markdown
Owner Author

Re-reviewed (same commit `b802a9e3` as the task/70 worktree review) — still approved, no changes since. 3-line fix, does what it says, CI all green.

Only thing outstanding: `mergeStateStatus: BEHIND` — master moved on since this branch was cut, so it needs an update (merge/rebase) before it'll merge cleanly. No actual conflict expected, just needs syncing.

Still stand by the earlier minor note: no regression test pins the specific bug (lowercase `edit`/`write` now matching) — existing tests only cover PascalCase. Not blocking.

@getappz
getappz merged commit 2690262 into master Jul 15, 2026
14 checks passed
@getappz
getappz deleted the task/70 branch July 15, 2026 07:12
getappz added a commit that referenced this pull request Aug 27, 2026
…t finalize (#622)

* No clippy warnings in the files I touched. The implementation is complete and verified.

## Status

Implemented per-turn checkpoint commits + finalize-time squash in `build_sdd_loop_step`/`build_finalize_step` (`src/work_item_pipeline.rs`), building on #191's `worktree_path` threading:

- **`crates/flare-git-core/src/worktree.rs`**: added `commit_uncommitted_at` (path-based variant of `commit_uncommitted`, with a `no_verify` flag), `head_sha`, and `squash_since` (`git reset --soft`). `commit_uncommitted` now delegates to `commit_uncommitted_at`. Thin wrappers added in `src/worktree.rs`.
- **`WorkItemData`**: added `checkpoint_base_sha: Option<String>`.
- **`build_sdd_loop_step`**: role dispatch now also returns `is_implementer_turn` (true only for non-review-only implementer/fix dispatches, never for judge/reviewer/analyst turns). After a successful implementer-turn `send()`, `checkpoint_implementer_turn` commits with `--no-verify` (skipping the LOC-freeze gate per open question #1) and lazily captures the pre-checkpoint HEAD sha.
- **`build_finalize_step`**: right before the existing `item_done` call, squashes every checkpoint commit back to `checkpoint_base_sha` via `squash_since`, so `item_done`'s own `commit_uncommitted` (which does run the LOC-freeze pre-commit hook) evaluates the whole run's diff as one commit. `.take()` on the sha prevents a step retry from double-squashing.
- Commit message convention: `wip(sdd-loop): task {id} checkpoint` (open question #2) — disposable, just grep/reflog-identifiable.

**One real bug caught and fixed during implementation**: my first draft fell back to the ambient process cwd when `worktree_path` was empty (mirroring `real_agent_send_hook`'s dispatch fallback). Running the test suite actually committed my own uncommitted working-tree changes into this session's real git history, since the test harness has no `worktree_path` set and ambient cwd was this repo. I reset that commit (`git reset --soft`, verified only my own 3 in-progress files were affected) and changed the checkpoint function to no-op entirely when `worktree_path` is empty — a `git commit` has no safe ambient fallback, unlike read-only agent dispatch. Added a regression test (`checkpoint_implementer_turn_is_a_noop_without_a_worktree_path`) guarding this specifically.

**Tests added**: 4 in `flare-git-core/src/worktree_tests.rs` (`commit_uncommitted_at`, `head_sha` ×2, `squash_since`), 2 in `work_item_pipeline/tests.rs` (no-op guard, and an end-to-end commit+squash test against a real claimed worktree). All pass — 207/207 (flare-git-core), 83/83 (work_item_pipeline module, 2 pre-existing unrelated ignores).

**Verification**: full builds clean, `scripts/loc-gate.sh` passes on every file I touched (had to trim doc comments twice to stay under work_item_pipeline.rs's 1500-line limit — now 1498), clippy shows no new warnings in touched files. One pre-existing repo-wide LOC-gate failure in `src/mcp_server/tests/item_tests.rs` (untouched by me, and per the gate script's own comments, not wired into CI) — not in scope.

No concerns beyond the above; nothing committed, per instructions this is left for your review.

Agentflare-Agent: claude-code_2-1-245_agent
Agentflare-Branch: task/193-sdd-loop-commit-progress-after-each-impl
Agentflare-Item: 193-sdd-loop-commit-progress-after-each-impl

* fmt

Agentflare-Agent: claude-code
Agentflare-Branch: pr-622-fmt-fix
Agentflare-Session: e77fc32e-33d0-4884-ab55-fdda48fe45fd

---------

Co-authored-by: shiva <shiva@gosysinfo.tech>
getappz pushed a commit that referenced this pull request Aug 27, 2026
…ta on push_and_open_pr (found-existing and created-new paths) without clobbering other metadata keys; is_pr_merged/pr_ci_status check metadata.pr.number first via pulls::get, falling back to branch heuristic only when absent. Verified: cargo build --lib clean, 21/21 worktree:: tests pass (incl. #191 regression + merge-no-clobber tests), clippy clean on worktree.rs. Not committed.

Agentflare-Agent: claude-code_2-1-245_agent
Agentflare-Branch: task/196-store-pr-identity-in-item-metadata-at-op
Agentflare-Item: 196-store-pr-identity-in-item-metadata-at-op
getappz added a commit that referenced this pull request Aug 27, 2026
…ta on push_and_open_pr (found-existing and created-new paths) without clobbering other metadata keys; is_pr_merged/pr_ci_status check metadata.pr.number first via pulls::get, falling back to branch heuristic only when absent. Verified: cargo build --lib clean, 21/21 worktree:: tests pass (incl. #191 regression + merge-no-clobber tests), clippy clean on worktree.rs. Not committed. (#623)

Agentflare-Agent: claude-code_2-1-245_agent
Agentflare-Branch: task/196-store-pr-identity-in-item-metadata-at-op
Agentflare-Item: 196-store-pr-identity-in-item-metadata-at-op

Co-authored-by: shiva <shiva@gosysinfo.tech>
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