Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions src/harness/built_in/planning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ pub async fn run_planning_pass(runtime: Arc<CompanyRuntime>, task_id: String) {

let prerequisites = verify_prerequisites(&runtime, &evidence, &draft.prerequisites).await;
let candidates = resolve_assignee_candidates(&evidence, &draft.assignee_candidates);
let candidates = prefer_company_over_baseline(&evidence, candidates);
// Issue #1106. One surviving candidate is a proposal and behaves exactly as
// it did before this change. Two or more is an open question, and a question
// is not something to answer by taking the first element — so nothing is
Expand Down Expand Up @@ -692,6 +693,10 @@ struct TeammateBrief {
description: Option<String>,
/// Effective tool grants — namespace names only, never a credential.
grants: Vec<String>,
/// Whether this teammate came from the global baseline rather than the
/// company's own roster (mirrors [`crate::company::types::Agent::global`]).
/// An overlay teammate is never global — it always has an author.
global: bool,
}

/// Everything the host gathered before the model was asked anything.
Expand Down Expand Up @@ -839,6 +844,7 @@ async fn gather_evidence(
role: a.role.clone(),
description: a.description.clone(),
grants: crate::runtime::builder::agent_effective_grants(&allow, &a.tools),
global: a.global,
})
.collect();
teammates.extend(
Expand All @@ -851,6 +857,7 @@ async fn gather_evidence(
role: overlay.role.clone(),
description: overlay.description.clone(),
grants: crate::runtime::builder::agent_effective_grants(&allow, &overlay.tools),
global: false,
}),
);

Expand Down Expand Up @@ -1346,6 +1353,9 @@ fn evidence_prompt(e: &Evidence) -> String {
if let Some(description) = &t.description {
out.push_str(&format!(" — {description}"));
}
if t.global {
out.push_str(" — from the shared baseline");
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
out.push('\n');
}
for (desk, members) in &e.desks {
Expand Down Expand Up @@ -1862,6 +1872,47 @@ fn resolve_assignee_candidates(
out
}

/// Issue #1196. Drops baseline candidates from a tie that also names a
/// company-authored teammate.
///
/// `resolve_assignee_candidates` only validates names — it stays that way.
/// This runs as a separate pass over its output because the tie it resolves
/// is not about which name is real, it is about provenance: every company
/// carries the same four baseline teammates ([`crate::globals`]), and when one
/// of them ties against a role the company chose to staff itself, the company
/// has already expressed the answer by staffing that role. A tie between two
/// baseline teammates, or between two company teammates, carries no such
/// signal and is left untouched — issue #1106's park-and-ask stands for both.
///
/// A candidate id that does not resolve to a manifest agent (a desk, or an
/// overlay teammate — [`OverlayAgent`](crate::ports::types::OverlayAgent) has
/// no `global` field, so it can never be one) counts as company-side: only a
/// manifest agent explicitly marked `global` is baseline.
fn prefer_company_over_baseline(
evidence: &Evidence,
candidates: Vec<AssigneeCandidate>,
) -> Vec<AssigneeCandidate> {
let is_baseline = |id: &str| {
evidence
.record
.manifest
.agents
.iter()
.find(|a| a.id == id)
.is_some_and(|a| a.global)
};
let has_company_side = candidates.iter().any(|c| !is_baseline(&c.id));
let has_baseline = candidates.iter().any(|c| is_baseline(&c.id));
if has_company_side && has_baseline {
candidates
.into_iter()
.filter(|c| !is_baseline(&c.id))
.collect()
} else {
candidates
}
}

/// The note line a card parks with when the pass declined to choose.
///
/// Rendered in the same shape as the blocked-on-prerequisites reason — a
Expand Down
155 changes: 155 additions & 0 deletions src/harness/built_in/planning/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ fn evidence() -> Evidence {
role: a.role.clone(),
description: a.description.clone(),
grants: crate::runtime::builder::agent_effective_grants(&allow, &a.tools),
global: a.global,
})
.collect();
Evidence {
Expand Down Expand Up @@ -1723,6 +1724,160 @@ async fn a_manifest_teammate_and_a_runtime_one_can_be_the_ambiguous_pair() {
);
}

/// Issue #1196. A tie between a company-authored teammate and a baseline one
/// is not the tie #1106 exists for: the company already expressed a
/// preference by staffing its own `Writer` (`maya`), so the baseline `writer`
/// (`globals/agents/writer.toml`, merged into every company's roster) steps
/// aside and the card dispatches instead of parking. Mirrors issue #1196's own
/// worked example — a company `Writer` tying against the global `writer`.
#[tokio::test]
async fn a_company_teammate_beats_a_baseline_tie_and_dispatches_without_parking() {
let reply = r#"{"description":"do it","steps":[],"prerequisites":[],"risks":[],
"verification":"v","scope":"s","assigneeCandidates":[
{"id":"maya","reason":"the company's own writer"},
{"id":"writer","reason":"the shared baseline writer"}]}"#;
let (_home, runtime) = runtime_with(ScriptedModel::replying(reply)).await;
runtime
.tasks()
.upsert(runtime.id(), &card("t-29", ""))
.await
.unwrap();

run_planning_pass(Arc::clone(&runtime), "t-29".to_string()).await;

let after = read(&runtime, "t-29").await;
assert_eq!(
after.column, COLUMN_IN_PROGRESS,
"the company's own pick dispatches rather than parking"
);
assert_eq!(after.assignee, "maya");
let plan = after.plan.expect("the brief is still written");
assert_eq!(
plan.proposed_assignee.as_deref(),
Some("maya"),
"the baseline candidate is dropped, leaving one proposal"
);
assert!(
plan.assignee_candidates.is_empty(),
"with one candidate left there is no ownership question to persist"
);
}

