fix(tui): let a resolved fleet member slot win over the legacy role label

[gaord]

Summary

Fix effective_fleet_role_with_source: it only consulted the resolved agent_profile when worker.agent_profile was empty, so a task whose role label is "manager" but whose agent_profile selects member:reviewer fell through to the legacy worker.role label and ran with the wrong (write-capable) authority. Prefer the resolved member slot; keep the legacy label only as a fallback.

Testing

• cargo fmt --all -- --check
• cargo test -p codewhale-tui --lib agent_profile_member_slot_overrides_legacy_role_label

Note: main currently has 5 pre-existing nonminimal_bool clippy errors, unrelated to this change.

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[Hmbown]

Reviewed at e6868d1df. The bug you describe is real and I reproduced it. I am requesting changes only because the fix flips the precedence unconditionally, and the mirror case silently widens authority in the other direction — which is the same harm your own test comment names.

What I verified

Detached worktree, RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --lib -- fleet::worker_runtime:

• On the PR head: 70 passed; 0 failed.
• Your test against the pre-fix effective_fleet_role_with_source (I spliced agent_profile_member_slot_overrides_legacy_role_label onto the 505fc7a27^ version of the file): FAILS, left: Some("manager"), right: Some("reviewer"). So member:reviewer really did fall through to the "manager" label. Confirmed.

I also traced why it is reachable: freeze_fleet_task_members (worker_runtime.rs:217-221) deliberately keeps a divergent worker.role when an explicit worker.agent_profile selector is present — it only canonicalizes the string:

worker.agent_profile = Some(format!("member:{}", profile.id));
if explicit_selector.is_none() {
    worker.role = Some(snapshot.role.clone());
} else if let Some(role) = worker.role.as_mut() {
    *role = canonical_public_role_name(role.trim());   // divergence preserved
}

So a task can carry agent_profile: member:X and role: Y past freezing with X.role != Y, and nothing rejects it. Someone has to lose. Today the label wins; after this PR the member wins.

The concern: the mirror case widens write authority

I added a throwaway probe test (member alice whose slot is implement; task worker.role = "reviewer") and ran it against both trees. Same input, fleet_task_to_worker_spec_with_profiles:

	spec.role	spec.agent_type	runtime_profile.permissions.write
origin/main	reviewer	Reviewer	false
this PR	implement	Builder	true

That is a read-only task quietly becoming write-capable. It is the exact failure your test guards against —

"reviewer authority must not be silently widened to a write-capable worker"

— just approached from the other side. Net, the change trades one silent widening for another rather than closing the class.

(For completeness: manager is not a canonical FleetRole, so it falls to FleetRole::Worker via fleet_role_to_agent_type — write-capable. Your case is genuinely a widening and genuinely worth fixing. I am not disputing the direction, only the unconditional rule.)

What I would like instead

Any one of these closes the class rather than rotating it:

1. Fail closed at freeze time (my preference). In freeze_fleet_task_members, when explicit_selector.is_some() and worker.role names a different posture than profile.role.name, bail! the way the surrounding code already bails for unknown selectors and ambiguous members (worker_runtime.rs:168-199). A conflicting spec is an authoring error; resolving it silently either way is how you get a worker running with authority nobody wrote down. This also keeps effective_fleet_role_with_source honest — by the time it runs, there is no conflict left to arbitrate.
2. Narrower wins. If you want conflicts to stay legal, resolve to the less authoritative posture rather than to a fixed side. That satisfies your case (reviewer < manager) and mine (reviewer < implement) with one rule.
3. If the team's settled answer really is "the member slot is always authoritative", then say so where it is enforced and make it visible: the freeze path should overwrite worker.role with snapshot.role in the explicit-selector branch too, instead of preserving a label that is now dead. Right now the code carefully preserves a field that the new precedence guarantees will never be read — that is a trap for the next reader.

Whichever you pick, please add the inverse-direction regression test alongside agent_profile_member_slot_overrides_legacy_role_label. The current test only pins the widening you fixed, so nothing stops the symmetric one from being reintroduced.

Smaller notes

• The role_source value stays "agent_profile.role" in the new branch, which is right, but with the fallthrough gone "task.role" is now only reachable when no member resolved at all. Worth saying that in the comment — it is a meaningful narrowing of what that receipt string means, and receipts are read by people debugging exactly this.
• DCO: 505fc7a27 has no Signed-off-by: trailer (the merge commit e6868d1df does not either). Check Signed-off-by is advisory in .github/workflows/dco.yml, so CI is green, but CONTRIBUTING asks for it — git commit --amend -s.
• No locale concern here; nothing user-visible was added.

Thanks for chasing this one down to a concrete fleet-e12f3160 repro — the diagnosis is right and the comment you wrote explaining the old condition is genuinely better than what it replaced.