Skip to content

fix(fleet): sanitize slashed branch names in inbox file paths#605

Merged
Wirasm merged 2 commits into
mainfrom
fix/fleet-slashed-branch-inbox
Feb 26, 2026
Merged

fix(fleet): sanitize slashed branch names in inbox file paths#605
Wirasm merged 2 commits into
mainfrom
fix/fleet-slashed-branch-inbox

Conversation

@Wirasm

@Wirasm Wirasm commented Feb 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add fleet_safe_name() to replace / with - in branch names used as inbox filenames — refactor/foo becomes refactor-foo.json instead of trying to create refactor/foo.json (nested path that fails with ENOENT)
  • Applied consistently across agent flags (--agent-name, --agent-id), inbox creation, config.json entries, inbox removal, and inject writes
  • Clarify the dual inbox/dropbox communication flow in the brain agent instructions — previously described as two disconnected systems with no explanation of how they fit together

Test plan

  • cargo fmt --check — no violations
  • cargo clippy --all -- -D warnings — no warnings
  • cargo test --all — all pass (6 new fleet tests)
  • Manual: kild create refactor/test-inject --daemon --agent claude creates flat refactor-test-inject.json inbox file
  • Manual: kild inject refactor/test-inject "test" delivers to both dropbox and Claude inbox
  • Manual: kild inbox refactor/test-inject shows task with correct delivery methods

Branch names with `/` (e.g. `refactor/consolidate-ipc`) were used raw
as inbox filenames, creating nested directory paths that don't exist.
Add `fleet_safe_name()` to replace `/` with `-` consistently across
agent flags, inbox creation, config entries, and inject writes.

Also clarify the dual inbox/dropbox communication flow in the brain
agent instructions — the two channels were described separately with
no explanation of how they fit together.
@Wirasm

Wirasm commented Feb 26, 2026

Copy link
Copy Markdown
Owner Author

PR Review Summary

Reviewed by 5 specialized agents: code-reviewer, silent-failure-hunter, pr-test-analyzer, comment-analyzer, docs-impact-agent.

Critical Issues: 0

Important Issues: 2

1. DRY violation — fleet_safe_name() duplicated in inject.rs

  • crates/kild/src/commands/inject.rs:104 uses branch.replace('/', "-") instead of calling fleet_safe_name()
  • fleet is pub(super) in sessions/mod.rs, so inject.rs genuinely can't access it currently
  • Fix: re-export fleet_safe_name from kild-core (e.g., pub use fleet::fleet_safe_name; in sessions/mod.rs) and call it in inject.rs
  • Without this, the two implementations are coupled by a comment, not the compiler

2. update_team_config missing eprintln! on write/serialize failure

  • crates/kild-core/src/sessions/fleet.rs:279-293 — config write and serialize failures log to tracing but produce no user-facing output
  • Every other failure path in this file has both warn! + eprintln!; the parallel remove_from_team_config does it correctly
  • Pre-existing issue, but this PR touches the function — natural moment to fix

Suggestions

Agent Suggestion Location
comment-analyzer Add fleet_agent_id() to fleet_safe_name() doc comment's usage list fleet.rs:27-37
comment-analyzer Update ensure_fleet_member() doc from <branch>.json to <safe_name>.json fleet.rs:121-127
comment-analyzer Update remove_fleet_member() doc similarly fleet.rs:319-321
comment-analyzer Update fleet_agent_id() doc from <branch>@honryu to <safe_name>@honryu fleet.rs:297-300
docs-impact Update CLAUDE.md lines 418+420 — fleet section still says <branch> for agent flags and inbox path CLAUDE.md:418-420

Test Coverage: Adequate, 2 improvements suggested

Rating Suggestion
7/10 Add remove_fleet_member round-trip test with slashed branch (create → remove → assert gone)
6/10 Add inject.rs test for slashed branch sanitization as divergence canary

Strengths

  • Core fix is correct and consistently applied across all 5 callsites in fleet.rs
  • Tests are well-targeted: positive + negative assertions, identity + edge cases
  • kild-brain.md rewrite clearly explains the dual inbox/dropbox flow
  • Error handling in new code paths is solid (no silent failures introduced)

Verdict: NEEDS FIXES — address the 2 important issues, then good to merge

- Re-export fleet_safe_name() from kild-core so inject.rs calls the
  canonical function instead of duplicating the logic inline
- Add eprintln! to update_team_config write/serialize failure paths
  for consistency with all other error paths in fleet.rs
- Update doc comments to reflect sanitized names (<safe_name> instead
  of <branch>) on ensure_fleet_member, remove_fleet_member, and
  fleet_agent_id
- Add fleet_agent_id() to fleet_safe_name() usage list in doc comment
- Update CLAUDE.md fleet section to document sanitized agent flags
  and inbox paths
- Add remove_fleet_member round-trip test with slashed branch
@Wirasm Wirasm merged commit a76ff27 into main Feb 26, 2026
6 checks passed
@Wirasm Wirasm deleted the fix/fleet-slashed-branch-inbox branch February 26, 2026 20:17
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