feat(hooks): fire SessionLaneChanged when a session's board lane changes

[rnoack1]

Problem / Motivation

Nothing server-side reacts when a chat session's tags change, so the kanban board
displays state but cannot trigger anything. There is no way to run an automation
when a session enters a lane — the motivating case being a close-out prompt when a
session is marked Done (tickets to close, follow-ups to open, branches to
delete).

HOOK_EVENTS carries five events — AgentSpawn, UserPromptSubmit,
PreToolUse, PostToolUse, Stop. All five are agent-turn lifecycle events, so
none can fire for a tag moved from the UI outside any turn.

Six riders travel with this feature — declared up front

Named here so they are visible without reading to the end; the reasoning and the
revert consequences are in Revert coupling below.

1. Every hook event's capability gate now resolves off the event loop. Cross-cutting,
   argued from a no-blocking-call-on-event-loop violation reachable from this
   event's dispatch. Symptom-level; the cause-level fix is named and out of scope.
2. Hand-edited "status" strings in tags.json are repaired to real bools at load.
   Cause-level rather than a rider in spirit — base readers (chat_tags.py:964,980)
   already treat the string "false" as truthy, so the delta filter this event needs
   would inherit that bug — but it does change board-lane classification for legacy and
   hand-edited tags independently of hooks, so it belongs on this list rather than
   being counted as part of the event.
3. Every script-hook run now files an invocation-outcome audit row.
   run_script_hook audited only the governance DECISION; the RESULT (ok, error
   with its exit code, timeout) lived solely in the in-memory hook.last_status,
   which no audit query can reach. One row builder feeds every completion path; the
   terminal ones write it INLINE, because deferring a cancellation does not ensure the
   deferred write is ever driven, and the singleton is warmed before the command runs
   so that inline write is never the blocking first touch. So this is a
   cross-cutting change: it applies to all five pre-existing
   events, not only SessionLaneChanged. It ships because the absence was filed as a
   security-class blocking finding requiring the record uniformly across every
   script-hook execution — so scoping it to this event alone is not available.
   It also makes hook availability depend on audit availability: if the audit log cannot
   be warmed, run_script_hook REFUSES the run (exit_code=2, last_status="blocked")
   rather than running it unrecorded. Exit 2 is the BLOCK signal on PreToolUse, so while
   audit is unavailable a broad matcher there denies every tool call it covers; on the other
   four events the hook silently does not run. Fail-closed is intended for the security gate,
   and for the rest it trades availability for auditability — the spec's failure-mode section
   carries the per-event table.
4. last_run and run_count are coerced to numbers when hooks.json is loaded.
   This is REQUIRED BY RIDER 5, not a pre-existing latent bug, and the two should be
   judged as one unit. Rider 5's _merge_run_bookkeeping compares these fields
   (ran.last_run >= live.last_run, ran.run_count > base_count); the base only ever
   ASSIGNS them, so a string from a hand-edited or older hooks.json raises no
   TypeError before this PR and the dispatch was never at risk. Measured at merge base
   e86469c1f1: 8 last_run occurrences, 7 assignments, 0 comparisons.
   _normalize_hook_number follows the file's existing _normalize_hook_timeout
   pattern.
5. ScriptHookStore now serves readers a committed snapshot and folds a finished
   run back into it. fire() was reworked around _committed_fire_targets,
   _merge_run_bookkeeping and run_and_publish: readers get list_all() off a
   deep-copied snapshot instead of the live dict, and a completed run's run_count /
   last_run are merged back as a DIFFERENCE under the mutex, off the event loop.
   This is the largest rider and the least visible: it is the read and bookkeeping
   path for ALL FIVE pre-existing events, including the security-critical
   PreToolUse gate, so an error in the fold would degrade every event silently
   rather than fail loudly. It is a fix to a PRE-EXISTING hazard rather than something
   this event introduces, and an earlier revision of this list wrongly framed it as
   feature-necessity: base fire() already iterates live hook objects and awaits
   asyncio.to_thread inside that loop, so the interleaving predates the lane path.
   The lane worker fires off the request path and so reaches it more readily, which is
   why the fix rides here, but the hazard is base's. The revert consequence is Revert coupling
   item 2 below, which this list previously left as the only place it was declared.
   TestPreExistingEventBookkeepingIsUnchanged pins the two invariants that carry
   the risk — the count is applied as a difference so a concurrent increment is never
   rolled back, and a slow older run cannot overwrite a newer outcome — and
   TestEveryPreExistingEventFoldsThroughTheSnapshot shows each of the five events
   traversing the path.
