Skip to content

feat: add crew members page with per-member DM threads - #6210

Merged
CrysisDeu merged 1 commit into
mainfrom
feat/crew-members-page-v1
Aug 28, 2026
Merged

feat: add crew members page with per-member DM threads#6210
CrysisDeu merged 1 commit into
mainfrom
feat/crew-members-page-v1

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

Crew members exist as configuration (crews in the manager, per-member spaces under $KIROCREW_HOME/members/<slug>/ since #2768), but there is no place to talk to one. Conversations with a crew member are ordinary sessions scattered through the sidebar: nothing is durable, nothing is pinned to the member's identity, and a member-initiated report has no fixed inbox.

Why it matters

The autonomous-crew direction makes "a named member you converse with" the primary object. Without a per-member thread, users cannot build a working relationship with a member: every conversation starts cold, an agent switch mid-thread silently changes who is speaking, and there is no surface where a member's escalations can reliably land.

What changed (motivation → approach → change)

Goal: one durable, pinned DM thread per crew member, on a dedicated page, with configuration writes staying in the existing crew manager (this page adds no second editor).

Approach: extend the existing per-member space (src/kiro_crew/members.py) with a dm.json binding, give member threads a reserved slot namespace (member-<slug>, mode="member"), and enforce the agent pin structurally rather than by convention.

What was built:

Backend

  • members.py: dm.json binding (atomic write, canonical slot_key enforced on both write and read — a tampered or stale binding reads as absent), member_slot_key(slug) derivation.
  • handlers/members.py: GET /api/members (roster: crew record fields + binding + O(1) running) and POST /api/members/{slug}/thread (idempotent get-or-create; the ONLY birthplace of member slots). Registry drift (crew renamed/deleted with a same-slug successor) fails closed with member_pin_mismatch — keyed off the binding, so it holds even when no live slot exists after a restart.
  • Constructor-central key reservation: state.get_or_create_slot refuses to mint a member-* key without mode="member", so every creation surface (send auto-create, slot-create, openai-compat, resume, channels) is covered by one gate instead of per-endpoint guards.
  • Pin enforcement at every slot.agent/slot.mode writer (enumerated and verified in review): send path, agent-switch endpoint, mode PATCH (member_mode_locked), openai-compat, fork (member mode never inherited), and the mid-turn EVENT_AGENT_SWITCHED veto (visible notice row + forced session reset).
  • Binding-first restore and resume: both persistence restore paths and the resume endpoint resolve a member key's agent+mode from dm.json before slot construction; transcript metadata (an operator-editable file) never overrides the pin. A member key without a binding is skipped on restore and refused on resume. Resume additionally requires the member key to resume only its own canonical history (structural check, no metadata trust) and hydrates from the single guard-validated metadata snapshot (no validate-A/hydrate-B window).

Frontend

  • /members route + sidebar entry (slotMode: 'member' claims these slots' unread counts; isChatPageSurface keeps them out of Sessions, now pinned by a test).
  • MembersPage: member list (live presence via the already-subscribed WS slots frames), the selected member's pinned thread hosted on the real ChatPane (new agentLocked prop suppresses the agent picker — every selection would 409 against the pin), and a read-only detail drawer (config summary, shared-memory disclosure, Edit jumps to /capabilities?tab=crews).
  • Identity is the exact crew name, never the slug: slugification is lossy, so a thread-open response naming a different member surfaces as a collision instead of silently mounting another crew's thread. The page POSTs the idempotent thread endpoint on every selection before mounting (the roster's bound flag is never trusted as mountable).
  • i18n across all 13 catalogs (plural keys registered), lucide icons only.

Docs: docs/system-specs/modules/learn-cron-dashboard.md gains the handlers/members.py entry.

Adversarial review (pre-push)

5 rounds × two blind cross-model seats (gpt-5.6-sol + claude-opus-5), fresh context per round, ~40 findings, ~25 fixed — including four structural refactors (constructor-central reservation; fail-closed replacing an in-endpoint re-pin that skipped the real switch endpoint's invariants; binding-first restore; structural resume validation). Round-5 seats verified the full writer enumeration and every creation surface. The three round-5 fixes (bindingless drift fail-closed, openai-compat 409 mapping, read_dm_binding totality) are pinned by tests but did not receive a further blind round (5-round cap).

Tests

  • test/test_members_dm_thread.py (46 tests): binding round-trip/canonicality/totality, roster contract, thread idempotency, foreign-slot refusal (legacy squatter simulated), colliding-slug first-bound-wins, registry-drift fail-closed with and without a live slot (mutation-checked: nothing moves, binding untouched), constructor reservation (unit + slot-create endpoint + send auto-create, with an existing-thread control), resume guards (foreign transcript 409 with no landmine slot, member-mode-on-ordinary-key 409, own-transcript 200 pinned from the binding against tampered metadata), persistence restore identity (binding wins over wrong metadata; bindingless key skipped), pin enforcement at send/switch/runner-event/openai with mutation verification, mode-PATCH lock + ordinary-slot control.
  • website/src/pages/members/MembersPage.test.tsx (11 tests): roster render/empty/error, thread open mounts the member slot with agentLocked + pin chip, bound rows still POST, collision surfaced not mounted, stale completion isolation, error retry, drawer content + toggle, edit-jump destinations (mutation-checked).
  • website/src/utils/channelOrigin.test.ts: isChatPageSurface member-exclusion pin.

Manual verification

Playwright capture harness (website/scripts/capture-members-page.mjs, self-checking: every frame asserts its state before writing). Frames below are the committed output.

Screenshots / video

Roster with live presence (dark):

roster dark

Pinned thread + detail drawer (dark) — pin chip, real chat stack, config summary, shared-memory disclosure:

thread drawer dark

390px narrow viewport:

mobile
mobile thread single-pane

Light theme parity:

thread light

Related Issues

no linked issue: built from the Crew Members design session (per-member DM threads); design doc and mockups live in the session workspace.

Known follow-ups (deliberately out of scope)

  • Legacy ordinary sessions whose keys happen to start with member- are skipped on restore (visible in History only), not migrated.
  • The thread endpoint's empty-agent repair is in-memory only (not persisted/broadcast until the next flush; idempotent per open).
  • The mid-turn veto notice is hardcoded English, consistent with the runner's other inline banners (backend strings have no catalog path).
  • While the always-POST repair is in flight, a previously opened thread renders optimistically from the cached key.
  • V2 items from the design: veto-window escalation cards, budget display, unread badges per row, per-member memory isolation (until then the drawer discloses shared memory).

Checklist

  • Tests added/updated for every behavior change
  • Docs updated in the same commit (learn-cron-dashboard.md)
  • Single commit, rebased onto current main
  • Screenshots committed and SHA-pinned

@CrysisDeu
CrysisDeu requested a review from a team August 27, 2026 03:52
@CrysisDeu
CrysisDeu requested a review from a team as a code owner August 27, 2026 03:52
@CrysisDeu
CrysisDeu requested a review from patrigao August 27, 2026 03:52
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of 7153cdaf3b2f4e8a44cca9d57604cb1c43b64736 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: CONCERNS

Solid roster→DM flow with honest states, but the thread header and composer speak the wrong vocabulary for a page whose whole premise is "who am I talking to."

Watch

  • "Pinned to radar" chip fails the cold read. The chip (pinned_to: "Pinned to {{name}}", 02-thread-drawer-dark.png) sits beside a header already saying "radar" and never says what is pinned — "pin" is the slot mechanism, not a user task. Shown on every thread open, every time → persistent confusion, no task failure. Fix: say the meaning, e.g. "Dedicated thread" or "Always {{name}}".
  • Composer says "Message Kiro Crew…" inside a member DM. All thread screenshots show the shared placeholder while the page teaches you're DMing radar — a first-time user can't confirm the recipient at the point of typing. Every message composed → recurring doubt. Smallest fix: let ChatPane accept a placeholder (the PR already threads agentLocked through) and pass "Message {{name}}…".
  • Crew vs. member drift in the same strings. empty_roster says "Create a crew … to see it here" and slug_collision says "Rename one of the crews" on a page titled Crew Members — a newcomer expects crews to contain members, so the empty state reads as a two-step setup that doesn't exist. First-run and error paths → comprehension friction. Pick one term ("member") in this page's copy.

Suggestions

  • Empty-roster CTA edit_in_crew_manager ("Edit in crew manager") promises editing when nothing exists yet — use "Open crew manager" or "Create a crew" for that button.
  • The "Details" toggle opens a panel headed "Configuration" — make them the same word so the click keeps its promise.
  • 03b-mobile-390-thread.png shows an icon-only Edit pencil the shipped code hides below md — recapture so the screenshots match what ships.

[UX-REVIEWED] 7153cda

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 7153cdaf3b2f4e8a44cca9d57604cb1c43b64736 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All mechanical checks are done. Writing the review.

First-Principles-Verdict: CONCERNS

Two files are reformatted wholesale with zero behavioral change riding in a feature commit, and the resume path rebuilds hydration the new thread endpoint already owns.

What this change ships

Intent: give each crew member one durable, pinned DM thread on a dedicated page — an ADDITION.

  1. New "Crew Members" sidebar entry + /members page (roster, thread, detail drawer) — justified
  2. GET /api/members roster endpoint — justified
  3. POST /api/members/{slug}/thread idempotent thread creator — justified
  4. Per-member binding file persisted under the agent-unreachable trust/ subtree — justified (named boundary)
  5. member-* slot keys refused at the single slot constructor — justified, cause-level
  6. Agent switch/mode change on a member thread refused at every writer, incl. mid-turn veto banner — justified
  7. Restart restore + History resume re-pin from the binding; bindingless member keys skipped — justified, but resume half is oversized
  8. Forking a member thread yields an ordinary chat — justified
  9. Agent picker hidden inside member threads (agentLocked) — justified
  10. Five files whole-file black-reformatted + pruned from .github/black-baseline.txt — rides along, undeclared

Watch

  • Undeclared reformat rider. 5 baseline files are reformatted in the feature commit; 2 of them (issue_radar/tests/test_members.py, test/test_dashboard_chat_handlers_coverage.py) carry no behavioral change at all. AGENTS.md: formatting a baseline file is "welcome but optional — do it in its own commit." Zero option costs nothing; the description never mentions it.
  • Spec written in this commit contradicts the diff. learn-cron-dashboard.md says the roster reads "bound/slot_key from the member dir's dm.json", while members.py states bindings live "under the keystone-gated trust/ subtree, NOT inside the member's own directory" (trust/member-bindings/<slug>.json), and the handler never emits a bound field ("bound itself is not exposed"). Same drift in write_dm_binding's docstring ("No fsync, deliberately") versus the fsync=True call below it.
  • Resume-path member machinery duplicates the thread endpoint. api_chat_slot_resume gains ~180 lines (early guard, late TOCTOU barrier, member_resume_conflict, metadata-absorb dance) to hydrate a member thread from History — the same binding-first get-or-create + rehydrate_slot_from_history_async(adopt_closed=True) that POST /api/members/{slug}/thread (this PR) already performs idempotently. The page itself adds 0 resume callers (it always POSTs the thread endpoint). A flat 409 on member keys pointing at the members surface removes the same harm with a fraction of the surface; the mirror-case refusal (member transcript on an ordinary key) is the only part resume genuinely needs.

Subtractions

  • Delete DM_FILE_NAME = "dm.json" (src/kiro_crew/members.py:56) — grep DM_FILE_NAME across src/: 1 definition, 0 consumers; the binding path is built from f"{slug}.json", and the constant's comment describes a location the code abandoned.
  • Replace the member-resume hydration block in api_chat_slot_resume (late barrier + member_resume_conflict + binding-first slot creation) with a refusal routing to the thread endpoint, which already owns binding-first reopen.
  • Drop the second member_thread_agent_pinned 409 in openai_compat.api_completions (the block before slot.agent = agent): the identical condition already refused at the top of the if slot_id: branch, and a slot created without slot_id can never carry mode="member".

[FIRST-PRINCIPLES-REVIEWED] 7153cda

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 7153cdaf3b2f4e8a44cca9d57604cb1c43b64736 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound feature, but the pin invariant is hand-replicated at every surface, and the lossy slug key manufactures most of the failure modes the PR then defends.

Watch

  • The pin holds only where a guard was remembered. Identity enforcement is duplicated across ~9 sites (send, openai-compat, resume early+late, agent-switch, mode-lock, runner veto, restore, constructor), each conditioned locally on mode == "member" — fail-open by omission. Any future dispatch surface that reaches a slot (a channel adapter, a new API) silently bypasses the pin; the registry-drift + binding-drift blocks in api_chat and api_completions are already verbatim copies.
  • Retroactive namespace reservation with no grandfather. The constructor refuses any member-* key without mode="member", restore skips ("leaving it unpublished") and resume 409s a member-prefixed history key with no dm.json. A pre-existing user session named member-… becomes permanently un-resumable after upgrade, with a misleading member_thread_agent_pinned error and no rename/migration escape.
  • Slug collisions make a crew un-DMable by design ("first name wins", config order): crew B's roster row opens a thread pinned to crew A, in a feature whose stated value is identity; drift cases dead-end in manual-resolution 409s.

Suggestions

  • Hoist the pre-dispatch pin validation (registry + binding drift) into one helper at the shared _run_chat/slot-dispatch boundary so both HTTP writers — and any future one — inherit it instead of copying it.
  • Consider a collision-free binding identifier (exact crew name, filename-escaped, or a minted id) — the entire "colliding slug hands the transcript to a successor" defense apparatus exists only because the key derivation is lossy.

[DESIGN-REVIEWED] 7153cda

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 7153cdaf3b2f4e8a44cca9d57604cb1c43b64736 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/dashboard/handlers/members.py:38 -- function-local import kiro_crew.dashboard.handlers as _pkg lacks the required circular-import explanation -> Fix: add a # circular import comment explaining the package re-export cycle.
[GPT-REVIEWED] 7153cda

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 7153cdaf3b2f4e8a44cca9d57604cb1c43b64736: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 7153cdaf3b2f4e8a44cca9d57604cb1c43b64736 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 7153cda

Verdict parsed from the review's SHA-scoped output markers for commit 7153cdaf3b2f4e8a44cca9d57604cb1c43b64736.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 7153cdaf3b2f4e8a44cca9d57604cb1c43b64736: <one-sentence reason>

@CrysisDeu
CrysisDeu force-pushed the feat/crew-members-page-v1 branch from c155bb0 to 502a746 Compare August 27, 2026 04:21
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 27, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/crew-members-page-v1 branch from 502a746 to fe3dd4d Compare August 27, 2026 04:55
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 27, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

UX disposition (fixes landed in fe3dd4d: single-pane narrow layout + back button, drawer overlays and starts closed below md, empty-state CTA to the crew manager, header Edit now uses the full 'Edit in crew manager' label, narrow capture now exercises the selected state — see 03b frame):

  • Noun unification (crew vs member): follow-up. Renaming the noun touches 13 catalogs and the crew-manager surface; doing it inside this PR would couple an unrelated copy migration to the feature. Tracked as a follow-up in the PR body.
  • Composer placeholder ('Message Kiro Crew…'): follow-up. The placeholder is ChatPane-owned copy shared by every surface; threading the member name through it is a ChatPane API change beyond the agentLocked seam this PR adds.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Design disposition (fix landed in fe3dd4d: GET /api/members now serializes an explicit four-field allowlist — the dataclass spread is gone, pinned by test):

  • Centralize the pin as a guarded slot mutation: accepted-and-deferred. The creation chokepoint exists (constructor reservation); a mode-gated agent setter is the right end state but converts 8 call sites and the runner's veto-with-notice exception in one PR. The PR body discloses the enumerated-writer risk; the setter is listed as a follow-up.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

First-principles disposition (subtractions landed in fe3dd4d: bound and top-level default_agent dropped from the roster response, created dropped from the thread response — the explicit allowlist replaced the dataclass spread):

  • Black reformat hunks ride the feature commit: accepted. The 6 baseline files became black-clean under this branch's toolchain and the baseline checker fails on any stale entry, so the prune must ride whichever PR trips it; this workflow keeps a single squashed commit, and the hunks are formatting-only (verified no semantic diff). Declared in the PR body rather than split.
  • Ninth slot.agent writer fails open: same disposition as the design lane — deferred to the centralized setter follow-up, disclosed in the PR body.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 27, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/crew-members-page-v1 branch from fe3dd4d to 072d02b Compare August 27, 2026 05:25
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 27, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

First-principles disposition (fixes landed in 072d02b: NOTES.md deleted from the commit — the blocker; pinned_agent/resolved_member dropped from all three 409 bodies):

  • Per-endpoint pre-check ahead of the constructor gate (chat_handlers slot-create path): deliberate, kept. The constructor's ValueError carries no machine-readable code; the pre-check exists to return member_slot_reserved per the repo's error-code contract (a new non-2xx JSON body MUST carry code). Mapping the ValueError generically would ship an un-coded 409.
  • Reformat-only hunks + black-baseline prune ride the feature commit: accepted, previously dispositioned — the baseline checker fails on stale entries the moment the branch's toolchain formats those files clean, and this workflow keeps one squashed commit; declared in the PR body.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT non-blocking findings disposition (blockers fixed in 072d02b: registry-drift guard on member sends returns member_pin_mismatch before dispatch, pinned by tests; function-local members_mod import hoisted to module scope in chat_persistence — the _sel() late-binding import stays, it is the annotated test-monkeypatch seam):

  • useQuery/useMutation for roster + thread open: follow-up, unchanged from the prior head's disposition — the page's fetch semantics (every selection POSTs, errors retryable per member) are pinned by tests; migrating to React Query is a mechanical refactor tracked in the PR body's follow-ups.

@github-actions github-actions Bot removed the readiness: checking Automated validation is still running label Aug 27, 2026
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 27, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/crew-members-page-v1 branch from 217c330 to fdbb886 Compare August 27, 2026 17:12
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision labels Aug 27, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the single blocker from the fdbb886 round is fixed in 6db7a54: POST /api/members/{slug}/thread now rehydrates a dormant thread's canonical transcript (via rehydrate_slot_from_history_async, dm.json-derived identity, off-loop reads, adopt_closed=True since this endpoint is the deliberate reopen path) BEFORE falling back to a bare slot create — reopening after a gateway restart or a ✕-close carries the prior DM context instead of an empty pane. Pinned by a test that plants a transcript with no live slot and asserts the reopened slot contains the prior messages.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the single blocker from the 82e2b06 round is fixed in b253873: write_dm_binding now passes fsync=True to atomic_write. The binding is the thread's durability anchor (the orphan-history guard refuses to rebind a slug whose binding vanished while its transcript survives), so it must be at least as durable as the transcript it attributes — a power loss after a transcript flush can no longer strand the thread behind member_binding_missing.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the b253873 round is fixed in eeb4f89: the member-thread validation now runs EARLY, before _unhide_folder and clear_closed — a resume the guard is going to 409 no longer leaves persistent side effects (a closed member thread silently reopened, its folder unhidden). The late guard stays in place unchanged as the authoritative TOCTOU barrier immediately before slot creation; the early twin only keeps rejected resumes side-effect-free. Pinned by test_rejected_member_resume_leaves_closed_flag_intact (member key with no binding + closed transcript -> 409, closed flag survives, no slot created).

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the eeb4f89 round is fixed in 7b45ada: the member-slot send guard now validates the on-disk binding alongside the existing registry-drift check. A live slot whose binding was deleted or corrupted while the tab stayed open refuses the send with a coded 409 (member_binding_missing) instead of persisting history that restore and thread-open would both refuse — the user learns at write time, not after the transcript is stranded. Same rare-send thread-IO budget as the registry check; ordinary sends untouched. Pinned by test_send_path_fails_closed_on_binding_drift, and _member_slot in the test helpers now writes the binding first, mirroring how real threads are born.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the 7b45ada round is fixed in 2adf711: the OpenAI-compat send path now mirrors chat_send's binding-drift guard. A completion targeting a live member slot whose dm.json was deleted or corrupted refuses with a coded 409 (member_binding_missing, both OpenAI error.code and top-level code shapes) instead of dispatching and persisting a transcript that restore skips and thread-open refuses. Note the exposure is narrow by construction — existing member slots are only addressable via body["id"] (the ephemeral else-branch mints oai-* keys and the constructor reservation blocks member-* creation) — and the guard sits on exactly that path, after the registry-drift check it mirrors. Pinned by test_completions_fail_closed_on_binding_drift.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the 2adf711 round is fixed in 7a19ded (a regression my previous fix introduced — correctly caught). A legitimate CLOSED member-thread resume passed the new early guard, clear_closed durably reopened the thread, and the late identity barrier then compared its post-clear snapshot against the PRE-clear baseline — a guaranteed self-409 issued after the reopen already landed. The fix absorbs exactly the self-inflicted mutation into the baseline: closed/closed_at are dropped from the LOCAL dict (the precise keys clear_closed pops) rather than re-reading the file, so every drift the barrier exists for — delete/recreate, concurrent edits, anything not these two keys — still mismatches and still refuses. A no-op'd compare-and-clear (thread re-closed mid-resume) leaves closed in the post-snapshot, which now mismatches the baseline and refuses: fail-closed, never fail-open. Pinned by test_resume_of_a_closed_member_thread_succeeds (closed member thread + valid binding -> 200, flag cleared, slot pinned).

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — both blockers from the 7a19ded round are fixed in 21338a9:

  1. Switch-veto SEL audit — the _run_chat agent-switch veto (the one pin enforcement site the HTTP guards cannot reach) now emits a denied log_api_access event into the immutable SEL chain before terminating the stream, matching every other member-pin refusal.
  2. Binding moved to the trust/ subtreedm.json no longer lives inside the agent-writable member directory. Both read and write paths now use dm_binding_path(slug) = trust/member-bindings/<slug>.json: trust/ is already whole-directory gated by the sensitive-path floor (like the SEL HMAC key and Spec Builder's decision record), so a prompt-injected file write can no longer re-point a thread's identity at a colliding crew. Directory modes are tightened owner-only best-effort to match the rest of the subtree; the gateway opens the path directly (keystone-writer pattern), so nothing legitimate breaks. No migration needed — the feature is unshipped, and a totality-contract read of the old location simply reads as unbound and re-creates.

Pinned by test_binding_lives_inside_the_trust_subtree (asserts trust in the path parts AND that no dm.json lands in the member dir); the corrupt/malformed/UTF-8/non-canonical totality tests all now exercise the trust-rooted path.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the 21338a9 round is fixed in 2185c0b, and the fix is a SWEEP, not the one site: every member-pin permission denial across the PR now emits a denied SEL event (source=member_pin), so this class is closed rather than whack-a-moled. Covered: all 4 denial sites in openai_compat (early pin — the flagged one — plus registry drift, binding drift, and the ephemeral-branch pin), the chat_send registry/binding drift refusals, all 5 resume-path guards (early pin, early mode-mismatch, late pin barrier, metadata-drift conflict, late mode-mismatch), and the 3 thread-open endpoint refusals (binding→non-owner, orphan history, live-slot registry drift). The chat_send pin itself already audited via _emit_agent_assignment (a SEL agent_assignment event) and the _run_chat switch veto gained its event last round — with this sweep every member-pin refusal in the PR lands in the immutable chain.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the 2185c0b round is fixed in 512fc75: the constructor's member-key reservation now casefolds before the prefix check, so a mixed-case squatter (Member-radar) is refused like the lowercase form — on a case-insensitive filesystem (Windows, default macOS) its transcript filename would alias the pinned thread's history. The reservation is the single choke point every creation surface funnels through, so folding it here closes the alias for send auto-create, slot-create, openai-compat, resume, and channels at once; canonical member keys are always lowercase (built from a validated lowercase slug), so no legitimate caller changes behavior. Pinned by test_reservation_is_case_insensitive (three case variants refused, canonical key + mode=member still admitted).

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the 86f9616 round is fixed in cf91f87 (a correct consequence-catch of the previous round's casefolded constructor reservation): all four NAME-based member-prefix guards in chat_handlers now casefold to match — the send auto-create refusal, the resume app-isolation 404, and both resume pin guards (early + late barrier). A mixed-case key like Member-radar now dies as a clean 409 at the guards instead of slipping past case-sensitive checks into the constructor, whose ValueError the resume path does not catch (HTTP 500). Slug extraction deliberately keeps the ORIGINAL bytes: an uppercase slug reads as unbound (slugs are validated lowercase), so every folded match fails closed. The openai-compat path already mapped the constructor refusal to a 409 (member_slot_reserved) and needed no change; live-slot slot.key checks are unaffected because member-mode slots only ever carry canonical lowercase keys. Pinned by test_mixed_case_member_resume_is_refused_not_500.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the cf91f87 round is fixed in 55a9f5c: the header Edit button is now hidden md:inline-flex, so a narrow viewport with a selected member shows exactly two peer actions (Back + Details) instead of three that can clip or wrap. No capability is lost — the Details drawer carries the same 'Edit in crew manager' jump one tap away, and the unit test already pins that BOTH affordances navigate to /capabilities?tab=crews. The mobile self-verification scenario's assertion is flipped to match the intended state (edit hidden below md, like the pin chip).

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the 55a9f5c round is fixed in 2eef6ab: the agent-switch veto branch now sets _produced_visible_output = True before terminating the stream. Without it the empty-response recovery saw a turn with no visible output and silently requeued the prompt — replaying any non-idempotent tool calls that had already completed before the switch event arrived. The veto notice IS visible output, and the completed side effects are precisely what must not run twice. Pinned by a new assertion in test_mid_turn_agent_switch_is_vetoed_on_member_threads: slot._empty_response_retries == 0 after the vetoed turn.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

GPT disposition — the blocker from the 2eef6ab round is fixed in 7153cda: _live_slot_resume_response is re-run immediately after the late binding await — the one suspension point between the earlier ownership re-checks and the publish. A concurrent resume that publishes the slot during that await is now seen: the loser answers with the live slot instead of get_or_create-ing the existing slot and hydrating the disk transcript onto it a second time. After this re-check every remaining step to the publish is synchronous, so the window is closed rather than narrowed. Pinned by test_concurrent_member_resume_does_not_duplicate_history (a patched binding read publishes the slot mid-await; the racing resume returns 200 via the live-slot path and the transcript appears exactly once).

@bolichen97

Copy link
Copy Markdown
Collaborator

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

  • PR #2842 is PARTIALLY_COVERED relative to this PR. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #2842: REBASE. One of five parts landed incidentally; rebase onto it rather than closing, since the entire authorization fix is still missing. Files: src/kiro_crew/dashboard/chat_handlers.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants