Repository navigation
Conversation
…for AI agent shells Compiled binary, copied per tool name into ~/.agentflare/shims/, mirroring mise's crates/mise-shim pattern. Solves the gap where Claude Code's PowerShell tool runs pwsh.exe -NoProfile -NonInteractive, so anything gated behind $PROFILE (lean-ctx's own shell-hook.ps1, or a .bashenv-style function) never loads — PATH resolution of bare command names still works regardless of -NoProfile, so a shim dir on PATH still gets hit. Gate order mirrors .bashenv's _af_dispatch: kill switches -> agent-env marker -> .agentflare project walk-up (stopping at $HOME, since ~/.agentflare is agentflare's own app-data dir, not a project marker) -> lean-ctx -c <tool> -> real-binary fallback. Strict no-op for anything outside that double gate.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change registers a new ChangesAgentflare shim integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Shell
participant Shim as agentflare-shim
participant LeanCtx as lean-ctx
participant Tool as real tool
Shell->>Shim: invoke tool with arguments
Shim->>Shim: evaluate environment and project scope
alt dispatch enabled
Shim->>LeanCtx: run -c tool
LeanCtx-->>Shim: exit status
alt unavailable
Shim->>Tool: execute real binary
Tool-->>Shim: exit status
end
else dispatch disabled
Shim->>Tool: execute real binary
Tool-->>Shim: exit status
end
Shim-->>Shell: return exit status
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Merged
1 of 3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/agentflare-shim/src/main.rs`:
- Around line 136-145: Remove the status-code-based fallback from the command
execution match in main: specifically, do not call run_real based on child exit
codes 126 or 127. Preserve and use an explicit lean-ctx fallback signal if one
exists, while retaining run_real only for the Err(_) process-spawn failure path,
and otherwise exit with the child’s returned code.
- Around line 74-77: Update path_without_shim_dir to normalize each PATH entry
and shim_dir before comparison, resolving relative paths, symlinks, and
platform-specific casing where applicable. Filter entries based on the
normalized equivalence while preserving unrelated PATH entries and the existing
join_paths error handling.
- Around line 125-127: Update the scope check in main’s dispatch flow to resolve
the boundary using the same AGENTFLARE_HOME_OVERRIDE-aware path-resolution logic
defined in paths.rs, rather than passing dirs::home_dir() directly to
in_scoped_project. Preserve the existing behavior when no override is configured
and ensure run_real is invoked when the current directory is outside the
resolved boundary.
🪄 Autofix (Beta)
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 Plus
Run ID: 3307faec-8549-4cee-8dd9-a84a3ce5c418
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.tomlcrates/agentflare-shim/Cargo.tomlcrates/agentflare-shim/src/main.rs
Comment on lines
+74
to
+77
| fn path_without_shim_dir(shim_dir: &Path) -> Option<OsString> { | ||
| let path_var = env::var_os("PATH")?; | ||
| env::join_paths(env::split_paths(&path_var).filter(|p| p != shim_dir)).ok() | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Normalize PATH entries before filtering the shim directory.
Raw path equality retains equivalent entries such as . (when run from the shim directory), relative paths, symlinked paths, or Windows casing variants. which_in can then resolve this shim again and recursively spawn it until resources are exhausted.
Proposed fix
fn path_without_shim_dir(shim_dir: &Path) -> Option<OsString> {
+ let shim_dir = shim_dir.canonicalize().ok()?;
let path_var = env::var_os("PATH")?;
- env::join_paths(env::split_paths(&path_var).filter(|p| p != shim_dir)).ok()
+ env::join_paths(env::split_paths(&path_var).filter(|entry| {
+ let resolved = if entry.as_os_str().is_empty() {
+ env::current_dir()
+ } else {
+ entry.canonicalize()
+ };
+ resolved.as_ref().map_or(true, |path| path != &shim_dir)
+ }))
+ .ok()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn path_without_shim_dir(shim_dir: &Path) -> Option<OsString> { | |
| let path_var = env::var_os("PATH")?; | |
| env::join_paths(env::split_paths(&path_var).filter(|p| p != shim_dir)).ok() | |
| } | |
| fn path_without_shim_dir(shim_dir: &Path) -> Option<OsString> { | |
| let shim_dir = shim_dir.canonicalize().ok()?; | |
| let path_var = env::var_os("PATH")?; | |
| env::join_paths(env::split_paths(&path_var).filter(|entry| { | |
| let resolved = if entry.as_os_str().is_empty() { | |
| env::current_dir() | |
| } else { | |
| entry.canonicalize() | |
| }; | |
| resolved.as_ref().map_or(true, |path| path != &shim_dir) | |
| })) | |
| .ok() | |
| } |
🤖 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/agentflare-shim/src/main.rs` around lines 74 - 77, Update
path_without_shim_dir to normalize each PATH entry and shim_dir before
comparison, resolving relative paths, symlinks, and platform-specific casing
where applicable. Filter entries based on the normalized equivalence while
preserving unrelated PATH entries and the existing join_paths error handling.
Comment on lines
+125
to
+127
| let cwd = env::current_dir().unwrap_or_default(); | ||
| if !in_scoped_project(&cwd, dirs::home_dir().as_deref()) { | ||
| run_real(&tool, filtered_path.as_ref(), &args); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Honor AGENTFLARE_HOME_OVERRIDE for the scope boundary.
This bypasses the project-wide home resolution contract in src/paths.rs. When the override is set, traversal can inspect a marker at or above the configured boundary and enable dispatch where it should stop.
Proposed fix
let cwd = env::current_dir().unwrap_or_default();
- if !in_scoped_project(&cwd, dirs::home_dir().as_deref()) {
+ let home = env::var_os("AGENTFLARE_HOME_OVERRIDE")
+ .map(PathBuf::from)
+ .or_else(dirs::home_dir);
+ if !in_scoped_project(&cwd, home.as_deref()) {
run_real(&tool, filtered_path.as_ref(), &args);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let cwd = env::current_dir().unwrap_or_default(); | |
| if !in_scoped_project(&cwd, dirs::home_dir().as_deref()) { | |
| run_real(&tool, filtered_path.as_ref(), &args); | |
| let cwd = env::current_dir().unwrap_or_default(); | |
| let home = env::var_os("AGENTFLARE_HOME_OVERRIDE") | |
| .map(PathBuf::from) | |
| .or_else(dirs::home_dir); | |
| if !in_scoped_project(&cwd, home.as_deref()) { | |
| run_real(&tool, filtered_path.as_ref(), &args); |
🤖 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/agentflare-shim/src/main.rs` around lines 125 - 127, Update the scope
check in main’s dispatch flow to resolve the boundary using the same
AGENTFLARE_HOME_OVERRIDE-aware path-resolution logic defined in paths.rs, rather
than passing dirs::home_dir() directly to in_scoped_project. Preserve the
existing behavior when no override is configured and ensure run_real is invoked
when the current directory is outside the resolved boundary.
Comment on lines
+136
to
+145
| match cmd.status() { | ||
| Ok(status) => { | ||
| let code = status.code().unwrap_or(1); | ||
| if code == 126 || code == 127 { | ||
| run_real(&tool, filtered_path.as_ref(), &args); | ||
| } | ||
| exit(code); | ||
| } | ||
| Err(_) => run_real(&tool, filtered_path.as_ref(), &args), | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not treat child exit codes 126/127 as a fallback protocol.
Once lean-ctx starts successfully, these codes may be returned by the wrapped tool itself. Re-running the real command can duplicate non-idempotent operations and masks its intended exit result. Use an explicit lean-ctx fallback signal; only process-spawn failure is unambiguous here.
🤖 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/agentflare-shim/src/main.rs` around lines 136 - 145, Remove the
status-code-based fallback from the command execution match in main:
specifically, do not call run_real based on child exit codes 126 or 127.
Preserve and use an explicit lean-ctx fallback signal if one exists, while
retaining run_real only for the Err(_) process-spawn failure path, and otherwise
exit with the child’s returned code.
Non-trivial branching logic (project-marker walk-up, stopping at home so ~/.agentflare's own dir never false-positives as a project marker -- a real bug already found once on the bash-function prototype of this same idea) shipped with zero test coverage.
Local rustfmt (1.9.0) and CI's disagree on brace-member ordering for this import -- no rust-toolchain.toml pins a version repo-wide. Matching what CI's cargo fmt --check actually requires since that's the real gate.
Owner
Author
|
Superseded by #279, which merged first and shipped a strict superset: the same gate/dispatch logic (kill switches, agent-env detection, project walk-up), refactored into a lib+bin split so can reuse the exec core. Diffing this branch against the now-merged master confirms it's fully redundant — merging as-is would delete and re-inline the same logic, a regression with no net-new functionality. Closes item #227 via #279 instead. The one thing this branch adds that master doesn't have yet is unit test coverage for 's home-boundary walk-up — worth porting as a small direct follow-up, not blocking. |
Owner
Author
|
(Correction — the previous close comment lost several words to shell quoting.) Superseded by #279, which merged first and shipped a strict superset: the same agentflare-shim gate/dispatch logic (kill switches, agent-env detection, .agentflare project walk-up), refactored into a lib+bin split so flare-git-shim can reuse the exec core. Diffing this branch against the now-merged master confirms it's fully redundant -- merging as-is would delete lib.rs and re-inline the same logic, a regression with no net-new functionality. Closes item #227 via #279 instead. The one thing this branch adds that master doesn't have yet is unit test coverage for in_scoped_project's home-boundary walk-up -- worth porting as a small direct follow-up, not blocking. |
This was referenced Jul 21, 2026
5 tasks done
5 tasks done
getappz
added a commit
that referenced
this pull request
Sep 17, 2026
…s-project dispatch retry loop) (#759) * fix(shim): own re-entry marker stops the lean-ctx <-> bash fork-bomb Neither agentflare dispatcher -- the `~/.bashenv` `_af_dispatch` function nor the compiled PATH shim -- had a re-entry guard of its own. Both only honored lean-ctx's user-facing opt-outs (LEAN_CTX_DISABLED/NO_HOOK), and lean-ctx deliberately clears its own LEAN_CTX_WRAPPED/LEAN_CTX_ACTIVE markers on its delegate-to-shell path, so a shell that lean-ctx spawns could re-enter a dispatcher unmarked, spawn a fresh unwrapped `lean-ctx -c`, which spawned another shell, without bound. Observed live on 2026-09-16 as ~8,000 orphaned bash.exe/lean-ctx.exe processes and system memory exhaustion (image-qc vents #274 and #278), and reproduced on 2026-09-17. Both dispatchers now stamp AGENTFLARE_SHIM_ACTIVE=1 on the lean-ctx child they spawn and treat it (plus LEAN_CTX_WRAPPED, one hop earlier) as a kill switch: anything inside a command tree an agentflare dispatcher already handed to lean-ctx execs the real tool. lean-ctx never touches that variable, so it survives every lean-ctx spawn path and every shell lean-ctx starts. The regression test drives the generated .bashenv block with a fake lean-ctx that re-runs its command in a fresh bash under the same BASH_ENV -- the exact pathological lap -- and gives up after five laps so a regression fails instead of hanging. Against the previously installed block that harness measures six laps (exit 99); with this change, one. Agentflare-Agent: claude-code_2-1-274_agent Agentflare-Branch: fix/process-storms Agentflare-Session: db2a8bd7-2d5e-425b-935e-bf58bc998a87 * fix(supervisor): record dispatch bookkeeping by canonical id so the failure ceiling can trip for every project The discovery tick walks every registered `project_dirs` folder, but its `AgentflareMcp` is linked to only the project this daemon process was started in. `dispatch_item`, `skip_item` and `ask_item` did their label swap and comment through the `ItemRequest`/`CommentRequest` MCP entry points, which resolve ids via `resolve_item_id` and reject any item outside that one linked project -- and every call was `let _ =`. The daemon started by the watchdog scheduled task runs from C:\Windows\System32, which matches no real project at all, so since 2026-08-31 not one `## supervisor — dispatched` marker landed on any item in any project. `dispatch_failure_ceiling` counts dispatch cycles by exactly that marker, so it counted zero, `handle_terminal_job_failure` put `ready-for-work` straight back after every terminal failure, and a deterministically failing item was re-dispatched every SUPERVISOR_DISCOVERY_INTERVAL without bound: image-qc item #273 ran 1,367 identical failing jobs, one every 12 seconds, across 31 hours (2026-09-16/17). A skipped item likewise never lost `ready-for-work` and was re-skipped, with a fresh comment, every tick. Do the swap and the comment directly against the backend by the item's own canonical ids (the same way `handle_terminal_job_failure` already does), and log a failure instead of swallowing it -- a swap that silently doesn't land is precisely what re-arms the loop (item #221). The regression test seeds a ready item in a second project reachable only through `project_dirs` and asserts the tick strips `ready-for-work`, adds `dispatched`, and posts the marker; against the previous code it fails on the first assertion. The trigger for #273's failures (a stale worktree registration that `remove_stale_registration_for` only clears when its HEAD names the same branch, while git refuses by path) is tracked separately as item #588. Agentflare-Agent: claude-code_2-1-274_agent Agentflare-Branch: fix/process-storms Agentflare-Session: db2a8bd7-2d5e-425b-935e-bf58bc998a87 * fix(supervisor): address CodeRabbit review findings on record_supervisor_action/dispatch_item Two of the three findings from PR #759's automated review: 1. record_supervisor_action's three backend writes (remove_label, add_label, comment::create) ran as separate auto-committed statements, not a transaction -- with_backend_db only locks and opens the connection, it does not start one. A failure partway through (e.g. add_label after remove_label already committed) left the exact half-applied state this PR's other fix exists to prevent: an item with neither label and no dispatch marker, invisible to both the discovery query and the failure ceiling. Wrapped in conn.unchecked_transaction(), the pattern this codebase already uses elsewhere (label.rs, project.rs, item/claim.rs, item/crud.rs). 2. dispatch_item resolved DISPATCHED_LABEL only when swapping labels, after the job was already enqueued. run_discovery_tick only requires READY_LABEL to exist before batching a project, so a project that never got DISPATCHED_LABEL seeded would enqueue a real job, remove ready-for-work, and leave the item wearing neither label. Resolved before enqueue_work_job now; returns NotDispatched (no job, no label change) when the label doesn't exist, so a human fixing the missing label sees the item again on the very next tick instead of a silently orphaned item. The third finding (a rarer failure mode: the enqueue and the bookkeeping are two different SQLite databases with no shared transaction, so the bookkeeping write can still fail outright after the job is already enqueued) is real but tagged "heavy lift" by the review itself -- tracked separately as agentflare item #591 rather than folded into this fix. Regression tests for both: dispatch_item_declines_when_the_project_has_no_dispatched_label asserts no job is enqueued and ready-for-work stays on; the existing cross-project test continues to cover the transactional write path. Agentflare-Agent: claude-code_2-1-274_agent Agentflare-Branch: fix/process-storms Agentflare-Session: db2a8bd7-2d5e-425b-935e-bf58bc998a87
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
crates/agentflare-shim: a compiled PATH shim (same pattern as mise'smise-shim) — one binary, hardlinked/copied under many tool names into~/.agentflare/shims/, prepended to PATH.lean-ctx -c <tool>or execs the real binary untouched.-NoProfile, which skips shell-profile-based hooks entirely; PATH resolution still hits a shim dir regardless..agentflareproject walk-up (stopping at$HOMEto avoid false-positiving on the whole home directory).Closes item #227 (Windows compiled-exe shim for lean-ctx).
Note for reviewers
There is a second, more-evolved copy of this same crate on #279 (
feat/flare-git-core-shim) — that branch refactoredagentflare-shiminto a lib+bin split (exposingpath_without_shim_dir/run_real/tool_name_from_exeas a reusable exec core) soflare-git-shimcould build a git-specific policy layer on top of it. The twocrates/agentflare-shimimplementations will conflict on merge; whichever lands second should rebase onto the other rather than reconciling two independent versions. Recommend merging #279 first (its lib+bin split is a strict superset) and rebasing this one on top, dropping this PR's ownagentflare-shimin favor of that one.Test plan
cargo check -p agentflare-shim— clean-NoProfile) and a Unix shellLEAN_CTX_DISABLED,LEAN_CTX_NO_HOOK) fall through to the real binarySummary by CodeRabbit
agentflare-shimPATH shim that detects the requested tool from its executable name..agentflaremarker, the shim routes execution throughlean-ctx.lean-ctxisn’t usable, the shim falls back to running the original tool directly.AGENTFLARE_SHIM_TRACE..agentflarescope detection and$HOMEboundary behavior.