6. An unsaved hook form is now remembered across an unmount. The lane matcher
   offers a link to the board so an author with no status columns can create one, and
   following it unmounted the route and destroyed the name, command and matcher
   already typed — on the first-run path, every time. website/src/utils/hookDrafts.ts
   is a thin instance of the repo's existing createSlotDraftStore, alongside
   chatDrafts and goalDrafts, session-scoped and keyed by the form's identity
   (new, or the hook id when editing). Cross-cutting because it applies to EVERY
   hook form and not only a SessionLaneChanged one, so it is declared here rather
   than counted as part of the feature. The draft is dropped in the create/update
   mutation's onSuccess, NOT on the Save click: a rejected save leaves the form
   mounted with its edits on screen, so an early clear would lose them at the next
   navigation. HooksPage.draftSurvivesNavigation.test.tsx pins that rejected-save
   path separately from the plain-navigation one.

Why it matters

The only mechanism available today is polling GET /api/chat/slots on a timer and
diffing against a remembered snapshot. Three costs fall on every consumer:
reaction latency equals the poll interval; each one reimplements the same delta
computation; and one that forgets to baseline fires on every session already
carrying the tag. The board is the natural place to express "when this reaches
Done, do X", and it currently cannot.

What this buys, stated exactly: latency and a shared delta — NOT the removal of
reconciliation. Reaction stops being bounded by a poll interval, and the delta is
computed once here instead of in every consumer. It does not retire the polling
loop for the motivating subscriber: delivery is at-most-once and in-memory, dropped
on overflow and on gateway restart, so close-out automation doing irreversible work
still has to reconcile against the board — which the spec says in those words. A
subscriber that can tolerate a missed transition may drop its timer; one that cannot
gains latency and keeps the loop. Retiring it needs a durable queue with redelivery
semantics, which is a different change and is not attempted here.

What changed (motivation → approach → change)

Goal: let something react once a status tag is set, without adding a
subsystem.

Approach. Both tag writers already end in the same shape — mutate,
save_slot_off_loop, push_slots_update(), audit — and push_slots_update() is
a browser broadcast with no server-side consumer, so there was no existing seam to
subscribe to. Rather than add one, this reuses the script-hook mechanism already
present:

• Hook definitions live in one global user-authored store (hooks.json), not
  per-agent. That is what makes the event possible at all: a tag change has no
  agent, so a per-agent lookup would have had nothing to key on.
• Dispatching from an aiohttp handler with no agent turn is already a supported
  shape — api_hook_test does exactly that, and fire() builds its own default
  payload when handed none.
• Because the store is user-authored and already gated by
  capabilities.script_hooks (default off), this needs no new trust decision and
  widens no capability surface.

Considered and rejected: consuming push_slots_update() server-side (couples
automation to a render notification, and fires on non-tag changes); a generic
session-state-changed event (broader, and every extra field is a compatibility
commitment — a tag-scoped event can be widened later); reusing Stop (cannot see
a tag moved while no turn is running, which is the motivating case).

What was built:

• SessionLaneChanged added to HOOK_EVENTS; fire() stamps slot, added,
  removed.
• fire_session_lane_changed, a fire-and-forget wrapper, dispatched from the
  THREE session-level status-tag transitions: api_chat_slot_tags (PUT tags),
  api_chat_slot_drop (drag-drop), and the api_chat_tag_delete strip loop —
  deleting a status tag means every session holding it just left that lane, so
  omitting it would make a *removed:done* hook silently miss lane deletion.

Three decisions worth flagging for review, because they are the parts expensive to
reverse once a hook subscribes:

