diff --git a/src/harness/built_in/planning.rs b/src/harness/built_in/planning.rs index bc4b7f9a6..e8640b5eb 100644 --- a/src/harness/built_in/planning.rs +++ b/src/harness/built_in/planning.rs @@ -352,6 +352,7 @@ pub async fn run_planning_pass(runtime: Arc, 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 @@ -692,6 +693,10 @@ struct TeammateBrief { description: Option, /// Effective tool grants — namespace names only, never a credential. grants: Vec, + /// 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. @@ -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( @@ -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, }), ); @@ -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"); + } out.push('\n'); } for (desk, members) in &e.desks { @@ -1862,6 +1872,66 @@ 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 resolves to exactly one of three provenances: a manifest +/// agent marked `global` is [`Baseline`](Provenance::Baseline); a manifest +/// agent that is not, or an overlay teammate — which +/// [`OverlayAgent`](crate::ports::types::OverlayAgent) can never be, having no +/// `global` field at all — is [`Company`](Provenance::Company); anything else +/// `resolve_assignee_candidates` could still have handed back (a desk) is +/// neither. A desk is not the company's own choice of *teammate*, so its mere +/// presence must not stand in for a real one: it neither triggers the drop nor +/// is dropped by it, on either side of the tie. +enum Provenance { + Baseline, + Company, +} + +fn provenance_of(evidence: &Evidence, id: &str) -> Option { + if let Some(agent) = evidence.record.manifest.agents.iter().find(|a| a.id == id) { + return Some(if agent.global { + Provenance::Baseline + } else { + Provenance::Company + }); + } + if evidence.record.overlay_agents.iter().any(|a| a.id == id) { + return Some(Provenance::Company); + } + None +} + +fn prefer_company_over_baseline( + evidence: &Evidence, + candidates: Vec, +) -> Vec { + let has_company = candidates + .iter() + .any(|c| matches!(provenance_of(evidence, &c.id), Some(Provenance::Company))); + let has_baseline = candidates + .iter() + .any(|c| matches!(provenance_of(evidence, &c.id), Some(Provenance::Baseline))); + if has_company && has_baseline { + candidates + .into_iter() + .filter(|c| !matches!(provenance_of(evidence, &c.id), Some(Provenance::Baseline))) + .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 diff --git a/src/harness/built_in/planning/test.rs b/src/harness/built_in/planning/test.rs index 2af49ed57..4b422d6ec 100644 --- a/src/harness/built_in/planning/test.rs +++ b/src/harness/built_in/planning/test.rs @@ -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 { @@ -1723,6 +1724,173 @@ 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!["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!["maya"] + ); + + // A teammate and a desk: no baseline teammate in the tie (the desk isn't + // one), so nothing is dropped — #1106's case. + 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" + ); + + // A baseline teammate and a desk: a desk is not the company's own choice + // of *teammate*, so its presence must not stand in for one and silently + // knock the real baseline candidate out of a tie nobody actually resolved + // in the company's favour. + let baseline_and_desk = + prefer_company_over_baseline(&evidence, vec![candidate("sam"), candidate("studio")]); + assert_eq!( + baseline_and_desk + .iter() + .map(|c| c.id.as_str()) + .collect::>(), + vec!["sam", "studio"], + "a desk is neutral: it neither triggers the drop nor gets dropped by it" + ); + + // 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]