fix(subagent): stop counting finished children as shared-checkout contenders
A builder sub-agent could not run `echo x > file` in the workspace. Every `Bash` write came back with "cannot prove a bounded file target for this shared-workspace write claim", and the advice — use worktree isolation — puts the work in a sibling checkout the operator never looks at. Writing the same path through `File` was allowed the whole time, so the gate was not protecting anything the child could not already do. The gate asked whether *this* agent holds a shared write claim. The risk it exists for is a *peer* overwriting the same paths, and claims outlive the agents that register them: a test workspace with six `Completed` agents still held four standing claims, three of them non-isolated. So a lone builder was refused on account of children that had finished long ago, and a workspace got more restrictive the more it was used. `has_peer_shared_write_claim` now asks the real question: is another child, still `Running`, writing in this shared checkout. Worktree-isolated peers are excluded because they cannot contend for these paths, and an owner missing from the agent map stays contended — a claim that predates this session should fail closed. Concurrent writers are unaffected: `child_write_tool_fails_closed_outside_registered_scope` still passes unchanged, because it registers a live peer. A new test, `lone_shared_writer_keeps_unbounded_shell`, pins the case that was broken. Verified live against the release binary, in a workspace carrying those four stale claims: the builder ran `echo shell_fix2_ok > shell_fix2.txt` via `Bash`, exit 0, and the file landed in the workspace root — not in a worktree. Assisted by Claude Code.
This commit is contained in:
@@ -431,6 +431,16 @@ File edits, terminal width, and Windows installation.
|
||||
- Large pasted input is no longer sent to the model twice as inline text and
|
||||
as a backup `.md` paste file; the submitted message now carries only the
|
||||
`@`-mention so the model reads the file once.
|
||||
- A builder sub-agent can run ordinary shell writes again. Write claims
|
||||
outlive the agents that register them, so a workspace accumulated one per
|
||||
builder that ever ran — six completed agents left four standing claims in
|
||||
testing — and the shared-checkout gate counted those long-finished children
|
||||
as live contenders. Every later builder was refused `Bash` writes with
|
||||
"cannot prove a bounded file target" and pushed toward worktree isolation,
|
||||
which puts the work in a checkout the operator never looks at. The gate now
|
||||
asks the question it meant to ask: is another *running* child writing in this
|
||||
shared checkout. Concurrent writers are still gated; a lone builder writes in
|
||||
the workspace you are actually watching.
|
||||
- Ctrl-C during the first moments of startup no longer kills Codewhale
|
||||
outright. The terminating-signal handlers were registered inside the task
|
||||
that waits on them, and a spawned task does not run until the scheduler
|
||||
|
||||
@@ -431,6 +431,16 @@ File edits, terminal width, and Windows installation.
|
||||
- Large pasted input is no longer sent to the model twice as inline text and
|
||||
as a backup `.md` paste file; the submitted message now carries only the
|
||||
`@`-mention so the model reads the file once.
|
||||
- A builder sub-agent can run ordinary shell writes again. Write claims
|
||||
outlive the agents that register them, so a workspace accumulated one per
|
||||
builder that ever ran — six completed agents left four standing claims in
|
||||
testing — and the shared-checkout gate counted those long-finished children
|
||||
as live contenders. Every later builder was refused `Bash` writes with
|
||||
"cannot prove a bounded file target" and pushed toward worktree isolation,
|
||||
which puts the work in a checkout the operator never looks at. The gate now
|
||||
asks the question it meant to ask: is another *running* child writing in this
|
||||
shared checkout. Concurrent writers are still gated; a lone builder writes in
|
||||
the workspace you are actually watching.
|
||||
- Ctrl-C during the first moments of startup no longer kills Codewhale
|
||||
outright. The terminating-signal handlers were registered inside the task
|
||||
that waits on them, and a spawned task does not run until the scheduler
|
||||
|
||||
@@ -3457,6 +3457,38 @@ impl SubAgentManager {
|
||||
.find(|record| record.claim.owner == owner && !record.isolated_worktree)
|
||||
}
|
||||
|
||||
/// Is another *live* child writing in the shared checkout?
|
||||
///
|
||||
/// This is the question the unbounded-write gate actually needs. A claim
|
||||
/// bounds a child so concurrent children cannot overwrite each other's
|
||||
/// files; with no second writer in the shared tree there is nothing to
|
||||
/// collide with, and a shell redirect is no more dangerous than the `File`
|
||||
/// write the same child is already allowed to perform.
|
||||
///
|
||||
/// Liveness is the load-bearing part. Claims outlive the agents that
|
||||
/// registered them, so a workspace accumulates one per builder that ever
|
||||
/// ran: six completed agents left four standing claims in testing. Counting
|
||||
/// those made every later builder look contended by children that had long
|
||||
/// since finished, and the contention only ever grew. A terminal owner
|
||||
/// cannot write anything, so its claim cannot contend.
|
||||
///
|
||||
/// Worktree-isolated peers are excluded too: they write into their own
|
||||
/// checkout and can never contend for these paths.
|
||||
fn has_peer_shared_write_claim(&self, owner: &str) -> bool {
|
||||
self.coordination
|
||||
.write_claims
|
||||
.iter()
|
||||
.filter(|record| record.claim.owner != owner && !record.isolated_worktree)
|
||||
.any(|record| {
|
||||
// Unknown owners stay contended: a claim whose agent is not in
|
||||
// this map may predate the current session, and failing closed
|
||||
// is the safe direction for a write gate.
|
||||
self.agents
|
||||
.get(&record.claim.owner)
|
||||
.is_none_or(|agent| matches!(agent.status, SubAgentStatus::Running))
|
||||
})
|
||||
}
|
||||
|
||||
/// Classify an agent by its `session_boot_id`: `true` when the
|
||||
/// agent was either (a) loaded from disk with no id, or (b) carries
|
||||
/// a different id than the manager's current boot. Filters
|
||||
@@ -13133,9 +13165,17 @@ impl SubAgentToolRegistry {
|
||||
}))
|
||||
{
|
||||
let manager = self.coordination_manager.read().await;
|
||||
if manager.shared_write_claim(&self.owner_agent_id).is_some() {
|
||||
// Only a *contended* shared checkout needs this gate. The claim
|
||||
// exists so concurrent children cannot overwrite each other; a lone
|
||||
// writer has no peer to collide with, and blocking it there bought
|
||||
// no safety while making a builder unable to run ordinary shell
|
||||
// work in the workspace the operator actually watches — worktree
|
||||
// isolation "fixes" that by writing somewhere they never see.
|
||||
if manager.shared_write_claim(&self.owner_agent_id).is_some()
|
||||
&& manager.has_peer_shared_write_claim(&self.owner_agent_id)
|
||||
{
|
||||
return Err(anyhow!(
|
||||
"Tool {name} cannot prove a bounded file target for this shared-workspace write claim. Use scope-aware file tools, or launch the child with worktree isolation."
|
||||
"Tool {name} cannot prove a bounded file target, and another child is writing in this shared checkout. Use scope-aware file tools, or launch the children with worktree isolation."
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9420,6 +9420,69 @@ async fn child_write_tool_fails_closed_outside_registered_scope() {
|
||||
assert!(!tmp.path().join("docs/run-escape.txt").exists());
|
||||
}
|
||||
|
||||
/// A lone writer in the shared checkout keeps its shell.
|
||||
///
|
||||
/// The gate above exists so concurrent children cannot overwrite each other.
|
||||
/// With no second writer there is nothing to collide with, and refusing the
|
||||
/// shell there bought no safety: the same child may already write these paths
|
||||
/// through `File`. It only pushed builders toward worktree isolation, which
|
||||
/// puts their work in a checkout the operator never looks at.
|
||||
#[tokio::test]
|
||||
async fn lone_shared_writer_keeps_unbounded_shell() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let manager = new_shared_subagent_manager(tmp.path().to_path_buf(), 4);
|
||||
{
|
||||
let mut guard = manager.write().await;
|
||||
guard
|
||||
.coordination
|
||||
.register_claim(
|
||||
WriteScopeClaim {
|
||||
owner: "agent_solo".into(),
|
||||
roots: vec!["src".into()],
|
||||
exact_files: vec![],
|
||||
contracts: vec![],
|
||||
},
|
||||
false,
|
||||
|_| false,
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
let mut runtime = stub_runtime();
|
||||
runtime.manager = Arc::clone(&manager);
|
||||
runtime.context = ToolContext::new(tmp.path());
|
||||
runtime.context.auto_approve = true;
|
||||
runtime.worker_profile = WorkerRuntimeProfile::for_role(FleetRole::Builder);
|
||||
let registry = SubAgentToolRegistry::new_with_owner(
|
||||
runtime,
|
||||
FleetRole::Builder,
|
||||
"agent_solo".into(),
|
||||
"implementer".into(),
|
||||
Some(vec![
|
||||
"File".into(),
|
||||
"Bash".into(),
|
||||
"Run".into(),
|
||||
"agents/coordinate".into(),
|
||||
]),
|
||||
Arc::new(Mutex::new(TodoList::new())),
|
||||
Arc::new(Mutex::new(PlanState::default())),
|
||||
);
|
||||
|
||||
let result = registry
|
||||
.execute(
|
||||
"agent_solo",
|
||||
"Bash",
|
||||
json!({"action": "run", "command": "echo solo > solo.txt"}),
|
||||
)
|
||||
.await;
|
||||
if let Err(err) = &result {
|
||||
assert!(
|
||||
!err.to_string()
|
||||
.contains("cannot prove a bounded file target"),
|
||||
"a lone shared writer must not hit the contention gate: {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shared_claim_shell_gate_normalizes_only_the_run_action() {
|
||||
assert!(is_unbounded_shell_run(
|
||||
|
||||
@@ -191,11 +191,12 @@
|
||||
"_todo_2026_08_06_unreleased": "Post-0.9.4 [Unreleased] work: aggregate 673375 -> 676325 and max module 17596 -> 17631. The bulk is Agent Plugins v1.0.0 consume/publish/slugify (a new plugins/agent_plugin.rs plus manifest/registry/export wiring, ~2.3k lines with its tests). The rest is the sub-agent billing work \u2014 the agent_spawned stream event that makes a child's model visible, the RLM block-intent scanner, memory revise/retire with journals, and the harness audit trail. main.rs takes the module bump from the stream event. No new packages or binaries. The standing note still applies: this is a ledger, not a new normal \u2014 pay it down by deletion and dedup rather than treating growth as routine.",
|
||||
"_todo_2026_08_07_ship": "v0.9.4 final ship re-baseline: aggregate 676325 -> 676604 (+279 lines) for the todo_write sole progress surface, the fleet named-role model-pin contract + its tests, mid-stream network resume/paste dedup, and the release-prep runtime-contract budget rebaseline. No new 1000-line modules; growth is in existing files. Pay down in 0.9.5.",
|
||||
"_todo_2026_08_07_signal_arming": "v0.9.4 release-gate repair: aggregate 676604 -> 676670 and max module 17631 -> 17684 (+66 lines). 53 in main.rs: splitting terminating-signal registration from the await so the OS disposition changes before `spawn_signal_cleanup_task` returns — a `tokio::spawn`ed task does not run until first polled, so registering inside it let a Ctrl-C in that window kill the process outright. The line cost is the `TerminatingSignals` struct and its two cfg'd impls replacing one free async fn, plus the comment recording why registration cannot be lazy. The other 13 are the shared guard that stops the two global-fetch-cache tests from resetting the cache under each other. No new modules or packages. Pay down in 0.9.5.",
|
||||
"_todo_2026_08_07_builder_shell": "v0.9.4 builder shell repair: aggregate 676670 -> 676710 (+40 lines). The shared-checkout write gate now asks whether another *running* child is writing, instead of whether any claim exists — claims outlive their agents, so a workspace accumulated phantom contenders and every later builder lost Bash writes. Growth is the liveness-aware peer check plus a regression test that a lone shared writer keeps its shell. No new modules. Pay down in 0.9.5.",
|
||||
"document_kind": "codewhale.source_structure_budget",
|
||||
"large_module_threshold_lines": 1000,
|
||||
"max_large_module_count": 177,
|
||||
"max_module_lines": 17684,
|
||||
"max_total_owned_rust_lines": 676670,
|
||||
"max_total_owned_rust_lines": 676710,
|
||||
"schema_version": 1,
|
||||
"workspace_packages": [
|
||||
"codewhale-agent",
|
||||
|
||||
Reference in New Issue
Block a user