1. The payload is a DELTA, and only a delta. It carries slot/added/removed.
   A resulting-list-only payload would make every consumer persist its own prior
   snapshot to answer "was Done just added?" — the polling problem relocated rather
   than solved. An earlier revision ALSO sent a tags list holding the post-change
   set; it is dropped. Dispatch is off the request path, so the board can move again
   before the hook runs, meaning such a list could only answer "what did this
   transition land on" while being shaped like an answer to "what is true now". The
   spec had to spend a paragraph telling subscribers not to trust it, which is worse
   than not sending it: a subscriber needing current state re-reads the live store.
   Dropped now because the event has zero subscribers — removing a payload key later
   is breaking, and this is the free moment.
2. Informational only — a hook cannot veto. Exit code 2 blocks a PreToolUse
   call; this event ignores it. By the time it fires the write is persisted and
   the user has already performed the drag, so a veto would make the board
   unusable when a hook breaks rather than preventing anything. Dispatch is off
   the request path so a slow hook cannot delay the response either.
3. Status tags only. maybe_auto_tag writes non-status tags routinely (and
   deliberately never writes status ones), so firing on every tag would make the
   event chatty for the board-lane case that motivates it while adding nothing.

This is additive: with no hook registered, every writer behaves exactly as before.

Three cross-cutting changes in hooks.py, all shipping

THREE changes here alter behaviour for every hook event, and all ship: an
asyncio.to_thread offload of the capabilities.script_hooks gate, an
invocation-outcome audit row when a hook run finishes, and a rework of
ScriptHookStore so readers are served an immutable snapshot instead of the live
hook objects. The snapshot is the load-bearing one: list_all() returns a committed
copy rather than self._hooks.values(), every mutation republishes under the store
mutex, and a hook run republishes through run_and_publish so its status reaches
readers. That changes what a reader observes mid-write, so it is named here rather
than left to the diff. A third — an allowed
governance row per permitted run — IS shipped, and every governance write now goes
through the same off-loop audit seam: a cold sel() does synchronous trust-dir
creation and an HMAC key load, so emitting the row inline would stall the gateway for
each permitted hook. Routing it, rather than deleting it, is what keeps the permitted
decision auditable on its own without putting filesystem I/O on the event loop.

The offload, because resolving that gate walks profiles/ synchronously, so an
async caller resolving it inline stalls the event loop — a
no-blocking-call-on-event-loop violation reachable from this event's dispatch.

The audit row, because run_script_hook filed a row when governance DENIED a hook
and nothing when it ALLOWED one, so a permitted run left no trace. Its own sibling,
the skills-only path in fire(), already audits both arms with the same helper and
the same capabilities.script_hooks scope; this makes the command-hook path match
it, so the two outcomes filter as one queryable pair. This asymmetry is
pre-existing in main, not introduced here — _audit_governance_hook_decision
appears four times in main and this PR adds none of those call sites. It is fixed
here because this event's dispatch is what reaches the un-audited path.

It is ONE seam, not a hop per caller and not keyed on the event:
_script_hooks_capability_denied_async wraps the synchronous gate, and both async
sites in hooks.py await it. Scoping it to SessionLaneChanged was tried and
withdrawn — it put an equality branch in shared dispatch that every future event
would grow, while leaving the other events stalling anyway. What changes for a
pre-existing event: its capability lookup resolves on a worker thread instead of
the event loop, and a finished hook run files one invocation-outcome row recording
what it did. Volume is bounded by hook RUNS, not by UI
activity — the lane permit gate deliberately does not also record an allow, which
would have filed a row on every lane drag including the default state where
capabilities.script_hooks is off and no hook can run.

This remains a CALLER-side workaround. The cause-level remedy is non-blocking
resolution, or a cached fingerprint, in the owning module — and it is centralised
in one wrapper precisely so that change has one seam to delete rather than a hop at
every call site.

Also outside the event's own surface: validation.ALLOWED_HOOK_EVENTS gains the
event, so a hook can be registered through the create/update API rather than only
by hand-editing hooks.json. Three event allowlists now diverge intentionally and
a test pins all three memberships together with the reason for each, so a follow-up
cannot "fix" the divergence by syncing them.

