Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions docs/changes/unreleased/1597-teams-manager-init.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
kind: fixed
title: manager conversation initialization, handle fallback, and approval posture inheritance
pr: 1597
surface: [chat, engine]
invalidates:
- "Sub-team manager conversations started via team_start previously ran under default approval postures rather than inheriting the initiating conversation's posture. They now inherit and persist the initiating posture."
- "Top-level team managers without an explicit title previously lacked a fallback handle. They now receive an assigned fallback handle upon creation so they can be messaged immediately."
- "Approval inheritance now uses the session-owned setter before team membership can wake work. A failed setter refuses startup; the UI no longer overwrites session metadata or borrows an unrelated foreground conversation’s posture."
---
8 changes: 8 additions & 0 deletions internal/manual/chat/team-manager.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,3 +389,11 @@ while it was closed.

The traffic itself is kept in the profile of the machine the conversations run on, in
`teams/<id>/traffic.jsonl`, one line per message, only ever added to.

## Which approval rules do new members inherit?

A member started by the manager inherits the manager's approval posture before
it joins and receives work. The conversation saves its own setting for resume.
If the new conversation cannot apply the posture, the start is refused and the
manager sees the reason. An untitled manager receives an available handle from
the team store, so it can be addressed immediately.
5 changes: 3 additions & 2 deletions internal/session/consent.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,13 +640,14 @@ func ConsentHead(tool, args string) string {
// consentRule is [Event.Rule] for a call: the policy's own words, and for a
// start with none, what the person is agreeing to pay for ([teamStartCost]).
func consentRule(call ai.ToolCall, decision approval.Decision) string {
if rule := strings.TrimSpace(decision.Rule); rule != "" {
rule := strings.TrimSpace(decision.Rule)
if rule != "" && rule != "default" && rule != "default (unset)" {
return rule
}
if call.Function.Name == teamStartToolName {
return teamStartCost
}
return ""
return rule
}

// consentReason is why the gate is asking, in the policy's own words where it
Expand Down
2 changes: 1 addition & 1 deletion internal/session/team_nest.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func (a *Agent) teamStartSubTeam(team teams.Team, role teamRole, handle, brief,
// here, where the membership was written. A refusal returned above and
// wrote nothing.
_ = teams.WriteMoveNotices(profile, movedIn)
start := teams.Entry{Kind: teams.KindStart, From: teams.FromManager, To: handle, Text: brief, Team: childID}
start := teams.Entry{Kind: teams.KindStart, From: teams.FromManager, To: handle, Text: brief, Team: childID, Approval: a.ResolvedApprovalPosture()}
if err := teams.AppendTraffic(profile, team.ID, start); err != nil {
return "The team " + strconv.Quote(name) + " was made, but its manager's start could not be written to the traffic: " + err.Error(), true
}
Expand Down
8 changes: 8 additions & 0 deletions internal/session/teamevent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,14 @@ func TestTheStartsCardSaysWhoIsStartedAndWhatItCosts(t *testing.T) {
if rule := consentRule(call, approval.Decision{Rule: "a rule said so"}); rule != "a rule said so" {
t.Errorf("a policy's own words were replaced: %q", rule)
}
for _, defRule := range []string{"default", "default (unset)"} {
if rule := consentRule(call, approval.Decision{Rule: defRule}); rule != teamStartCost {
t.Errorf("for rule %q, consentRule returned %q, want %q", defRule, rule, teamStartCost)
}
if reason := consentReason(call, approval.Decision{Rule: defRule}); reason != teamStartCost {
t.Errorf("for rule %q, consentReason returned %q, want %q", defRule, reason, teamStartCost)
}
}
bash := ai.ToolCall{Function: ai.ToolCallFunction{Name: "bash", Arguments: `{"command":"ls"}`}}
if head := ConsentHead("bash", bash.Function.Arguments); head != "needs your ok to run bash" {
t.Errorf("an ordinary head changed: %q", head)
Expand Down
2 changes: 1 addition & 1 deletion internal/session/tools_team.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ func (a *Agent) teamStartTool(ctx context.Context, args json.RawMessage) (string
if _, taken := team.ByHandle(handle); taken {
return fmt.Sprintf("@%s is already a member of %q. Pick another handle, or team_send it the work.", handle, team.Name), true, nil
}
entry := teams.Entry{Kind: teams.KindStart, From: teams.FromManager, To: handle, Text: brief}
entry := teams.Entry{Kind: teams.KindStart, From: teams.FromManager, To: handle, Text: brief, Approval: a.ResolvedApprovalPosture()}
if err := teams.AppendTraffic(a.config.teamProfile(), team.ID, entry); err != nil {
return "The start could not be written to the team's traffic: " + err.Error(), true, nil
}
Expand Down
16 changes: 16 additions & 0 deletions internal/teams/handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,19 @@ func uniqueHandle(t Team, key, base string) string {
}
}

// FallbackHandle returns a deterministic, valid handle for a member of t
// (such as an untitled manager) that is unique within t.
func FallbackHandle(t Team, key string) string {
base := ""
if !t.Root && t.Name != "" {
base = DeriveHandle(t.Name)
}
if base == "" || ValidHandle(base) != nil {
base = "lead"
}
return uniqueHandle(t, key, base)
}

// assignHandles clears every handle in t that is invalid or repeats one an
// earlier member has, then gives each member with a title and no handle one,
// in member order. It reports whether it changed anything.
Expand All @@ -247,6 +260,9 @@ func assignHandles(t *Team) bool {
continue
}
base := DeriveHandle(m.Word)
if base == "" && t.Manager != "" && t.Manager == m.Key {
base = FallbackHandle(*t, m.Key)
}
if base == "" {
continue
}
Expand Down
28 changes: 28 additions & 0 deletions internal/teams/handle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,31 @@ func TestChooseHandleClashesAndTypedHandles(t *testing.T) {
t.Errorf("an unusable answer changed the member: %+v", after)
}
}

func TestFallbackHandle(t *testing.T) {
team := Team{ID: "t1", Name: "harbor", Members: []Member{{Key: "m1", Word: ""}}}
fb := FallbackHandle(team, "m1")
if fb != "harbor" {
t.Errorf("got %q, want harbor", fb)
}
if err := ValidHandle(fb); err != nil {
t.Errorf("fallback handle %q is invalid: %v", fb, err)
}

// Collision resolution:
team.Members = append(team.Members, Member{Key: "m2", Handle: "harbor"})
fb2 := FallbackHandle(team, "m1")
if fb2 != "harbor2" {
t.Errorf("got %q, want harbor2", fb2)
}

// Root team falls back to lead
rootTeam := Team{ID: "root", Name: RootName, Root: true, Members: []Member{{Key: "gm"}}}
rfb := FallbackHandle(rootTeam, "gm")
if rfb != "lead" {
t.Errorf("got %q, want lead", rfb)
}
if err := ValidHandle(rfb); err != nil {
t.Errorf("root fallback %q is invalid: %v", rfb, err)
}
}
4 changes: 3 additions & 1 deletion internal/teams/teams.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,9 @@ func (f *File) SetManager(id, key string) error {
if err := f.AddMember(id, Member{Key: key}); err != nil {
return err
}
f.Teams[Index(f.Teams, id)].Manager = key
idx := Index(f.Teams, id)
f.Teams[idx].Manager = key
assignHandles(&f.Teams[idx])
return nil
}

Expand Down
3 changes: 3 additions & 0 deletions internal/teams/traffic.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ type Entry struct {
// brief, makes itself that team's manager. It is empty on every other
// start and every other kind.
Team string `json:"team,omitempty"`
// Approval is, on a [KindStart], the approval posture the started
// conversation inherits from the manager unless overridden.
Approval string `json:"approval,omitempty"`
}

// trafficRotateBytes is the size past which the log starts a new file.
Expand Down
33 changes: 32 additions & 1 deletion internal/tui3/teamtraffic.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tui3

import (
"fmt"
"strings"
"time"

Expand Down Expand Up @@ -465,9 +466,37 @@ func (a *app) trafficStart(t team, e teamstore.Entry) tea.Cmd {
a.trafficStartRefused(handle, newUnavailableWord)
return nil
}
posture := strings.TrimSpace(e.Approval)
if posture == "" {
// Old traffic has no captured posture. Only the initiating manager
// can supply it; an unrelated foreground conversation cannot.
var manager Agent
if t.Manager == a.frontTabKey() {
manager = a.agent
} else if held := a.behind[t.Manager]; held != nil {
manager = held.conv.Agent
}
if dial, ok := manager.(interface{ ResolvedApprovalPosture() string }); ok {
posture = dial.ResolvedApprovalPosture()
}
}
start, where, id := a.start, a.teamWhere(t), t.ID
return a.besideLine(func() func(bool) tea.Cmd {
conv, err := start(where)
// The session owns both the gate and its persisted posture. Apply it
// off the UI loop, before membership can wake the first turn.
if err == nil && conv.Agent != nil && posture != "" {
setter, ok := conv.Agent.(interface{ SetApprovalPosture(string) error })
if !ok {
err = fmt.Errorf("the new conversation cannot inherit approvals")
} else {
err = setter.SetApprovalPosture(posture)
}
if err != nil {
_ = conv.Agent.Close()
err = fmt.Errorf("inherit approvals: %w", err)
}
}
return func(bool) tea.Cmd { return a.trafficStarted(id, handle, conv, err) }
})
}
Expand Down Expand Up @@ -504,8 +533,10 @@ func (a *app) trafficStarted(id, handle string, conv Conversation, err error) te
if err := f.AddMember(t.ID, m); err != nil {
return err
}
if got, _ := f.Teams[teamIndex(f.Teams, t.ID)].Member(m.Key); got.Handle != handle && teamstore.ValidHandle(handle) == nil {
curTeam := f.Teams[teamIndex(f.Teams, t.ID)]
if got, _ := curTeam.Member(m.Key); got.Handle != handle && teamstore.ValidHandle(handle) == nil {
_ = f.SetHandle(t.ID, m.Key, handle)

}
return nil
}); err != nil {
Expand Down
71 changes: 71 additions & 0 deletions internal/tui3/teamtraffic_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package tui3

import (
"errors"
"fmt"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -515,3 +517,72 @@ func TestAReplayedBriefIsTheManagersCard(t *testing.T) {
t.Fatalf("the brief draws as:\n%s", rows)
}
}

type postureTestAgent struct {
fakeAgent
posture string
setErr error
}

func (p *postureTestAgent) SetApprovalPosture(posture string) error {
p.posture = posture
return p.setErr
}

func TestTrafficStartInheritsApprovalPosture(t *testing.T) {
a, harbor, _, _ := trafficApp(t)
var fresh *postureTestAgent
tmpDir := t.TempDir()
sessionFile := filepath.Join(tmpDir, "session.jsonl")
a.start = func(workspace string) (Conversation, error) {
fresh = &postureTestAgent{fakeAgent: fakeAgent{model: "m"}}
return Conversation{Agent: fresh, SessionFile: sessionFile, Workspace: workspace}, nil
}
trafficAppend(t, a, harbor, teamstore.Entry{Kind: teamstore.KindStart, From: teamstore.FromManager, To: "submgr", Text: "lead the sub team", Approval: session.PostureAllow})
trafficReadNow(t, a)
trafficReadNow(t, a)

if fresh == nil || fresh.posture != session.PostureAllow {
t.Fatalf("started agent did not inherit posture: got %+v", fresh)
}

}

func TestUntitledManagerGetsFallbackHandle(t *testing.T) {
a, harbor, _, _ := trafficApp(t)
tab := chatTab{key: "mgr-key-untitled", word: ""}
if err := a.teamMakeManager(harbor, tab); err != nil {
t.Fatalf("teamMakeManager failed: %v", err)
}
teamsFlush(t, a)
team := mustTeam(t, a, harbor)
m, ok := team.Member("mgr-key-untitled")
if !ok {
t.Fatalf("manager not in team: %+v", team.Members)
}
if m.Handle == "" {
t.Fatalf("manager has empty handle: %+v", m)
}
if err := teamstore.ValidHandle(m.Handle); err != nil {
t.Fatalf("fallback handle %q is invalid: %v", m.Handle, err)
}
}

// A child must not join the team and wake under a gate it failed to inherit.
func TestTrafficStartRefusesFailedApprovalInheritance(t *testing.T) {
a, harbor, _, _ := trafficApp(t)
fresh := &postureTestAgent{setErr: errors.New("cannot rebuild rules")}
file := filepath.Join(t.TempDir(), "session.jsonl")
a.start = func(workspace string) (Conversation, error) {
return Conversation{Agent: fresh, SessionFile: file, Workspace: workspace}, nil
}
trafficAppend(t, a, harbor, teamstore.Entry{Kind: teamstore.KindStart, From: teamstore.FromManager, To: "worker", Text: "start", Approval: session.PostureDeny})
trafficReadNow(t, a)
trafficReadNow(t, a)
if fresh.closes != 1 {
t.Fatalf("failed child closed %d times", fresh.closes)
}
if _, ok := mustTeam(t, a, harbor).ByHandle("worker"); ok {
t.Fatal("failed child joined team")
}
}
Loading