Repository navigation
Conversation
📝 WalkthroughWalkthroughThe PR adds deterministic slugified task branch names, parses item IDs from slugged branches, and replaces local ChangesTask branch naming
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
getappz
changed the title
added 2 commits
August 12, 2026 14:31
…nment blocker: this worktree's git metadata directory (`/home/avihs/projects/agentflare/.git/worktrees/89`) sits on a read-only mount — the whole main repo filesystem is `ro`, with only this worktree's own subtree bind-mounted `rw`. `git commit` can't create `index.lock` there, and there's no privilege escalation available (uid 1000, sudo blocked by `no new privileges`). Even the vent-tool call to report this failed with the same read-only error, so the environment is broadly read-only outside the worktree bind mount — I can't work around this myself.
**What's done (uncommitted, staged):**
- `crates/flare-git-core/src/worktree.rs`: added `task_branch_name(item)` — lowercases/slugs `item.name`, collapses non-alphanumeric runs to `-`, trims, truncates to 40 chars, falls back to bare `task/{sequence_id}` if the slug is empty. Wired into `create_worktree` and `push_branch` (replacing the two duplicated `format!("task/{}", ...)` calls), and into the two hardcoded test helpers plus several other tests that were also hardcoding `"task/1"`. Added 4 new unit tests for `task_branch_name` (normal title, empty title, all-symbol/unicode title, long title).
- `crates/flare-git-core/src/provenance.rs`: extracted `item_id_from_branch` and fixed it to take only the leading digit run after `task/` instead of the whole remainder, so a slugged branch's `Agentflare-Item:` trailer still resolves to just the numeric id. Added a sibling test for the slugged form plus a non-task-branch case.
- Directory path (`.worktrees/task/{sequence_id}`) left numeric-only as specified.
- `cargo test -p flare-git-core --lib` (37 relevant tests) and `cargo build -p agentflare` both pass clean, no new clippy warnings.
**Flagging back, not fixing (per scope instructions):** three more places hardcode the bare `task/{sequence_id}` pattern and will now diverge from the real (slugged) branch name — these are functional, not cosmetic:
- `src/worktree.rs:32` (`branch_diverged`), `:69` (`is_pr_merged` — GitHub PR lookup by branch name), `:108` (`pr_ci_status` — same lookup for CI status)
- `src/mcp_server/handoff.rs:122` (feeds `verify_continuation_commit`'s branch-ref check)
All four independently recompute `format!("task/{}", item.sequence_id)` rather than calling shared logic; once branches are slugged, these lookups will silently miss (soft-fail paths already in place) since they'll search for the wrong branch name. They'd need `task_branch_name` made `pub` (or re-exported) and wired in.
I could not commit these changes myself — the worktree is stuck read-only at the git-metadata level. Someone with access to the host filesystem will need to either fix the mount or manually pull the diff from this worktree and commit it.
…k/<id>
branch_diverged, is_pr_merged, pr_ci_status (src/worktree.rs) and the
verify_continuation_commit branch lookup (handoff.rs) independently
recomputed the bare format!("task/{sequence_id}") form, which stopped
matching once task_branch_name started appending a title slug -- a real
regression, not just formatting. task_branch_name is now pub so these
callers (a different crate) can use it directly. Also fixes 3 test call
sites in action_tests.rs with the same hardcoded pattern, plus a
cargo fmt pass on worktree_tests.rs.
Agentflare-Agent: claude-code
Agentflare-Branch: task/89
Agentflare-Item: 89
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/flare-git-core/src/provenance.rs`:
- Around line 26-31: Update item_id_from_branch to accept a task branch only
when the digit sequence consumes the remainder of the branch name or is
immediately followed by “-”; return None for suffixes such as “17foo”. Add tests
covering malformed suffixes and preserve the existing supported branch forms.
In `@crates/flare-git-core/src/worktree.rs`:
- Around line 262-268: Update the claim flow that uses task_branch_name so the
selected branch name is persisted on the claimed item and reused by reclaim,
push, divergence checks, and PR lookup paths instead of recomputing it from
item.name. Preserve the existing fallback naming behavior, and add an
integration test covering renaming an item after claim before push and
completion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 280249bc-5494-4327-9640-f28232aff413
📒 Files selected for processing (6)
crates/flare-git-core/src/provenance.rscrates/flare-git-core/src/worktree.rscrates/flare-git-core/src/worktree_tests.rssrc/mcp_server/handoff.rssrc/mcp_server/tests/action_tests.rssrc/worktree.rs
Comment on lines
+26
to
+31
| let rest = branch?.strip_prefix("task/")?; | ||
| let digits: String = rest.chars().take_while(|c| c.is_ascii_digit()).collect(); | ||
| if digits.is_empty() { | ||
| None | ||
| } else { | ||
| Some(digits) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject task branches without a separator after the sequence ID.
item_id_from_branch(Some("task/17foo")) returns Some("17"). This branch does not match either supported form. It can attach provenance to the wrong item.
Require the digit run to end at the end of the string or immediately before -. Add malformed-suffix test cases.
Proposed fix
let digits: String = rest.chars().take_while(|c| c.is_ascii_digit()).collect();
-if digits.is_empty() {
+let suffix = &rest[digits.len()..];
+if digits.is_empty() || (!suffix.is_empty() && !suffix.starts_with('-')) {
None
} else {
Some(digits)
} assert_eq!(item_id_from_branch(Some("main")), None);
assert_eq!(item_id_from_branch(Some("task/")), None);
+assert_eq!(item_id_from_branch(Some("task/17foo")), None);
+assert_eq!(item_id_from_branch(Some("task/17/foo")), None);
assert_eq!(item_id_from_branch(None), None);Also applies to: 150-153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flare-git-core/src/provenance.rs` around lines 26 - 31, Update
item_id_from_branch to accept a task branch only when the digit sequence
consumes the remainder of the branch name or is immediately followed by “-”;
return None for suffixes such as “17foo”. Add tests covering malformed suffixes
and preserve the existing supported branch forms.
Comment on lines
+262
to
+268
| pub fn task_branch_name(item: &Item) -> String { | ||
| let slug = slugify_branch_title(&item.name); | ||
| if slug.is_empty() { | ||
| format!("task/{}", item.sequence_id) | ||
| } else { | ||
| format!("task/{}-{slug}", item.sequence_id) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# The ctx_* tools are unavailable in this review environment.
# Inspect whether UpdateItem permits title changes and trace branch consumers.
ast-grep outline crates/agentflare-backend/src/item.rs --items all --match UpdateItem
rg -n -C 5 'UpdateItem|name:' crates/agentflare-backend/src/item.rs src crates
rg -n -C 5 'task_branch_name|create_worktree|push_branch|branch_diverged' src cratesRepository: getappz/agentflare
Length of output: 50375
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- item update implementation ---'
sed -n '260,360p' crates/agentflare-backend/src/item.rs
printf '%s\n' '--- worktree branch helper and nearby callers ---'
sed -n '220,330p' crates/flare-git-core/src/worktree.rs
printf '%s\n' '--- all task_branch_name references ---'
rg -n -C 8 'task_branch_name' crates src
printf '%s\n' '--- worktree and branch operation references ---'
rg -n -C 5 'create_worktree|worktree|push_branch|branch_diverged|diverg|reclaim|completion|complete' crates/flare-git-core crates src -g '*.rs' | head -n 1200Repository: getappz/agentflare
Length of output: 50375
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
item = Path("crates/agentflare-backend/src/item.rs").read_text()
worktree = Path("crates/flare-git-core/src/worktree.rs").read_text()
main_worktree = Path("src/worktree.rs").read_text()
assert re.search(r"struct\s+UpdateItem\s*\{.*?pub name:\s*Option<String>", item, re.S)
assert re.search(r'if input\.name\.is_some\(\).*?name\s*=\s*\?{param_idx}', item, re.S)
assert "pub fn task_branch_name(item: &Item)" in worktree
assert len(re.findall(r"task_branch_name\s*\(\s*item\s*\)", worktree)) >= 2
assert "task_branch_name(item)" in main_worktree
print("UpdateItem.name is mutable through item::update.")
print("Branch consumers recompute task_branch_name(item) from the current Item.")
print("No persisted branch field is present in UpdateItem or the task_branch_name API.")
PY
printf '%s\n' '--- claim and reclaim paths ---'
rg -n -C 10 'pub fn claim|ClaimOutcome|already_isolated_for|create_worktree\(' crates/agentflare-backend/src/item.rs crates/flare-git-core/src/worktree.rs src -g '*.rs' | head -n 900
printf '%s\n' '--- branch persistence/schema candidates ---'
rg -n -C 4 'branch(_name)?|worktree_path|worktree' crates/agentflare-backend crates/flare-git-core src -g '*.rs' | rg -v 'target_branch|task_branch_name|worktree_tests' | head -n 900
printf '%s\n' '--- title-update tests ---'
rg -n -C 8 'UpdateItem\s*\{[^}]*name|name:\s*Some|rename|renam|title' crates/agentflare-backend/src/item.rs src crates/flare-git-core/src/worktree_tests.rs -g '*.rs' | head -n 700Repository: getappz/agentflare
Length of output: 50375
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- push and cleanup branch resolution ---'
sed -n '540,660p' crates/flare-git-core/src/worktree.rs
sed -n '1,145p' src/worktree.rs
printf '%s\n' '--- completion and publish flow ---'
rg -n -C 12 'push_and_open_pr|mark_completed|cleanup_worktree|cleanup_item_worktree|item_done' src/mcp_server crates/flare-git-core -g '*.rs' | head -n 1000
printf '%s\n' '--- item update call sites that can change name ---'
rg -n -C 12 'item::update|update\(\s*conn|UpdateItem' src crates -g '*.rs' | rg -B 12 -A 12 'name:|UpdateItem' | head -n 1000
printf '%s\n' '--- existing worktree title-stability coverage ---'
rg -n -C 10 'rename|renamed|title.*claim|claim.*title|name:\s*Some|UpdateItem' src/mcp_server/tests crates/flare-git-core/src/worktree_tests.rs crates/agentflare-backend/src/item.rs -g '*.rs' | head -n 1000Repository: getappz/agentflare
Length of output: 50375
Persist the task branch name when the item is claimed.
If item.name changes after claim, reclaim, push, divergence checks, and PR lookups derive a new branch name while the worktree remains on the original branch. Persist the selected branch name, or reject title changes after claim. Add an integration test that renames a claimed item before push and completion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flare-git-core/src/worktree.rs` around lines 262 - 268, Update the
claim flow that uses task_branch_name so the selected branch name is persisted
on the claimed item and reused by reclaim, push, divergence checks, and PR
lookup paths instead of recomputing it from item.name. Preserve the existing
fallback naming behavior, and add an integration test covering renaming an item
after claim before push and completion.
Source: Coding guidelines
6 tasks done
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.
All code changes are complete and pass tests, but I hit a hard environment blocker: this worktree's git metadata directory (
/home/avihs/projects/agentflare/.git/worktrees/89) sits on a read-only mount — the whole main repo filesystem isro, with only this worktree's own subtree bind-mountedrw.git commitcan't createindex.lockthere, and there's no privilege escalation available (uid 1000, sudo blocked byno new privileges). Even the vent-tool call to report this failed with the same read-only error, so the environment is broadly read-only outside the worktree bind mount — I can't work around this myself.What's done (uncommitted, staged):
crates/flare-git-core/src/worktree.rs: addedtask_branch_name(item)— lowercases/slugsitem.name, collapses non-alphanumeric runs to-, trims, truncates to 40 chars, falls back to baretask/{sequence_id}if the slug is empty. Wired intocreate_worktreeandpush_branch(replacing the two duplicatedformat!("task/{}", ...)calls), and into the two hardcoded test helpers plus several other tests that were also hardcoding"task/1". Added 4 new unit tests fortask_branch_name(normal title, empty title, all-symbol/unicode title, long title).crates/flare-git-core/src/provenance.rs: extracteditem_id_from_branchand fixed it to take only the leading digit run aftertask/instead of the whole remainder, so a slugged branch'sAgentflare-Item:trailer still resolves to just the numeric id. Added a sibling test for the slugged form plus a non-task-branch case..worktrees/task/{sequence_id}) left numeric-only as specified.cargo test -p flare-git-core --lib(37 relevant tests) andcargo build -p agentflareboth pass clean, no new clippy warnings.Flagging back, not fixing (per scope instructions): three more places hardcode the bare
task/{sequence_id}pattern and will now diverge from the real (slugged) branch name — these are functional, not cosmetic:src/worktree.rs:32(branch_diverged),:69(is_pr_merged— GitHub PR lookup by branch name),:108(pr_ci_status— same lookup for CI status)src/mcp_server/handoff.rs:122(feedsverify_continuation_commit's branch-ref check)All four independently recompute
format!("task/{}", item.sequence_id)rather than calling shared logic; once branches are slugged, these lookups will silently miss (soft-fail paths already in place) since they'll search for the wrong branch name. They'd needtask_branch_namemadepub(or re-exported) and wired in.I could not commit these changes myself — the worktree is stuck read-only at the git-metadata level. Someone with access to the host filesystem will need to either fix the mount or manually pull the diff from this worktree and commit it.
Summary by CodeRabbit