3. The dispatch is bounded, and absorbs rather than sheds. Deltas go onto ONE
   bounded FIFO queue — _LANE_QUEUE_MAXSIZE (512) — drained by ONE worker, so an
   ordinary burst is absorbed instead of dropped. Each dispatch can spawn a
   hook subprocess, so an unbounded fire-and-forget scheduler was an fd/process
   exhaustion path; a single drainer bounds that real resource, and the queue depth
   bounds memory. Only an overflow past 512 is dropped, and that drop is audited
   (SEL outcome=rejected) — it means hooks are not draining at all, not that
   traffic is merely brisk. One queue and one worker make delivery totally ordered,
   which subsumes the per-session ordering this event actually promises: a session
   dragged out of a lane and back must not deliver "entered" and "left" in
   either order, because a close-out subscriber acting on the wrong one does something
   irreversible. An earlier revision sharded the queue 4 ways on a digest of the slot
   key so one wedged hook could not defer another session's fire; that is withdrawn.
   It bought cross-session latency isolation this event's own at-most-once contract
   already tolerates, and cost four queues, four workers and a hashing rule to
   document and test. A slow hook now delays later fires, bounded by the 1-300s hook
   timeout. Dispatch stays off the request path either way, so a slow or broken hook
   can neither veto nor delay the tag write.

   An earlier revision capped in-flight dispatches at 8 and dropped past the cap
   at scheduling time. That was replaced because routine shedding is a poor trade
   for an event whose subscribers act irreversibly; the constant it used no longer
   exists.

One persisted-data repair, named because it rewrites user state.
DashboardState.load_tags now coerces a non-bool status field to a real bool at
load and persists the repair, so tags.json can be rewritten on load. This is not
cosmetic, and the reason is this PR's own new readers rather than any pre-existing
disagreement: every reader of that field on the base tree already agrees on plain
truthiness (chat_auto_tag.py, and two sites in chat_tags.py, plus the frontend
tag list), and ["status"] is True appears zero times. The gap is that the
create/update API coerces with bool(...) on WRITE, so a value that never passed
through it — a hand-edited tags.json — can still hold a string, and "false" is
truthy. This PR adds new readers of the field on the lane-delta path, so a stringy
value would now decide whether a transition is a LANE transition at all.
Normalising once at the single load entry point is therefore the cause-level fix,
and it keeps every reader, old and new, on plain truthiness. The alternative — a
defensive is True at each reader — leaves the persisted state wrong and the next
reader exposed.

Revert coupling: TWO hunk groups a revert would take with it. Everything else here
belongs to this feature, so a revert of the feature is a revert of the feature and
needs no hunk-retention recipe. These two are not, and are called out so the
decision to couple them is explicit rather than discovered later:

1. The off-loop gate resolution and the invocation-outcome audit row change
   behaviour for EVERY hook event, not just this one. Reverting this PR returns every
   event's capability lookup to the event loop and stops recording what a hook run
   actually did. Both are argued above; both are cross-cutting.

2. The committed-snapshot freeze in _committed_fire_targets is cross-cutting for
   the same reason. fire() previously iterated the live hook dict, so a command
   belonging to a mutation that _atomic_mutation rolled back could still execute, with
   no undo — TestARolledBackHookNeverFires pins that. Reverting this PR reopens that
   window for all five existing events, not just this one.

Named deferral, not a claim of completeness. The governance_permits walk is
NOT fixed generally by this PR. It is offloaded only at the two call sites inside
hooks.py, because those are the two this event's dispatch traverses. The same
synchronous walk sits behind every other caller — 26 sites across src/ at the
time of writing, including dashboard/chat_runner.py on the async turn path. The
general fix belongs in governance_profiles (non-blocking resolution, or a cached
fingerprint) rather than a to_thread at each of 26 callers, and is deliberately
out of scope here. This is stated in the code at the offload site and in the spec.

Writer coverage is enumerated in the spec, including one known gap. "Status
tags changed" is not true of every site that assigns slot.tags: hydration,
restore and fork-copy paths assign it while no transition is happening, and firing
there would fire on process start. The spec now tables which sites fire and which
do not, with the reason for each. One is a genuine gap rather than a deliberate
exclusion: folder inheritance (validate_folder_tag_ids callers) can stamp a
status tag at filing time and does NOT fire, so a hook cannot use this event to
catch lane membership acquired by filing. That is stated in the spec as a known
gap rather than left for a subscriber to discover; instrumenting the filing path
belongs with that endpoint family, not bolted onto the tag writers.