/// The baseline exists for a company that never staffed a role itself — so a
/// tie between two baseline teammates carries no company preference and must
/// keep parking exactly like #1106's original case.
#[tokio::test]
async fn two_baseline_teammates_still_park_with_both() {
let reply = r#"{"description":"do it","steps":[],"prerequisites":[],"risks":[],
"verification":"v","scope":"s","assigneeCandidates":[
{"id":"writer","reason":"could turn this into copy"},
{"id":"researcher","reason":"could dig up the source material first"}]}"#;
let (_home, runtime) = runtime_with(ScriptedModel::replying(reply)).await;
runtime
.tasks()
.upsert(runtime.id(), &card("t-30", ""))
.await
.unwrap();

run_planning_pass(Arc::clone(&runtime), "t-30".to_string()).await;

let after = read(&runtime, "t-30").await;
assert_eq!(
after.column, COLUMN_TODO,
"neither baseline teammate outranks the other"
);
assert_eq!(after.assignee, "");
assert_eq!(
after
.plan
.expect("plan")
.assignee_candidates
.iter()
.map(|c| c.id.as_str())
.collect::<Vec<_>>(),
vec!["writer", "researcher"],
"both survive, the same as any other unresolved tie"
);
}

/// Issue #1196. The prompt marks a baseline teammate as such, so the model
/// has the provenance evidence directly — even on a pass where the host-side
/// precedence never has to act on it, as here: one company teammate proposed,
/// no tie in play.
#[tokio::test]
async fn the_prompt_marks_a_baseline_teammate_from_the_shared_baseline() {
let model = ScriptedModel::replying(CLEAN_PLAN);
let (_home, runtime) = runtime_with(Arc::clone(&model)).await;
runtime
.tasks()
.upsert(runtime.id(), &card("t-31", ""))
.await
.unwrap();

run_planning_pass(Arc::clone(&runtime), "t-31".to_string()).await;

let prompt = model.last_prompt();
let writer_line = prompt
.lines()
.find(|l| l.contains("`writer`"))
.unwrap_or_else(|| panic!("the merged baseline puts `writer` on the roster:\n{prompt}"));
assert!(
writer_line.contains("— from the shared baseline"),
"{writer_line}"
);
let maya_line = prompt
.lines()
.find(|l| l.contains("`maya`"))
.unwrap_or_else(|| panic!("the company's own roster is still shown:\n{prompt}"));
assert!(
!maya_line.contains("— from the shared baseline"),
"a company-authored teammate is never mis-marked:\n{maya_line}"
);
}

/// Direct unit coverage of the precedence filter, independent of the planning
/// pass and any one scripted model.
#[test]
fn prefer_company_over_baseline_drops_only_a_true_mixed_tie() {
let mut evidence = evidence();
for agent in evidence.record.manifest.agents.iter_mut() {
if agent.id == "sam" {
agent.global = true;
}
}
let candidate = |id: &str| AssigneeCandidate {
id: id.to_string(),
reason: String::new(),
};

// A company teammate and a baseline one: the baseline is dropped.
let mixed = prefer_company_over_baseline(&evidence, vec![candidate("maya"), candidate("sam")]);
assert_eq!(
mixed.iter().map(|c| c.id.as_str()).collect::<Vec<_>>(),
vec!["maya"]
);

// A teammate and a desk: neither resolves to a baseline agent, so both
// count as company-side and the tie is untouched — #1106's case, and the
// reason an unresolved id (a desk) must default to company-side rather
// than silently misfiring as baseline.
let teammate_and_desk =
prefer_company_over_baseline(&evidence, vec![candidate("maya"), candidate("studio")]);
assert_eq!(
teammate_and_desk.len(),
2,
"no baseline teammate in the tie"
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

// A single baseline candidate, alone: nothing to prefer it over.
let solo_baseline = prefer_company_over_baseline(&evidence, vec![candidate("sam")]);
assert_eq!(
solo_baseline.len(),
1,
"a lone baseline candidate is not a tie"
);
}

/// Direct unit coverage of the resolver's caps and drops, so the rules hold
/// independently of what any one scripted model happens to emit.
#[test]
Expand Down
Loading