From 471d14782b17183ff0da1b4a08f7133ce0638f46 Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 11:44:13 -0700 Subject: [PATCH 01/10] fix(fleet): the watcher spawned a console window per session on Windows DETACHED_PROCESS on a console-subsystem binary gives the child its own console, which is a window on screen. Since any session revives a watcher it believes is stale, one flashes per SessionStart, and a machine running several sessions gets a stream of them. CREATE_NO_WINDOW is the flag that means "console application, no window". It is documented as invalid combined with DETACHED_PROCESS, so this replaces that flag rather than adding to it; CREATE_NEW_PROCESS_GROUP still keeps the watcher out of the parent's Ctrl-C group. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/fleet/internal/watch/detach_windows.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/cmd/fleet/internal/watch/detach_windows.go b/cmd/fleet/internal/watch/detach_windows.go index 30d6a217..b0a8e415 100644 --- a/cmd/fleet/internal/watch/detach_windows.go +++ b/cmd/fleet/internal/watch/detach_windows.go @@ -7,10 +7,18 @@ import ( "syscall" ) -// detach starts the watcher in a new process group with no console, so the hook's -// exit and the harness's console do not take it. +// detach starts the watcher in a new process group with no console window, so the +// hook's exit and the harness's console do not take it. +// +// CREATE_NO_WINDOW rather than DETACHED_PROCESS. Both keep the child off the parent's +// console, but `go build` produces a console-subsystem binary, and under +// DETACHED_PROCESS Windows gives such a child its own console - which is a window that +// pops on screen for every spawn. Since any session revives a dead watcher, a watcher +// that fails to stay up flashes a window per SessionStart. CREATE_NO_WINDOW is the flag +// that means "console application, no window"; it is documented as invalid combined +// with DETACHED_PROCESS, so this replaces it rather than adding to it. func detach(cmd *exec.Cmd) { const createNewProcessGroup = 0x00000200 - const detachedProcess = 0x00000008 - cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: createNewProcessGroup | detachedProcess} + const createNoWindow = 0x08000000 + cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: createNewProcessGroup | createNoWindow} } From a8f6fbc707d7bdd7e2ec4485fc915a24f04e5278 Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 11:44:26 -0700 Subject: [PATCH 02/10] fix(fleet): pool derived the repo label from the directory name, and assign hid the useful git error Two fixes in the seat path, both found standing a pool up on a machine that already had roles.map lines. pool took the label in a seat's role from filepath.Base(checkout). A directory basename is not a repo identity: a checkout at ~/dev/Mono produced :Mono beside an existing :mono, so `fleet work --for :mono` matched none of the new seats and the board showed two families for one repo. There was no way to ask for the other spelling either, since pool takes --tenant but no label. It now inherits the label from the checkout's own map line, exactly as poolTenant already inherits the tenant, and falls back to the basename only when there is no line. This also removes the reason a hand-fix would not stick. pool re-roles the seats it KEEPS, not only the ones it creates, so deriving the label per run overwrote a corrected value on every top-up. assign tries `checkout ` then `checkout -b origin/`, and reported the second failure. The fallback exists for a branch that is not local yet, so its message can only say the branch already exists, which is what the first attempt just established. The first attempt's message names the worktree holding the branch, which is the whole diagnostic, and this is the common case when a seat takes over an existing branch. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/fleet/internal/verbs/views.go | 57 +++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/cmd/fleet/internal/verbs/views.go b/cmd/fleet/internal/verbs/views.go index af0244ac..906fe2d6 100644 --- a/cmd/fleet/internal/verbs/views.go +++ b/cmd/fleet/internal/verbs/views.go @@ -583,7 +583,7 @@ func cmdPool(checkout, kind, nArg string, rewarm bool, tenant string) error { if err != nil { return err } - p := newPooler(checkout, base, parent, tenant, rewarm, cfg) + p := newPooler(checkout, base, parent, tenant, poolLabel(checkout, base), rewarm, cfg) for _, k := range order { for i := 1; i <= wanted[k]; i++ { if err := p.one(k, i); err != nil { @@ -595,8 +595,8 @@ func cmdPool(checkout, kind, nArg string, rewarm bool, tenant string) error { return nil } -func newPooler(checkout, base, parent, tenant string, rewarm bool, cfg map[string]any) *pooler { - p := &pooler{checkout: checkout, base: base, parent: parent, tenant: tenant, rewarm: rewarm, cmds: warmCommands(cfg), +func newPooler(checkout, base, parent, tenant, label string, rewarm bool, cfg map[string]any) *pooler { + p := &pooler{checkout: checkout, base: base, parent: parent, tenant: tenant, label: label, rewarm: rewarm, cmds: warmCommands(cfg), named: map[string]string{}, worktrees: registeredWorktrees(checkout)} _, rows := fleet.MapRows(fleet.RolesMap()) for _, r := range rows { @@ -680,15 +680,36 @@ func poolTenant(tenant, checkout, base string) (string, error) { return tenant, nil } +// poolLabel is the repo label a seat's role carries, the `mono` in `:mono`. +// +// Inherited from the checkout's own map line, the way poolTenant inherits the tenant, +// and derived from the directory name only when there is no line to inherit from. A +// directory basename is not a repo identity: a checkout at `~/dev/Mono` produced +// `:Mono` beside an existing `:mono`, which is two labels for one repo, so +// `fleet work --for :mono` matched none of the new seats. Because pool re-roles +// the seats it KEEPS and not only the ones it creates, deriving the label here also +// overwrote a corrected value on every top-up, leaving no durable fix outside the tool. +func poolLabel(checkout, base string) string { + role := fleet.RoleOf(checkout) + if _, label, ok := strings.Cut(role, ":"); ok && label != "" { + return label + } + return base +} + // pooler is one `fleet pool` run: what it knows before the loop, what it did. type pooler struct { checkout, base, parent, tenant string - rewarm bool - cmds []string - named map[string]string - worktrees map[string]bool - live []fleet.Rec - made, kept, warmed []string + // label is the repo label in a seat's role. Distinct from base, which names the + // seat directory: the directory can be called anything, the label must match the + // rest of the repo's roles.map lines. + label string + rewarm bool + cmds []string + named map[string]string + worktrees map[string]bool + live []fleet.Rec + made, kept, warmed []string } // one creates or keeps one seat: never disturbs an occupied one, refuses a name @@ -712,7 +733,7 @@ func (p *pooler) one(k string, i int) error { return refuse("fleet pool: git worktree add %s failed: %s", path, out) } } - if err := cmdRole(path, k+":"+p.base, false, p.tenant, slot); err != nil { + if err := cmdRole(path, k+":"+p.label, false, p.tenant, slot); err != nil { return err } if fresh { @@ -843,12 +864,18 @@ func assignGuards(slot, path, branch string) error { // assignCheckout puts the seat's tree on the branch, creating it from origin when // it is not local, and confirms where the tree landed. func assignCheckout(slot, path, branch string) error { - rc, txt := gitTry(path, gitTimeout, "checkout", "--quiet", branch) + rc, first := gitTry(path, gitTimeout, "checkout", "--quiet", branch) if rc != 0 { - rc, txt = gitTry(path, gitTimeout, "checkout", "--quiet", "-b", branch, "origin/"+branch) - } - if rc != 0 { - return refuse("fleet assign: could not check out %s in %s: %s; nothing was assigned", branch, slot, txt) + // The -b fallback is only for a branch that is not local yet, so when it fails + // too, the first attempt's message is the one worth printing: it names the + // worktree already holding the branch. The fallback can only say the branch + // exists, which is what the first attempt just established, and reporting that + // instead sends the reader back to the checkout that already failed. This is the + // common case when a seat takes over an existing branch, because a branch already + // worked on this machine is local. + if rc2, _ := gitTry(path, gitTimeout, "checkout", "--quiet", "-b", branch, "origin/"+branch); rc2 != 0 { + return refuse("fleet assign: could not check out %s in %s: %s; nothing was assigned", branch, slot, first) + } } _, landed := gitTry(path, gitTimeout, "rev-parse", "--abbrev-ref", "HEAD") if landed != branch { From 1171b7e11407f262dec63e159462518ee173aec7 Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 11:44:41 -0700 Subject: [PATCH 03/10] feat(fleet): say when a seat keeps a deny its manifest no longer declares writeDenies unions the seat's existing denies with the manifest's, so a deny can only ever be added. Dropping one from a lane manifest is a no-op against seats already roled, and a seat's permissions become a high-water mark of every deny that lane ever declared. The merge itself is deliberate and stays: it preserves denies a person added to settings.local.json by hand. What was missing is any signal that a narrowing did not take. Roling two seats of one kind in a single pool run, from one manifest, produced two seats enforcing different rules and said nothing. writeDenies now also returns the denies present in the file that the manifest does not declare, and cmdRole names them and points at the file to edit. Nothing about what gets written changes. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/fleet/internal/verbs/role.go | 39 +++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/cmd/fleet/internal/verbs/role.go b/cmd/fleet/internal/verbs/role.go index aba76dfb..f701e076 100644 --- a/cmd/fleet/internal/verbs/role.go +++ b/cmd/fleet/internal/verbs/role.go @@ -423,22 +423,39 @@ func writeMapLine(lines []string, same []fleet.MapRow, mapfile, checkout, tenant } // writeDenies merges the manifest's denies into the checkout's Claude settings and -// writes them; returns the merged deny list. indent=2: a file a human maintains by -// hand; do not collapse it. -func writeDenies(existing, manifest map[string]any, settingsTarget string) ([]string, error) { +// writes them; returns the merged deny list and the denies present in the file that the +// manifest no longer declares. indent=2: a file a human maintains by hand; do not +// collapse it. +// +// The merge is deliberate: it preserves denies a human added to settings.local.json. +// The cost is that a deny can only ever be ADDED to a seat, so dropping one from a lane +// manifest is a no-op against seats already roled, and a seat's permissions become a +// high-water mark of every deny its lane ever declared. That silence produced two seats +// of one kind, roled in one `fleet pool` run from one manifest, enforcing different +// rules. Returning the extras does not change what is written; it lets the caller say +// that a narrowing did not take. +func writeDenies(existing, manifest map[string]any, settingsTarget string) (deny, extra []string, err error) { perms, _ := existing["permissions"].(map[string]any) if perms == nil { perms = map[string]any{} existing["permissions"] = perms } + fromManifest := map[string]bool{} + for _, d := range fleet.Strs(manifest, "denies") { + fromManifest[d] = true + } denySet := map[string]bool{} for _, d := range fleet.Strs(perms, "deny") { denySet[d] = true + if !fromManifest[d] { + extra = append(extra, d) + } } - for _, d := range fleet.Strs(manifest, "denies") { + for d := range fromManifest { denySet[d] = true } - deny := sortedKeys(denySet) + sort.Strings(extra) + deny = sortedKeys(denySet) denyAny := make([]any, len(deny)) for i, d := range deny { denyAny[i] = d @@ -446,9 +463,9 @@ func writeDenies(existing, manifest map[string]any, settingsTarget string) ([]st perms["deny"] = denyAny sb, _ := json.MarshalIndent(existing, "", " ") if err := os.WriteFile(settingsTarget, append(sb, '\n'), 0o644); err != nil { - return nil, err + return nil, nil, err } - return deny, nil + return deny, extra, nil } func roleUnderLock(checkout, role string, force bool, tenant, slot, kind string, manifest map[string]any, card, cfgTarget, cfgText, hooksTarget string, hooksData map[string]any, rulesTarget, settingsTarget string, existing map[string]any, mapfile string) error { @@ -485,7 +502,7 @@ func roleUnderLock(checkout, role string, force bool, tenant, slot, kind string, if err := os.MkdirAll(filepath.Join(checkout, ".claude"), 0o755); err != nil { return err } - deny, err := writeDenies(existing, manifest, settingsTarget) + deny, extraDenies, err := writeDenies(existing, manifest, settingsTarget) if err != nil { return err } @@ -506,6 +523,12 @@ func roleUnderLock(checkout, role string, force bool, tenant, slot, kind string, if excluded := excludeLocalArtifacts(checkout); excluded != "" { note = " All generated files excluded via " + excluded + "." } + // A deny the manifest dropped stays in the seat's settings, so say so: otherwise a + // narrowing looks applied and is not. + if len(extraDenies) > 0 { + note += fmt.Sprintf(" NOTE: %d deny(s) in this seat are not in the manifest and were kept: %s. Remove them from %s by hand to narrow this seat.", + len(extraDenies), strings.Join(extraDenies, ", "), fleet.LongPath(settingsTarget)) + } say("%s is now %s (manifest %s): Claude card + %d denies; Codex developer card + user hooks + %d exact command rules. Open a NEW Codex tab there, trust the project configuration, then trust the hook definitions.%s", checkout, role, fleet.LongPath(filepath.Join(fleet.LanesDir(), kind, "manifest.json")), len(deny), strings.Count(rules, "prefix_rule("), note) return nil From 4af5d438dc238960e730d6c315c740a27980257e Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 11:44:41 -0700 Subject: [PATCH 04/10] fix(fleet): install.sh --rollback could restore a backup it never wrote rollback picked `ls -1t "$f".bak-* | head -1`. The ordering is right, the glob is not: it also matches an unrelated sibling someone left beside the file, and restoring one over live harness config silently drops whatever else that snapshot did not contain. This happened with a settings.json.bak-env, which cost three env vars before it was noticed. Constrain the glob to the timestamp shape the script itself writes. A restore target should be a file the tool can prove it created. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/fleet/install.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/fleet/install.sh b/cmd/fleet/install.sh index 9b65256d..de5667f6 100755 --- a/cmd/fleet/install.sh +++ b/cmd/fleet/install.sh @@ -40,8 +40,13 @@ plan() { say " [$mode] $*"; } rollback() { for f in "$claude_settings" "$codex_hooks"; do - latest="$(ls -1t "$f".bak-* 2>/dev/null | head -1 || true)" - if [ -z "$latest" ]; then say "no backup for $f; nothing to restore"; continue; fi + # Only backups THIS script wrote, i.e. `.bak-` plus the $stamp shape. `.bak-*` also + # matches an unrelated sibling a person left beside the file — `settings.json.bak-env` + # is a real example — and restoring one of those over live harness config is a silent + # loss of whatever else it did not contain. A restore target should be a file the tool + # can prove it created. + latest="$(ls -1t "$f".bak-[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]-[0-9][0-9][0-9][0-9][0-9][0-9] 2>/dev/null | head -1 || true)" + if [ -z "$latest" ]; then say "no backup written by this script for $f; nothing to restore"; continue; fi say "restore $latest -> $f" cp "$latest" "$f" done From 3efd774582db3429474c162d22343f49a3c3540e Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 12:29:32 -0700 Subject: [PATCH 05/10] fix(fleet): let a main checkout stay unroled without breaking pool Leaving a main checkout unroled is the right call once people open ad-hoc sessions in it: otherwise every such session boots wearing a lane's card, and for an exclusive lane that means two sessions on one role. But both the tenant and the repo label were inherited from that single line, so removing it broke pool two ways. poolTenant refused for want of a tenant. poolLabel fell back to the directory basename, and since pool re-roles the seats it KEEPS, that silently re-labelled every existing seat of the repo. Add sameRepoRow: a sibling seat of the same repo, matched on RepoID, knows both answers. Consulted after the checkout's own line and before ORG_TENANT, so explicit configuration still wins and nothing about the roled case changes. Verified against a real pool: with no line for the checkout at all, `fleet pool` inherits tenant and label from a sibling seat, keeps six seats, and leaves every label intact. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/fleet/internal/verbs/views.go | 52 ++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/cmd/fleet/internal/verbs/views.go b/cmd/fleet/internal/verbs/views.go index 906fe2d6..f6fd8c81 100644 --- a/cmd/fleet/internal/verbs/views.go +++ b/cmd/fleet/internal/verbs/views.go @@ -665,33 +665,61 @@ func anyKeys(m map[string]any) map[string]bool { return out } -// poolTenant settles the tenant BEFORE any worktree exists: given, inherited from -// the checkout's own map line, or ORG_TENANT. +// sameRepoRow is an existing map row for a checkout of the same repo, or a zero row. +// +// The seam that lets a MAIN checkout stay unroled. Leaving one unroled is the right call +// once people open ad-hoc sessions in it, since otherwise every such session boots +// wearing a lane's card. But both the tenant and the label were inherited from that one +// line, so removing it made `fleet pool` refuse for want of a tenant and fall back to the +// directory name for the label — and because pool re-roles the seats it KEEPS, that +// silently re-labels every existing seat. A sibling seat of the same repo knows both +// answers, so ask it. +func sameRepoRow(checkout string) fleet.MapRow { + want := fleet.RepoID(checkout) + if want == "" { + return fleet.MapRow{} + } + _, rows := fleet.MapRows(fleet.RolesMap()) + for _, r := range rows { + if fleet.RepoID(r.Path) == want { + return r + } + } + return fleet.MapRow{} +} + +// poolTenant settles the tenant BEFORE any worktree exists: given, inherited from the +// checkout's own map line, from a sibling seat of the same repo, or ORG_TENANT. func poolTenant(tenant, checkout, base string) (string, error) { if tenant == "" { tenant = fleet.TenantOf(checkout) } + if tenant == "" { + tenant = sameRepoRow(checkout).Tenant + } if tenant == "" { tenant = os.Getenv("ORG_TENANT") } if tenant == "" { - return "", refuse("fleet pool: no tenant for slots of %s: %s has no roles.map line to inherit from and ORG_TENANT is unset. Next action: fleet pool %s ... --tenant ", base, checkout, checkout) + return "", refuse("fleet pool: no tenant for slots of %s: %s has no roles.map line to inherit from, no sibling seat of that repo has one, and ORG_TENANT is unset. Next action: fleet pool %s ... --tenant ", base, checkout, checkout) } return tenant, nil } // poolLabel is the repo label a seat's role carries, the `mono` in `:mono`. // -// Inherited from the checkout's own map line, the way poolTenant inherits the tenant, -// and derived from the directory name only when there is no line to inherit from. A -// directory basename is not a repo identity: a checkout at `~/dev/Mono` produced -// `:Mono` beside an existing `:mono`, which is two labels for one repo, so -// `fleet work --for :mono` matched none of the new seats. Because pool re-roles -// the seats it KEEPS and not only the ones it creates, deriving the label here also -// overwrote a corrected value on every top-up, leaving no durable fix outside the tool. +// Inherited from the checkout's own map line, then from a sibling seat of the same repo, +// and derived from the directory name only when neither exists. A directory basename is +// not a repo identity: a checkout at `~/dev/Mono` produced `:Mono` beside an +// existing `:mono`, which is two labels for one repo, so `fleet work --for +// :mono` matched none of the new seats. Because pool re-roles the seats it KEEPS +// and not only the ones it creates, deriving the label here also overwrote a corrected +// value on every top-up, leaving no durable fix outside the tool. func poolLabel(checkout, base string) string { - role := fleet.RoleOf(checkout) - if _, label, ok := strings.Cut(role, ":"); ok && label != "" { + if _, label, ok := strings.Cut(fleet.RoleOf(checkout), ":"); ok && label != "" { + return label + } + if _, label, ok := strings.Cut(sameRepoRow(checkout).Role, ":"); ok && label != "" { return label } return base From 0af61008b5e870e49a663b4f08dc30e41a5e71f9 Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 12:56:01 -0700 Subject: [PATCH 06/10] fix(fleet): the board said "no one accountable" about work whose assignment named a role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fleet assign --for ` records the accountable role, the dispatcher, the seat and the brief. undeclaredRows read none of it and hardcoded them nil, so the board printed "for no one accountable" about a change whose own assignment record named the role. Two records fleet wrote, disagreeing, and the operator reasonably read the board as the truth. The STATE stays `undeclared` — that was right, a seat assignment is not an ownership declaration — but the columns now come from the assignment when there is one. A branch nobody assigned still reads "no one accountable", so the change is precise rather than blanket. Also rename undeliveredAssigns to assignsByChange. Its comment claimed "every assignment not yet read by a session", but nothing in it filters `delivered_to`; that filter lives in the board's row builder. Reusing it under the old name would have looked like it dropped delivered assignments when it does not. Verified on a live board: two assigned seats now group under author:nx-apps and liverun:sidebar and name their seats, while an unassigned branch still reports no one accountable. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/fleet/internal/verbs/views.go | 11 ++++++++--- cmd/fleet/internal/verbs/work.go | 21 +++++++++++++++++++-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/cmd/fleet/internal/verbs/views.go b/cmd/fleet/internal/verbs/views.go index f6fd8c81..6ed206d9 100644 --- a/cmd/fleet/internal/verbs/views.go +++ b/cmd/fleet/internal/verbs/views.go @@ -1127,7 +1127,7 @@ func Unowned(repo string) map[string]any { } pairs = filtered } - assigns := undeliveredAssigns() + assigns := assignsByChange() host, _ := os.Hostname() if host == "" { host = "?" @@ -1172,8 +1172,13 @@ func mapKeys[V any](m map[string]V) map[string]bool { return out } -// undeliveredAssigns is every assignment not yet read by a session, by (repo, branch). -func undeliveredAssigns() map[[2]string]fleet.Rec { +// assignsByChange is EVERY assignment record, keyed by (repo, branch). +// +// Named for what it returns. It was called undeliveredAssigns, whose comment claimed "not +// yet read by a session" — but nothing here filters `delivered_to`; that filter lives in +// the board's row builder. A caller trusting the old name would have silently dropped +// every assignment a session had already picked up. +func assignsByChange() map[[2]string]fleet.Rec { assigns := map[[2]string]fleet.Rec{} d := fleet.Path("assign") ents, _ := os.ReadDir(d) diff --git a/cmd/fleet/internal/verbs/work.go b/cmd/fleet/internal/verbs/work.go index 8ee1760e..4e4c829d 100644 --- a/cmd/fleet/internal/verbs/work.go +++ b/cmd/fleet/internal/verbs/work.go @@ -337,8 +337,17 @@ func evidenceState(row WorkRow, rid, branch, rel, state string) string { // undeclaredRows is every branch a session holds that no row declares: undeclared // while the holder lives, dead once it does not. +// +// `undeclared` means no ownership ROW exists, which is not the same as nobody being +// accountable. `fleet assign --for ` records the accountable role, the dispatcher, +// the seat and the brief in the assignment; reading none of that made the board print +// "for no one accountable" about a change whose own assignment named the role — two +// records fleet wrote, disagreeing. The STATE stays `undeclared`, which was right: a seat +// assignment is not an ownership declaration. The columns now come from the assignment +// when there is one. func undeclaredRows(declared map[string]bool) []WorkRow { var rows []WorkRow + assigns := assignsByChange() for _, l := range leaseRows() { key := fleet.S(l, "key") if fleet.B(l, "occupancy") || fleet.IsResource(key) || declared[key] { @@ -350,8 +359,16 @@ func undeclaredRows(declared map[string]bool) []WorkRow { state = "dead" // a dead holder nobody declared is still a dead holder } parts := fleet.KeyParts(key) - rows = append(rows, WorkRow{"change": fleet.S(parts, "branch"), "repo": fleet.S(parts, "repo"), "relationship": nil, "for": nil, "by": nil, - "at": l["at"], "due": nil, "slot": nil, "brief": nil, "key": key, "hands": sid, "state": state, "head": nil, "done_at": nil}) + repo, branch := fleet.S(parts, "repo"), fleet.S(parts, "branch") + row := WorkRow{"change": branch, "repo": repo, "relationship": nil, "for": nil, "by": nil, + "at": l["at"], "due": nil, "slot": nil, "brief": nil, "key": key, "hands": sid, "state": state, "head": nil, "done_at": nil} + if a := assigns[[2]string{repo, branch}]; a != nil { + row["for"] = nilIfEmpty(fleet.S(a, "for")) + row["by"] = nilIfEmpty(fleet.S(a, "by")) + row["slot"] = nilIfEmpty(fleet.S(a, "slot")) + row["brief"] = nilIfEmpty(fleet.S(a, "brief")) + } + rows = append(rows, row) } return rows } From 925b785a8076d9dbdc594cb89f0b47a9b9ae6700 Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 19:45:23 -0700 Subject: [PATCH 07/10] fix(fleet): refuse ambiguous pool inheritance before changing seats --- .github/workflows/fleet-portability.yml | 41 +++++ cmd/fleet/install_test.go | 27 ++++ .../internal/verbs/pool_identity_test.go | 150 ++++++++++++++++++ cmd/fleet/internal/verbs/views.go | 96 ++++++----- .../internal/watch/detach_windows_test.go | 23 +++ friction-log.md | 13 ++ 6 files changed, 308 insertions(+), 42 deletions(-) create mode 100644 .github/workflows/fleet-portability.yml create mode 100644 cmd/fleet/internal/verbs/pool_identity_test.go create mode 100644 cmd/fleet/internal/watch/detach_windows_test.go diff --git a/.github/workflows/fleet-portability.yml b/.github/workflows/fleet-portability.yml new file mode 100644 index 00000000..6b411819 --- /dev/null +++ b/.github/workflows/fleet-portability.yml @@ -0,0 +1,41 @@ +name: Fleet portability + +on: + pull_request: + paths: + - 'cmd/fleet/**' + - '.github/workflows/fleet-portability.yml' + - 'go.mod' + - 'go.sum' + push: + branches: [main] + paths: + - 'cmd/fleet/**' + - '.github/workflows/fleet-portability.yml' + - 'go.mod' + - 'go.sum' + +permissions: + contents: read + +concurrency: + group: fleet-portability-${{ github.ref }} + cancel-in-progress: true + +jobs: + seats: + strategy: + fail-fast: false + matrix: + os: [macos-latest, windows-latest] + runs-on: ${{ matrix.os }} + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Real Git seat identity and deny preservation + run: go test ./cmd/fleet/internal/verbs -run 'TestPool|TestWriteDenies' -count=1 + - name: Watcher tests (Windows includes process flags, not visual proof) + run: go test ./cmd/fleet/internal/watch -count=1 diff --git a/cmd/fleet/install_test.go b/cmd/fleet/install_test.go index 618b9de5..ea5f80ca 100644 --- a/cmd/fleet/install_test.go +++ b/cmd/fleet/install_test.go @@ -67,3 +67,30 @@ func TestInstallerRemovesOnlyFleetShadowHooks(t *testing.T) { t.Fatal(first) } } + +func TestInstallerRollbackIgnoresUnrelatedBackup(t *testing.T) { + bash, err := exec.LookPath("bash") + if err != nil { + t.Skip("bash unavailable") + } + root := t.TempDir() + dir := filepath.Join(root, ".claude") + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + p := filepath.Join(dir, "settings.json") + for name, body := range map[string]string{p: "current", p + ".bak-20260908-120000": "expected", p + ".bak-env": "unrelated"} { + if err := os.WriteFile(name, []byte(body), 0600); err != nil { + t.Fatal(err) + } + } + c := exec.Command(bash, "install.sh", "--rollback") + c.Env = append(os.Environ(), "HOME="+root, "FLEET_HOME="+filepath.Join(root, "fleet")) + if out, err := c.CombinedOutput(); err != nil { + t.Fatalf("rollback: %v %s", err, out) + } + got, err := os.ReadFile(p) + if err != nil || string(got) != "expected" { + t.Fatalf("restored=%q err=%v", got, err) + } +} diff --git a/cmd/fleet/internal/verbs/pool_identity_test.go b/cmd/fleet/internal/verbs/pool_identity_test.go new file mode 100644 index 00000000..6e2a4335 --- /dev/null +++ b/cmd/fleet/internal/verbs/pool_identity_test.go @@ -0,0 +1,150 @@ +package verbs + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/itsHabib/workbench/cmd/fleet/internal/fleet" +) + +func poolFixture(t *testing.T) (string, string, string) { + t.Helper() + root := t.TempDir() + old := fleet.OrgState + fleet.OrgState = root + t.Cleanup(func() { fleet.OrgState = old }) + t.Setenv("ORG_TENANT", "") + repo := filepath.Join(root, "Mono") + run := func(args ...string) { + t.Helper() + c := exec.Command("git", args...) + if b, e := c.CombinedOutput(); e != nil { + t.Fatalf("git %v: %v %s", args, e, b) + } + } + run("init", repo) + run("-C", repo, "-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--allow-empty", "-m", "fixture") + a, b := filepath.Join(root, "seat-a"), filepath.Join(root, "seat-b") + run("-C", repo, "worktree", "add", "--detach", a) + run("-C", repo, "worktree", "add", "--detach", b) + return repo, a, b +} + +func poolMap(t *testing.T, lines ...string) { + t.Helper() + if err := os.WriteFile(fleet.RolesMap(), []byte(strings.Join(lines, "\n")+"\n"), 0600); err != nil { + t.Fatal(err) + } +} + +func TestPoolTenantRejectsAmbiguousSiblings(t *testing.T) { + repo, a, b := poolFixture(t) + poolMap(t, a+" first worker:mono", b+" second worker:mono") + tenant, err := poolTenant("", repo, "Mono") + if err == nil || !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("wanted ambiguous tenant refusal; tenant=%q err=%v", tenant, err) + } + if got, err := poolTenant("second", repo, "Mono"); err != nil || got != "second" { + t.Fatalf("explicit tenant=%q err=%v", got, err) + } +} + +func TestPoolLabelStaysWithinTenant(t *testing.T) { + repo, a, b := poolFixture(t) + poolMap(t, a+" first worker:foreign", b+" second worker:mono") + label, err := poolLabel(repo, "Mono", "second") + if err != nil || label != "mono" { + t.Fatalf("label=%q err=%v", label, err) + } +} + +func TestPoolLabelRejectsAmbiguousSiblings(t *testing.T) { + repo, a, b := poolFixture(t) + poolMap(t, a+" work worker:Mono", b+" work worker:mono") + _, err := poolLabel(repo, "Mono", "work") + if err == nil || !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("wanted ambiguous label refusal: %v", err) + } +} + +func TestPoolInheritsUnanimousLabelWithoutRolingMain(t *testing.T) { + repo, a, b := poolFixture(t) + poolMap(t, a+" work worker:mono", b+" work checker:mono") + tenant, err := poolTenant("", repo, "Mono") + if err != nil || tenant != "work" { + t.Fatalf("tenant=%q err=%v", tenant, err) + } + label, err := poolLabel(repo, "Mono", tenant) + if err != nil || label != "mono" { + t.Fatalf("label=%q err=%v", label, err) + } + if fleet.RoleOf(repo) != "" { + t.Fatal("main gained a role") + } +} + +func TestPoolExactBindingWinsOverSiblingLabel(t *testing.T) { + repo, a, _ := poolFixture(t) + poolMap(t, repo+" work lead:preferred", a+" work worker:old") + label, err := poolLabel(repo, "Mono", "work") + if err != nil || label != "preferred" { + t.Fatalf("label=%q err=%v", label, err) + } +} + +func TestWriteDeniesReportsAndPreservesExtras(t *testing.T) { + existing := map[string]any{"permissions": map[string]any{"deny": []any{"Bash(old)", "Bash(shared)"}}, "unrelated": true} + manifest := map[string]any{"denies": []any{"Bash(shared)", "Bash(new)"}} + target := filepath.Join(t.TempDir(), "settings.json") + deny, extra, err := writeDenies(existing, manifest, target) + if err != nil || strings.Join(extra, ",") != "Bash(old)" || len(deny) != 3 { + t.Fatalf("deny=%v extra=%v err=%v", deny, extra, err) + } + got := fleet.ReadJSON(target) + if !fleet.B(got, "unrelated") || len(fleet.Strs(fleet.M(got, "permissions"), "deny")) != 3 { + t.Fatalf("written settings=%v", got) + } +} + +func TestPoolAmbiguityRefusesBeforeCreatingSeats(t *testing.T) { + repo, a, b := poolFixture(t) + oldState := fleet.State + fleet.State = t.TempDir() + t.Cleanup(func() { fleet.State = oldState }) + lanes, err := filepath.Abs("../../testdata/lanes") + if err != nil { + t.Fatal(err) + } + t.Setenv("FLEET_LANES", lanes) + poolMap(t, a+" work worker:Mono", b+" work worker:mono") + before, err := os.ReadFile(fleet.RolesMap()) + if err != nil { + t.Fatal(err) + } + err = cmdPool(repo, "author", "1", false, "work") + if err == nil || !strings.Contains(err.Error(), "ambiguous labels") { + t.Fatalf("expected label ambiguity, got %v", err) + } + after, err := os.ReadFile(fleet.RolesMap()) + if err != nil || string(before) != string(after) { + t.Fatalf("roles changed on refusal: %v", err) + } + if _, err := os.Stat(filepath.Join(filepath.Dir(repo), "Mono-author-1")); !os.IsNotExist(err) { + t.Fatalf("seat created on refusal: %v", err) + } +} + +func TestAssignReportsHoldingWorktree(t *testing.T) { + repo, a, _ := poolFixture(t) + branch := fleet.BranchOf(repo) + err := assignCheckout("seat-a", a, branch) + if err == nil || !strings.Contains(strings.ReplaceAll(err.Error(), "\\", "/"), strings.ReplaceAll(repo, "\\", "/")) { + t.Fatalf("missing holding checkout %s: %v", repo, err) + } + if fleet.BranchOf(a) != "" { + t.Fatal("refused assignment changed seat branch") + } +} diff --git a/cmd/fleet/internal/verbs/views.go b/cmd/fleet/internal/verbs/views.go index 6ed206d9..972ae308 100644 --- a/cmd/fleet/internal/verbs/views.go +++ b/cmd/fleet/internal/verbs/views.go @@ -583,7 +583,11 @@ func cmdPool(checkout, kind, nArg string, rewarm bool, tenant string) error { if err != nil { return err } - p := newPooler(checkout, base, parent, tenant, poolLabel(checkout, base), rewarm, cfg) + label, err := poolLabel(checkout, base, tenant) + if err != nil { + return err + } + p := newPooler(checkout, base, parent, tenant, label, rewarm, cfg) for _, k := range order { for i := 1; i <= wanted[k]; i++ { if err := p.one(k, i); err != nil { @@ -665,64 +669,72 @@ func anyKeys(m map[string]any) map[string]bool { return out } -// sameRepoRow is an existing map row for a checkout of the same repo, or a zero row. -// -// The seam that lets a MAIN checkout stay unroled. Leaving one unroled is the right call -// once people open ad-hoc sessions in it, since otherwise every such session boots -// wearing a lane's card. But both the tenant and the label were inherited from that one -// line, so removing it made `fleet pool` refuse for want of a tenant and fall back to the -// directory name for the label — and because pool re-roles the seats it KEEPS, that -// silently re-labels every existing seat. A sibling seat of the same repo knows both -// answers, so ask it. -func sameRepoRow(checkout string) fleet.MapRow { +// sameRepoRows finds bindings that share the checkout's git common directory. +// A repository may have several tenants or labels; file order is not authority. +func sameRepoRows(checkout string) []fleet.MapRow { want := fleet.RepoID(checkout) if want == "" { - return fleet.MapRow{} + return nil } _, rows := fleet.MapRows(fleet.RolesMap()) + var found []fleet.MapRow for _, r := range rows { if fleet.RepoID(r.Path) == want { - return r + found = append(found, r) } } - return fleet.MapRow{} + return found } -// poolTenant settles the tenant BEFORE any worktree exists: given, inherited from the -// checkout's own map line, from a sibling seat of the same repo, or ORG_TENANT. +// poolTenant inherits a sibling tenant only when all candidates agree. Resolve +// this before creating any seat; an explicit tenant can disambiguate the pool. func poolTenant(tenant, checkout, base string) (string, error) { - if tenant == "" { - tenant = fleet.TenantOf(checkout) + if tenant != "" { + return tenant, nil + } + if tenant = fleet.TenantOf(checkout); tenant != "" { + return tenant, nil } - if tenant == "" { - tenant = sameRepoRow(checkout).Tenant + seen := map[string]bool{} + for _, r := range sameRepoRows(checkout) { + seen[r.Tenant] = true } - if tenant == "" { - tenant = os.Getenv("ORG_TENANT") + if len(seen) > 1 { + return "", refuse("fleet pool: ambiguous tenants for %s in %s: %s; select --tenant before creating seats", base, fleet.RolesMap(), strings.Join(sortedKeys(seen), ", ")) } - if tenant == "" { - return "", refuse("fleet pool: no tenant for slots of %s: %s has no roles.map line to inherit from, no sibling seat of that repo has one, and ORG_TENANT is unset. Next action: fleet pool %s ... --tenant ", base, checkout, checkout) + for t := range seen { + return t, nil } - return tenant, nil + if tenant = os.Getenv("ORG_TENANT"); tenant != "" { + return tenant, nil + } + return "", refuse("fleet pool: no tenant for slots of %s: %s has no roles.map line to inherit from, no sibling seat of that repo has one, and ORG_TENANT is unset. Next action: fleet pool %s ... --tenant ", base, checkout, checkout) } -// poolLabel is the repo label a seat's role carries, the `mono` in `:mono`. -// -// Inherited from the checkout's own map line, then from a sibling seat of the same repo, -// and derived from the directory name only when neither exists. A directory basename is -// not a repo identity: a checkout at `~/dev/Mono` produced `:Mono` beside an -// existing `:mono`, which is two labels for one repo, so `fleet work --for -// :mono` matched none of the new seats. Because pool re-roles the seats it KEEPS -// and not only the ones it creates, deriving the label here also overwrote a corrected -// value on every top-up, leaving no durable fix outside the tool. -func poolLabel(checkout, base string) string { - if _, label, ok := strings.Cut(fleet.RoleOf(checkout), ":"); ok && label != "" { - return label - } - if _, label, ok := strings.Cut(sameRepoRow(checkout).Role, ":"); ok && label != "" { - return label - } - return base +// poolLabel inherits only within the selected tenant. Conflicting labels require +// reconciliation, not silently rewriting kept seats to the first row's spelling. +func poolLabel(checkout, base, tenant string) (string, error) { + if fleet.TenantOf(checkout) == tenant { + if _, label, ok := strings.Cut(fleet.RoleOf(checkout), ":"); ok && label != "" { + return label, nil + } + } + labels := map[string]bool{} + for _, r := range sameRepoRows(checkout) { + if r.Tenant != tenant { + continue + } + if _, label, ok := strings.Cut(r.Role, ":"); ok && label != "" { + labels[label] = true + } + } + if len(labels) > 1 { + return "", refuse("fleet pool: ambiguous labels for tenant %s in %s: %s; reconcile the bindings before creating seats", tenant, fleet.RolesMap(), strings.Join(sortedKeys(labels), ", ")) + } + for label := range labels { + return label, nil + } + return base, nil } // pooler is one `fleet pool` run: what it knows before the loop, what it did. diff --git a/cmd/fleet/internal/watch/detach_windows_test.go b/cmd/fleet/internal/watch/detach_windows_test.go new file mode 100644 index 00000000..de9dabb0 --- /dev/null +++ b/cmd/fleet/internal/watch/detach_windows_test.go @@ -0,0 +1,23 @@ +//go:build windows + +package watch + +import ( + "os/exec" + "testing" +) + +func TestDetachHasNoConsoleWindow(t *testing.T) { + cmd := exec.Command("unused") + detach(cmd) + const noWindow = 0x08000000 + const detachedProcess = 0x00000008 + const newGroup = 0x00000200 + if cmd.SysProcAttr == nil { + t.Fatal("missing process attributes") + } + flags := cmd.SysProcAttr.CreationFlags + if flags&noWindow == 0 || flags&newGroup == 0 || flags&detachedProcess != 0 { + t.Fatalf("unexpected creation flags: %#x", flags) + } +} diff --git a/friction-log.md b/friction-log.md index 2ea0942c..ed91ba33 100644 --- a/friction-log.md +++ b/friction-log.md @@ -359,3 +359,16 @@ Second occurrence of the `#214` entry above, one failure mode further in. - **Workaround available today:** post the bare `@claude please review` as its own comment so the attestation fires, and put the focus areas in a second comment. Costs nothing and keeps the panel complete. + +### 2026-09-08 — Pool inherited the first sibling's tenant and label + +- What I tried: review work-machine Fleet #289 on macOS using temporary Git worktrees. +- What happened: two bindings of one repository in different tenants made poolTenant + silently select the first row. The same first-row lookup could supply a label from a + different tenant, or conceal conflicting labels before a pool top-up re-roled seats. +- Class: wrong-default. +- Smallest fix: inherit only an unambiguous tenant, filter sibling labels by the selected + tenant, and refuse conflicting labels before creating seats or rewriting roles.map. +- Status: fixed with real Git regression tests; the new tenant test failed on 0af6100. + Mac Fleet race tests pass. Windows execution is delegated to the portability CI job; + a visible-window check still needs the work machine's next real session start. From 077fcd960a19fe2e813f9854f46f7508f1bcd79b Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 19:52:55 -0700 Subject: [PATCH 08/10] fix(fleet): bind board context to its holder and honor tenant ambiguity --- .github/workflows/fleet-portability.yml | 2 +- .../internal/verbs/pool_identity_test.go | 9 ++++++ cmd/fleet/internal/verbs/views.go | 8 +++-- cmd/fleet/internal/verbs/work.go | 21 ++++++++++-- .../internal/verbs/work_assignment_test.go | 32 +++++++++++++++++++ cmd/fleet/internal/watch/detach_windows.go | 3 +- friction-log.md | 10 ++++++ 7 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 cmd/fleet/internal/verbs/work_assignment_test.go diff --git a/.github/workflows/fleet-portability.yml b/.github/workflows/fleet-portability.yml index 6b411819..2f1c84cb 100644 --- a/.github/workflows/fleet-portability.yml +++ b/.github/workflows/fleet-portability.yml @@ -36,6 +36,6 @@ jobs: with: go-version-file: go.mod - name: Real Git seat identity and deny preservation - run: go test ./cmd/fleet/internal/verbs -run 'TestPool|TestWriteDenies' -count=1 + run: go test ./cmd/fleet/internal/verbs -run 'TestPool|TestWriteDenies|TestAssignReports|TestUndeclaredUsesHolderAssignment' -count=1 - name: Watcher tests (Windows includes process flags, not visual proof) run: go test ./cmd/fleet/internal/watch -count=1 diff --git a/cmd/fleet/internal/verbs/pool_identity_test.go b/cmd/fleet/internal/verbs/pool_identity_test.go index 6e2a4335..9be8cd16 100644 --- a/cmd/fleet/internal/verbs/pool_identity_test.go +++ b/cmd/fleet/internal/verbs/pool_identity_test.go @@ -148,3 +148,12 @@ func TestAssignReportsHoldingWorktree(t *testing.T) { t.Fatal("refused assignment changed seat branch") } } + +func TestPoolAncestorDoesNotHideSiblingAmbiguity(t *testing.T) { + repo, a, b := poolFixture(t) + poolMap(t, filepath.Dir(repo)+" inherited lead:parent", a+" first worker:mono", b+" second worker:mono") + _, err := poolTenant("", repo, "Mono") + if err == nil || !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("ancestor hid tenant ambiguity: %v", err) + } +} diff --git a/cmd/fleet/internal/verbs/views.go b/cmd/fleet/internal/verbs/views.go index 972ae308..e7248946 100644 --- a/cmd/fleet/internal/verbs/views.go +++ b/cmd/fleet/internal/verbs/views.go @@ -692,8 +692,9 @@ func poolTenant(tenant, checkout, base string) (string, error) { if tenant != "" { return tenant, nil } - if tenant = fleet.TenantOf(checkout); tenant != "" { - return tenant, nil + role, inherited, _ := fleet.MapRowsFor(checkout) + if role != "" && inherited != "" { + return inherited, nil } seen := map[string]bool{} for _, r := range sameRepoRows(checkout) { @@ -705,6 +706,9 @@ func poolTenant(tenant, checkout, base string) (string, error) { for t := range seen { return t, nil } + if inherited != "" { + return inherited, nil + } if tenant = os.Getenv("ORG_TENANT"); tenant != "" { return tenant, nil } diff --git a/cmd/fleet/internal/verbs/work.go b/cmd/fleet/internal/verbs/work.go index 4e4c829d..a51fbe6d 100644 --- a/cmd/fleet/internal/verbs/work.go +++ b/cmd/fleet/internal/verbs/work.go @@ -347,7 +347,6 @@ func evidenceState(row WorkRow, rid, branch, rel, state string) string { // when there is one. func undeclaredRows(declared map[string]bool) []WorkRow { var rows []WorkRow - assigns := assignsByChange() for _, l := range leaseRows() { key := fleet.S(l, "key") if fleet.B(l, "occupancy") || fleet.IsResource(key) || declared[key] { @@ -362,7 +361,7 @@ func undeclaredRows(declared map[string]bool) []WorkRow { repo, branch := fleet.S(parts, "repo"), fleet.S(parts, "branch") row := WorkRow{"change": branch, "repo": repo, "relationship": nil, "for": nil, "by": nil, "at": l["at"], "due": nil, "slot": nil, "brief": nil, "key": key, "hands": sid, "state": state, "head": nil, "done_at": nil} - if a := assigns[[2]string{repo, branch}]; a != nil { + if a := holderAssignment(repo, branch, sid); a != nil { row["for"] = nilIfEmpty(fleet.S(a, "for")) row["by"] = nilIfEmpty(fleet.S(a, "by")) row["slot"] = nilIfEmpty(fleet.S(a, "slot")) @@ -373,6 +372,24 @@ func undeclaredRows(declared map[string]bool) []WorkRow { return rows } +// holderAssignment reads the holder's seat, never a branch-wide collapse of +// assignments left behind in other seats. Another session's delivery is stale +// context even when the seat and branch have since been reused. +func holderAssignment(repo, branch, sid string) fleet.Rec { + slot := fleet.S(fleet.SessionRecord(sid), "slot") + if slot == "" { + return nil + } + a := fleet.ReadJSON(fleet.Path("assign", fleet.Safe(slot)+".json")) + if fleet.S(a, "repo") != repo || fleet.S(a, "branch") != branch || fleet.S(a, "slot") != slot { + return nil + } + if recipient := fleet.S(a, "delivered_to"); recipient != "" && recipient != sid { + return nil + } + return a +} + // WorkAttention is the set of work states a hub must decide something about. var WorkAttention = map[string]bool{"dead": true, "late": true, "undeclared": true, "abandoned": true, "failed": true, "unknown": true} diff --git a/cmd/fleet/internal/verbs/work_assignment_test.go b/cmd/fleet/internal/verbs/work_assignment_test.go new file mode 100644 index 00000000..20c491e2 --- /dev/null +++ b/cmd/fleet/internal/verbs/work_assignment_test.go @@ -0,0 +1,32 @@ +package verbs + +import ( + "github.com/itsHabib/workbench/cmd/fleet/internal/fleet" + "testing" +) + +func TestUndeclaredUsesHolderAssignment(t *testing.T) { + old := fleet.State + fleet.State = t.TempDir() + t.Cleanup(func() { fleet.State = old }) + write := func(p string, r fleet.Rec) { + t.Helper() + if err := fleet.WriteJSON(p, r); err != nil { + t.Fatal(err) + } + } + write(fleet.Path("sessions", "current.json"), fleet.Rec{"session": "current", "slot": "a-current", "last": fleet.Now()}) + write(fleet.KeyFile("leases", "repo:fixture:topic"), fleet.Rec{"key": "repo:fixture:topic", "session": "current", "at": fleet.Now()}) + write(fleet.Path("assign", "a-current.json"), fleet.Rec{"repo": "fixture", "branch": "topic", "slot": "a-current", "delivered_to": "current", "for": "lead:current"}) + write(fleet.Path("assign", "z-old.json"), fleet.Rec{"repo": "fixture", "branch": "topic", "slot": "z-old", "delivered_to": "old", "for": "lead:old"}) + rows := undeclaredRows(map[string]bool{}) + if len(rows) != 1 || fleet.S(rows[0], "for") != "lead:current" || fleet.S(rows[0], "slot") != "a-current" { + t.Fatalf("wrong assignment attached: %v", rows) + } + // A reused seat's record delivered to another session is not the holder's work. + write(fleet.Path("assign", "a-current.json"), fleet.Rec{"repo": "fixture", "branch": "topic", "slot": "a-current", "delivered_to": "other", "for": "lead:other"}) + rows = undeclaredRows(map[string]bool{}) + if len(rows) != 1 || rows[0]["for"] != nil { + t.Fatalf("foreign delivery attached: %v", rows) + } +} diff --git a/cmd/fleet/internal/watch/detach_windows.go b/cmd/fleet/internal/watch/detach_windows.go index b0a8e415..76c5c61d 100644 --- a/cmd/fleet/internal/watch/detach_windows.go +++ b/cmd/fleet/internal/watch/detach_windows.go @@ -18,7 +18,6 @@ import ( // that means "console application, no window"; it is documented as invalid combined // with DETACHED_PROCESS, so this replaces it rather than adding to it. func detach(cmd *exec.Cmd) { - const createNewProcessGroup = 0x00000200 const createNoWindow = 0x08000000 - cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: createNewProcessGroup | createNoWindow} + cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP | createNoWindow} } diff --git a/friction-log.md b/friction-log.md index ed91ba33..56b79322 100644 --- a/friction-log.md +++ b/friction-log.md @@ -372,3 +372,13 @@ Second occurrence of the `#214` entry above, one failure mode further in. - Status: fixed with real Git regression tests; the new tenant test failed on 0af6100. Mac Fleet race tests pass. Windows execution is delegated to the portability CI job; a visible-window check still needs the work machine's next real session start. + +### 2026-09-08 — Board accountability came from an older seat assignment + +- What I tried: fold Codex's review of #289 with a two-seat regression fixture. +- What happened: a branch-wide map collapsed both assignments by filename order, + showing lead:old and z-old against the current holder in a-current. +- Class: misleading-status. +- Smallest fix: read the holder's recorded slot, verify repository/branch/slot, and + reject an assignment delivered to another session. Unknown holder context stays empty. +- Status: fixed; regression failed on 925b785 before the change. From b16c52387fcaadf35a63065c36845ba3aa74e6ab Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 19:56:58 -0700 Subject: [PATCH 09/10] test(fleet): normalize Windows paths in assignment diagnostics --- cmd/fleet/internal/verbs/pool_identity_test.go | 2 +- cmd/fleet/internal/verbs/work_assignment_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/fleet/internal/verbs/pool_identity_test.go b/cmd/fleet/internal/verbs/pool_identity_test.go index 9be8cd16..0e1a6dc2 100644 --- a/cmd/fleet/internal/verbs/pool_identity_test.go +++ b/cmd/fleet/internal/verbs/pool_identity_test.go @@ -141,7 +141,7 @@ func TestAssignReportsHoldingWorktree(t *testing.T) { repo, a, _ := poolFixture(t) branch := fleet.BranchOf(repo) err := assignCheckout("seat-a", a, branch) - if err == nil || !strings.Contains(strings.ReplaceAll(err.Error(), "\\", "/"), strings.ReplaceAll(repo, "\\", "/")) { + if err == nil || !strings.Contains(fleet.NormCase(strings.ReplaceAll(err.Error(), "\\", "/")), fleet.NormCase(fleet.LongPath(repo))) { t.Fatalf("missing holding checkout %s: %v", repo, err) } if fleet.BranchOf(a) != "" { diff --git a/cmd/fleet/internal/verbs/work_assignment_test.go b/cmd/fleet/internal/verbs/work_assignment_test.go index 20c491e2..160f2e37 100644 --- a/cmd/fleet/internal/verbs/work_assignment_test.go +++ b/cmd/fleet/internal/verbs/work_assignment_test.go @@ -1,8 +1,9 @@ package verbs import ( - "github.com/itsHabib/workbench/cmd/fleet/internal/fleet" "testing" + + "github.com/itsHabib/workbench/cmd/fleet/internal/fleet" ) func TestUndeclaredUsesHolderAssignment(t *testing.T) { From aefbe14f75ecfe35e668eca4ff10886d7304ba7b Mon Sep 17 00:00:00 2001 From: Michael Habib Date: Tue, 8 Sep 2026 20:00:25 -0700 Subject: [PATCH 10/10] docs(fleet): record capped review residuals --- FOLLOWUPS.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/FOLLOWUPS.md b/FOLLOWUPS.md index d7db2769..859e395d 100644 --- a/FOLLOWUPS.md +++ b/FOLLOWUPS.md @@ -523,3 +523,22 @@ same bar workbench-mcp cleared. Evaluate then; not before. A future indexed reader could preserve longer windows within the same resource budget. Empty telemetry reason/out strings remain a compatibility deferral: current consumers normalize them identically to null/absent fields. + +## Fleet #289 — residual after the capped review rounds + +Code head: `b16c52387fcaadf35a63065c36845ba3aa74e6ab`. The Mac review used the +initial panel plus two fix rounds; do not start a fourth panel cycle for these nits. + +- **Diagnostic only: duplicate extra denies.** Copilot identified, and Claude confirmed, + that manually duplicated entries in settings.local.json can appear twice in the + retained-extra NOTE. `writeDenies` deduplicates the actual written permissions through + `denySet`, so enforcement is unchanged and the resulting file is normalized. A future + small change can deduplicate `extra` too, with a duplicate-input regression. Deferred + under the review cap rather than changing enforcement or claiming the finding vanished. +- **Repeated roles.map reads.** Tenant and label resolution independently scan bindings. + Consolidating one validated snapshot may simplify a later pool transaction change; + this PR does not claim atomicity against concurrent edits to all role bindings. + +No residual acceptance or merge authority is recorded here. The merge decision still +belongs to the operator's governed path. Windows visible-window acceptance and effective +hook migration remain separate from the green portability tests.