Event name narrowed to SessionLaneChanged before merge. The name is the one
compatibility surface that cannot be corrected once a hook subscribes. The firing
contract is status-tags-only, so a tag-general name would over-promise — and it
would make the obvious later widening (fire on ALL tag changes) a BREAKING change
rather than an additive one, since every no-matcher subscriber would begin receiving
auto-tag noise from maybe_auto_tag. Under the lane-scoped name that widening is a
NEW event beside this one. The rationale is recorded in the spec so it is not undone.

Spec updated in this commit. docs/system-specs/modules/memory-skills-hooks.md
gains a SessionLaneChanged section documenting the payload keys, the
direction-tagged matcher grammar, the whole-string-glob wildcard requirement (a
bare done matches nothing), the lane-rename behaviour (name tokens follow a
rename, id tokens are stable), the informational-only contract, the dispatch cap,
and the allowlist divergence — so the first registered hook is not what freezes an
undocumented compatibility surface.
The event is deliberately absent from _VALID_HOOK_EVENTS, so it does not leak
into the generated kiro-cli agent config, which would reject an event it does not
know.

Tests

test/test_session_lane_changed_hook.py, 51 tests:

• The event is registered in HOOK_EVENTS, and is not in
  _VALID_HOOK_EVENTS (locks in that it cannot leak to the kiro-cli spec).
• PUT /tags fires once with added/removed/slot correct.
• POST /drop fires and reports the replaced lane in removed.
• A non-status tag change does not fire, while the write still lands.
• A hook that raises does not fail the response, and the tags still persist.
• The wrapper returns normally when the store's fire raises, having really
  dispatched.
• A None store is a no-op.

Full affected surface re-run green: 773 passed, 0 failed across
test_chat_tags, test_hooks, test_hooks_coverage, test_dashboard_hooks,
test_agent, test_api_kiro_hooks, test_fire_tool_hooks,
test_hook_validation_parity and the new file.

Negative control: removing the dispatch from the PUT writer turns exactly one
test red, and restoring it returns the diff byte-identically — so the new tests
can fail for the intended reason. One caveat stated plainly: the
non-status-tag test passes with and without the dispatch, as any absence
assertion does; it is meaningful only paired with the positive test.

Manual verification

Both writers are exercised end-to-end through the real aiohttp handlers in the new
tests. There is ONE user-visible change: the dashboard hooks page offers
SessionLaneChanged in its event picker, so the feature is reachable without
calling the API by hand. The picker's event list was a second hard-coded copy of
the backend allowlist, so a review round found the two had silently diverged and
the described user could not reach the feature at all; a test now asserts the two
lists are equal, so the drift fails the build instead of needing a reviewer.

Screenshot evidence

Captured from the real page through an isolated capture entry
(website/capture/session-lane-changed-hook.html), driven by
website/scripts/capture-session-lane-changed-hook.mjs. The harness asserts as well as
photographs: it exits non-zero unless the picker really offers six events including this
one, so a silent regression cannot produce a passing capture. Every hook shown is a
fixture -- no real session, name or token appears in either frame.

Related Issues

Part of #7663.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — the events reference page is the
  follow-up phase. The event picker is NO LONGER deferred: it ships here,
  because deferring it left the event registrable only through the API
  and the feature unreachable from the dashboard
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

Placeholder per the PR template — the CLA wording is to be supplied by OSPO and
has not been invented here.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 10477c1e39da45857b5f5be2e7822d7ae16360b6 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound, well-fenced feature — but it carries a rework of the hook engine's read path and a new fail-closed audit dependency for all five pre-existing events.

Watch

• Revert coupling is the real risk. Three riders (off-loop capability gate, audit-gate refusal, ScriptHookStore snapshot/fold rework) change behaviour for every existing event including the PreToolUse security gate; the PR itself concedes a feature revert "reopens that window for all five existing events." A regression in _merge_run_bookkeeping or the snapshot fold degrades events this feature never touched, and the only rollback takes the fixes with it. The cross-cutting hooks.py/sel.py changes are separable from the lane event by construction.
  Clears when: riders 1/3/5 land as their own PR (feature rebased on top), or a maintainer explicitly accepts the coupled revert in the PR thread.
• The audit gate makes hook availability depend on SEL writability, on an unverifiable mandate. _audit_gate_row probes per run; a cold/unwritable audit log now denies every tool call a broad PreToolUse matcher covers ("every tool call a matcher covers is denied", per the spec hunk) and silently disables informational hooks. Fail-closed is right for the security gate; extending it uniformly rests on "the absence was filed as a security-class blocking finding," which the diff cannot show.
  Clears when: that finding is linked in the PR, or the refusal is scoped to exit-code-honouring events with the others degrading to run-plus-recorded-gap.

Suggestions

• The per-run probe row (script_hook_audit_gate, outcome "ok") is a synthetic fact; writing a real "started" invocation row would prove writability and carry information instead of doubling row kinds.

[DESIGN-REVIEWED] 10477c1

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 10477c1e39da45857b5f5be2e7822d7ae16360b6 via the fork AI-review pipeline — 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.

First-Principles-Verdict: CONCERNS

The fail-closed audit gate on every script-hook run rests on an uncheckable "security-class blocking finding" — the verifiable gap justifies recording, not refusing.

Not justified as shipped

• Item 3 — rides along, inherited premise. The audit gap is real on base (outcomes lived only in hook.last_status; base run_script_hook audited decisions only), and that supports the best-effort outcome rows (item 4). But the escalation to refusing the run when audit is cold — exit 2, so a broad PreToolUse matcher denies every covered tool call, and the other events silently stop — is supported only by "the absence was filed as a security-class blocking finding requiring the record uniformly." No issue, link, or thread is given, so the requirement cannot be checked; it also buys a per-run gate row (script_hook_audit_gate, outcome ok) on every hook execution forever.
• Item 10 — the error notice on a failed tag create (TagManagerList.tsx, createTagMutation.error) is undeclared; harmless, noting for the record.

What this change ships

Inventory (10 items) — 8 justified

Intent: let an automation react when a session enters a board lane (motivating case: close-out on Done) — an ADDITION, with six declared riders.

1. New SessionLaneChanged hook event fires on board-lane transitions (queued, at-most-once, dashboard-only) — justified
2. Hooks page: lane event in the picker, column picker, dead-matcher warning, board deep-link — justified
3. Script-hook runs refuse to start when the audit log can't be written (exit 2) — rides along; inherited: the fail-closed escalation cites an unlinkable security finding
4. Every script-hook run now files SEL outcome rows (ok/error/timeout/cancelled) — rides along
5. Hook run status and fire targets reworked onto a committed snapshot for all five pre-existing events — rides along
6. Hand-edited "status" strings in tags.json repaired to real bools at load — rides along (verified: base chat_tags.py:964,980 use truthiness)
7. last_run/run_count coerced numeric at load — rides along (required by item 5; verified 0 comparisons on base)
8. Unsaved hook forms survive navigation — rides along (uses existing createSlotDraftStore, 7 sibling instances)
9. Capability-gate resolution moved off the event loop for all events — rides along (declared symptom-level, cause named out of scope)
10. Tag manager gains a create-as-board-column mode; failed creates now show an error notice — undeclared (the notice)

Watch

• Item 3's premise: "It ships because the absence was filed as a security-class blocking finding" — nothing pointable backs the refusal half, and its blast radius is every hook (and, via PreToolUse, tool availability) during any SEL outage. Clears when: the finding is linked (issue or review thread) so the uniform fail-closed requirement can be read, or the refusal is narrowed to best-effort.

Subtractions

• Delete the _audit_gate_row probe and the if not recorded: … exit_code=2 refusal branch in run_script_hook; keep the outcome rows, which already record the gap via (audit row not recorded) in last_error — unless the blocking finding can be pointed at.

[FIRST-PRINCIPLES-REVIEWED] 10477c1

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 10477c1e39da45857b5f5be2e7822d7ae16360b6 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 10477c1

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 10477c1e39da45857b5f5be2e7822d7ae16360b6 via the fork AI-review pipeline; updated in place on each push.

Review details

Both candidates rest on the same premise: that reading shared state "after the write lock is released" opens a race window. I traced the actual control flow to test that.

In the drop handler (api_chat_slot_drop) and the PUT handler (api_chat_slot_tags), the only statement between the async with _tags_write_lock(state): block exit and the _dispatch_lane_changed(...) call is state.push_slots_update() — which is synchronous (state.py:7747, uses a threading lock, no await). LoopBoundLock.__aexit__ (loop_lock.py:149) just calls the synchronous release(); awaiting a coroutine that never suspends does not yield to the event loop. So the awaiting task does not yield control between the in-lock mutation and the dispatch call.

Candidate 1: slot.tags is evaluated as an argument synchronously at the call site, with no yield since slot.tags = written_tags was set under the lock. It therefore equals written_tags on the applied path (the only path reached — refusals return early). No concurrent PUT can rebind it in a zero-yield window. Falsified — (b) requires a suspension point that does not exist.

Candidate 2: _dispatch_lane_changed computes tag_index = {t["id"]: t for t in state._tags}, status_before, status_after, and the status_before == status_after decision all before its first await (_lane_dispatch_allowed). The coroutine runs synchronously from entry until that await, and entry itself follows the lock release with no intervening yield. So state._tags is classified in the same synchronous step as the write; a concurrent api_chat_tag_update (which needs the lock, hence a yield) cannot intervene. Falsified — same missing suspension point.

No findings.

[OPUS-REVIEWED] 10477c1

[bolichen97]

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

• This PR is OVERLAPPING with PR #5933. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7669: KEEP. Independent, both worth landing, but they textually conflict in api_chat_tag_delete. Agree an order: if 5933 lands first, 7669's per-holder enqueue moves into the two-pass stripped list; if 7669 lands first, 5933's rewrite must carry the SessionLaneDelta accumulation forward or lane deletion silently stops firing. Files: src/kiro_crew/dashboard/chat_tags.py.
• This PR is OVERLAPPING with PR #7779. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7669: KEEP. Neither blocks the other and they do not conflict textually, but they must agree on two things: whether the agent self-tagging writer fires SessionLaneChanged (otherwise the event's documented writer table is wrong the day 7779 lands), and one convention for reading a tag's status flag. Files: src/kiro_crew/dashboard/session_directive_apply.py, src/kiro_crew/dashboard/chat_tags.py.
• This PR is OVERLAPPING with PR #7877. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7669: KEEP. Different goals with a real interaction worth naming once both exist: 7877 could eventually drop its poll loop in favour of this event, and its own writes will fire it. Files: src/kiro_crew/apps/builtins/chat_status_tags/store.py.
• This PR is OVERLAPPING with PR #7966. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7669: KEEP. The merged PR is a design document, not an implementation, so it supersedes nothing. Worth adding to this PR: update the RFC's implementation-prs / status and the index row's on-main column, and state the SessionLaneChanged-vs-SessionTagsChanged rename where the RFC's readers will see it. Files: docs/request-for-change/rfc-session-tag-change-event.md.

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

[jeeshofone]

Coordination from the #7779 side, following the maintainer's 2026-09-04 relationship audit (it asked our two PRs to agree on two things):

1. Should agent self-tagging fire SessionLaneChanged? Yes — once both land, a chat_tag set_state transition is a lane change and should emit the same event your human writers do, so your documented writer table stays true. Whichever PR lands second wires the applier call; if that is feat(board): governed agent self-tagging via a chat_tag session directive + [BOARD] context line #7779, the commit follows within a round of your merge.

2. One convention for reading a tag's status flag: these compose rather than conflict. Your load-time normalization makes the in-memory field a real boolean for display/event semantics — good, and feat(board): governed agent self-tagging via a chat_tag session directive + [BOARD] context line #7779 benefits from it. feat(board): governed agent self-tagging via a chat_tag session directive + [BOARD] context line #7779 additionally treats that field as display-only for the AGENT path: what an agent may do, and what counts as workflow state for agent-driven transitions, is sourced exclusively from a protected grants store (strict-boolean at its own parse boundary), because tags.json is agent-writable and a review round on feat(board): governed agent self-tagging via a chat_tag session directive + [BOARD] context line #7779 established that even a well-typed file value must not mint authority. With your normalization in, feat(board): governed agent self-tagging via a chat_tag session directive + [BOARD] context line #7779's remaining is True reads on the dict field are redundant-but-harmless defense in depth.

Mirrored on #7779. No changes requested to this PR.

[github-actions]

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of 10477c1e39da45857b5f5be2e7822d7ae16360b6 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have what I need: the full frontend diff (HooksPage form, event picker, lane matcher help/picker/warning/empty/error states, TagManagerList status-create mode, ChatSidebar deep link, all locale strings), the PR description, and confirmation that all six screenshots are fork-added binaries not materialized in this checkout. Reconciliation done; emitting the review.

UX-Verdict: CONCERNS

Careful, layered copy throughout — but nothing here has been seen: no blind read, no materialized screenshots, and the tag-manager handoff mode was never photographed.

Watch

• The status-create-mode exit button is a bare "Cancel" (ChatSidebar.tsx, status-create-mode-exit) beside the panel's X close: a user who clicks it expecting to close the panel silently drops into plain-tag mode, creates a non-column tag, and returns to a hook still reading "No board columns yet." Moderate frequency on the first-run path, silent wrong outcome. Rename to the outcome, e.g. "Create regular tags instead".
• "Create one in the tag manager" is a one-way trip: the draft protects the form, but nothing at the destination points back to the half-built hook — the user must remember to return to /hooks. Smallest fix: a "back to your hook" link in the panel while statusCreateMode is on.

Evidence gaps

• No blind read ran (fork lane): the picker entry "SessionLaneChanged — board column", table pill gloss, bridge sentence, "Advanced: write the matcher by hand", "Pick a column", the never-fires warning, empty/error/Retry states, and draft restore are all unread by a first-time user.
• All six committed screenshots are fork-only binary markers here — push the branch to this repository to materialize them and run the blind read.
• The tag-manager column-create mode (Zap icon, "New tag — board column" placeholder, bridge line + Cancel) appears in no screenshot, committed or claimed.
• The disabled "Pick a column" state (pickWouldNoOp) is shown nowhere.

Suggestions

• When pickWouldNoOp disables the picker, add one muted line saying why ("Your hand-written matcher can't be combined — edit it or clear it"); a silently dead control reads as broken.

[UX-REVIEWED] 10477c1

[bolichen97]

@rnoack1 Thanks for this. Audited at bed0642, so the head has moved since; the overlaps below are file and design level and should still hold. Four other open PRs touch the same seams, and I would like the coordination settled before any of them lands.

#8185 (@Pearcekieser) instruments the same three writers in src/kiro_crew/dashboard/chat_tags.py: it adds a tags_revision bump and a stricter rollback compare-and-set in api_chat_tag_delete, api_chat_slot_tags and api_chat_slot_drop, exactly where you insert _dispatch_lane_changed. These compose (its stricter rollback guard makes your "a rolled-back write never fires" claim safer), so this is a mechanical conflict only. Whichever lands first, the other rebases.

#7779 (@jeeshofone) adds a third lane writer, _apply_chat_tag, which holds no web.Request. Your dispatch gate is fail-closed on an absent app claim (== ""), so wiring agent transitions is a governance decision about what claim they present, not plumbing. Please agree that answer with @jeeshofone rather than defaulting it.

#7877 (@billygerhard) writes slot tags through its own store.py facade under tags_write_lock, and stamps request["app"] = "chat-status-tags" on its routes. Under your gate it can never fire this event by either path, and each attempt writes a denied audit row. It is also the event's most plausible first subscriber, so it is worth deciding together whether an app identity should ever be admitted.

#7843 (@aniruddhaadak80) reworks ScriptHook, ScriptHookResult and ScriptHookStore in src/kiro_crew/hooks.py to enforce PreToolUse deny on the subagent and task-runner paths, while you rewrite the same store's read path and fire() loop. Same security-critical surface from two ends: please agree an order.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.