feat(dashboard): let the session-card PR/issue chips be switched off

[chenmingwei23]

1. What is the problem?

Every session card in the chat sidebar renders a chip strip for each pull
request, merge request and issue URL mentioned anywhere in that session's
transcript, and there is no setting, no per-folder option and no config key that
turns it off. _ChatSlot.to_dict() always serializes source_links and
source_links_total, and SessionSourceChips renders whenever that payload is
non-empty. Nothing between the two consults config.

The chips also drive a periodic credentialed refresh: the owner websocket loop
calls DashboardState.source_link_urls() every TTL and schedules a check-status
round that reaches the provider CLI.

2. Why this issue matters to the user

A mention is not a workstream. The extractor accepts any /pull/,
/merge_requests/, /issues/ or /browse/ URL it finds, so a session that
merely read or quoted someone else's pull request gets a chip that reads as
ownership.

The sidebar is the densest surface in the app and the strip is a whole extra row
per card, while link_previews, session_grid, auto_open_git_panel and the
rest of the chat display preferences are all already switchable. Those numbers
are also on screen for the entire session, so pull-request and issue numbers
from unrelated work land in every screen share, screenshot and bug report.

And it is not free: a user who does not want the chips is still paying for the
background provider calls that keep their status fresh.

3. How our fix solves it

New dashboard.session_card_source_links, default true, so an install that
never mentions the key behaves exactly as it does today. Both halves of the
feature are gated, because gating only the payload would leave the provider
polling for chips nobody renders:

Payload. to_dict reads the switch where it uses it and skips
_pr_source_links() entirely rather than paying for the transcript scan and then
emptying two fields. No flag is threaded through the serializers: the read is an
in-memory snapshot lookup, so there is no per-slot cost to amortize, and
publication happens on the loop, so a synchronous slot loop cannot observe a flip
mid-push and emit a mixed payload. (An earlier revision did thread a flag from
serialize_slots; the First Principles lane correctly pointed out that its
justification -- a per-slot config read -- stopped existing once the getter became
cache-only.)

Refresh. source_link_urls() and source_link_urls_for_slot() return
nothing while the switch is off, which stops both the periodic owner-WS round and
the turn-boundary refresh before they reach the provider.

The "+N" overflow endpoint is deliberately NOT gated: its only caller is the pill,
which exists only while the strip renders, and the write pushes fresh slots so the
pill goes at once. An app token that owns the slot could ask directly, but it can
already read that slot's messages -- these URLs are extracted FROM those messages, so
a gate there would withhold nothing it does not already have. (An earlier revision did
gate it; the First Principles lane counted the consumers.)

Where the value is read from. Not from config on the loop. The switch rides
the snapshot ensure_gitlab_hosts_loaded() already refreshes in a worker thread
for the self-managed GitLab and Jira allowlists: same config read, same TTL, same
lock, same generation counter. That is the mechanism this file's own
test_gitlab_allowlist_never_reads_config_on_the_event_loop exists to protect,
and the reason is identical -- the value is consumed by synchronous slot
serialization that runs on the sole event loop for every slot on every push,
while KiroCrewConfig.load() stats, reads, parses and validates config files.
Two things worth stating plainly:

• A click is PUBLISHED, not polled. PUT /api/dashboard/config writes the value,
  then installs it in the snapshot and pushes the slots, so the chips appear or
  disappear on the next frame. That publish takes the same lock
  ensure_gitlab_hosts_loaded holds across its threaded load, which is what keeps
  the write and the poll ordered: a poll already in flight holds a pre-write reading,
  and the lock makes the write land last. The 30s TTL refresh remains the backstop that picks
  up an edit made outside the dashboard (kirocrew config set, a hand edit).
  Raised by the UX lane, which was right that leaving it to the poll made an
  instant-looking switch sit inert for up to half a minute.
• The snapshot starts True where the allowlists start empty. The asymmetry is
  deliberate and commented: an unknown host must fail CLOSED, but the chip strip
  predates this switch, so a cold snapshot has to fail OPEN or every install would
  render no chips until the first refresh landed. An unreadable config is treated
  the same way.

One shared fix came with it. ChatPanel's dashboard mutation rebuilt the WHOLE
config from its own query cache on every toggle (mutate({ ...dashCfg, ...patch }))
and PUT all of it. Because the handler applies whichever keys are present, that
wrote every other setting back at its cached value -- so with two dashboards open,
flipping any switch restored the panel to the state that tab last read. It now
sends only the changed keys, which is what BrowserPanel's own dashboard mutation
already does, with the optimistic cache write merged instead of replaced. Found by
the GPT lane on this PR's new toggle; the window predates it and covered all
thirteen switches on the page.

No renderer change was needed: the call site in ChatSidebar.tsx already guards
on s.source_links.length > 0, so an empty strip draws no row.

The toggle lands in Settings -> Chat beside Link Previews and Auto-Open Git, as
the issue proposed, with the string in all 12 catalogs, the regenerated
settingsRegistry.gen.ts entry (so settings search finds it) and the regenerated
config-baseline.json.

What this does not fix

Point 1 of the issue -- a chip for a pull request the session merely QUOTED reads as
ownership -- is only answered here for someone who turns the strip off. The cause is
_ChatSlot._pr_source_links, which accepts every /pull/, /merge_requests/,
/issues/ and /browse/ URL in a transcript with no ownership signal at all, and
this PR does not touch it. Default-true therefore means every existing install keeps
that mis-attribution. So: mechanism-level for the extra row and the credentialed
polling, symptom-level for the mis-attribution. Narrowing the extractor is a
behaviour change to what a chip MEANS and wants its own decision, not a rider here.
Raised by the First Principles lane.

Out of scope, as filed: the in-session Resources / Changes side panel. Opening a
pull request from inside the session it belongs to is a different act from
advertising it on the card.

Screenshots

The evidence is a comparison, because the change is a subtraction: a lone shot of
a sidebar without chips is indistinguishable from a session that never mentioned
a pull request.

The strip as it ships -- a session that only quoted someone else's PR carries its
number, and a five-link session shows three chips plus +2:

The new row in Settings -> Chat, on the untouched default:

The same row after clicking it off, read back from the server:

And the same three cards with the switch off -- no strip, no +2, the row
reclaimed:

Captured by the committed harness website/scripts/capture-session-card-source-links.mjs,
which runs the real built SPA behind the shared loopback static server with every
/api/** call answered from fixtures (no gateway, no dashboard token, no provider
CLI). The sequence is driven by the real control: clicking the toggle issues the
same PUT /api/dashboard/config the dashboard sends, and the fixture then applies
the same rule the server applies, so the fourth frame photographs the payload
shape production pushes rather than a hand-blanked one. The harness asserts the
chip is present in frame 1, that the PUT carried false, that the switch reads
aria-checked=false, and that no chip renders in frame 4 -- it fails rather than
saving a misleading frame. The backend half is pinned by tests, not by a still
image.

4. What tests we did

New test/test_session_card_source_links_knob.py, 22 tests, every assertion
mutation-verified red against the un-gated code:

• Snapshot read: a cold snapshot means on; the getter reflects the snapshot; the
  getter never reaches KiroCrewConfig.load() or the off-loop loader (both
  patched to fail the test if called) -- the companion to
  test_gitlab_allowlist_never_reads_config_on_the_event_loop; a flip bumps the
  shared generation so open tabs get pushed, and republishing the same value does
  not; an unreadable config loads as on while the hosts in the same tuple stay
  closed.
• Payload: default serializes the strip; off empties both fields without dropping
  the keys; off does not call _pr_source_links() at all (patched to raise).
• serialize_slots: off strips every slot, the switch is resolved exactly once
  for a five-slot push (counting fake), and a lone serialize_slot resolves it
  itself.
• Refresh feed: both URL feeds are empty while off, and
  refresh_slot_source_status never reaches request_check_refresh_now.
• Expand endpoint: empty payload while off, and an unknown slot still 404s (gone
  and switched-off are different answers).
• Config surface: default true, save/load round trip, non-bool fallback, the
  generated JSON schema carries label + help, and all three endpoint wiring points
  -- PUT allowlist, validation branch (invalid_session_card_source_links), GET
  echo.

Mutation verification: reverting the to_dict gate reds 4 tests; removing either
URL-feed gate reds the refresh tests; making serialize_slots defer the read to
each slot reds the read-once test; removing the endpoint gate reds the expand
test.

Eight assertions in SettingsChatPanelCoverage.test.tsx pinned the old
full-object PUT body (one was even named "keeping siblings"). They now assert the
changed key alone: the REQUIREMENT -- do not lose sibling settings -- is unchanged
and is met by the handler, which writes only the keys in the body; the assertions
had encoded the mechanism. 68/68 still pass, and the pre-existing
capture-verbosity-levels.mjs harness -- whose whole point is that the value
re-read after the write is the one just chosen -- still passes end to end.

The row-framing geometry my screenshot harness needed already existed in that
verbosity harness, and jscpd runs at a 0% duplication threshold, so the second
copy failed Frontend Lint. Extracted to scripts/lib/settings-row-shot.mjs and
both harnesses now call it; npx jscpd . is back to one clone, the pre-existing
capture-grid-divider-align / capture-sidebar-dragbar pair, which measures 0%
and passes.

Also green: test_source_providers.py (the two harness fakes updated for the
widened loader return), test_dashboard_source_links_expand.py and
test_dashboard_state_ws.py unchanged -- which is the
default-behaviour-identical check -- plus test_auto_open_git_panel_config.py
and test_link_previews_config.py. tsc --noEmit clean; vitest run on
catalogParity / duplicateKeys / deadKeys (107) and
SettingsChatPanelCoverage (68); check-i18n-keys, check-dnt-catalogs and
check-i18n-strings clean; isort, flake8 and the black gate clean. mypy on the
touched modules reports only 2 pre-existing errors in transcribe.py, which this
diff does not touch.

Two test_source_providers.py tests (test_local_token_uses_configured_owner_subject,
test_local_token_carries_embed_parent_port_claim) fail in a whole-file run and
pass in isolation. A/B against the same file with this diff stashed reproduces
both, so they are a pre-existing ordering artefact, not this change.

Per the repo's CPU rule I ran targeted suites only and left the full run to CI.

5. Any other suggestions on the work

• _raw_config()'s docstring claims "cached per process" and the body re-reads
  the file on every call. This PR no longer depends on it, but several dashboard
  tunables still read through it -- including effective_max_background_turns(),
  which is reached from the turn-dispatch path on the loop. Worth its own PR:
  either a real fingerprint-keyed cache or a corrected docstring, because the
  current one invites exactly the mistake this review caught.
• The one-TTL latency is inherited from the allowlist mechanism rather than
  chosen. If a settings toggle should feel immediate, the honest fix is for
  /api/dashboard/config to expire that snapshot after a successful write, which
  would make the whole family (allowlists included) refresh promptly. Left out
  here to keep one owner for the refresh.
• Per-source suppression (a set of enabled source ids rather than one boolean) is
  the natural follow-on once more than one source can exist, per the discussion on
  the issue. Deliberately not in this PR.
• The other two items from that discussion -- the chip presentation fix (label,
  neutral icon fallthrough, widened payload type) and the two source/check
  registries -- stay separate; feat: pluggable source-provider seam for the Changes panel and sidebar chips #6925 is already carrying the seam work.

Closes #6574

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

temp-screenshots/ is an established convention (README plus hundreds of entries on base). The design is verified against the diff: both halves gated, fail-open cold snapshot reasoned, write-time publish ordered under the existing lock, new key backward-compatible and reversible.

Design-Verdict: PASS

Real harm (dead row + credentialed polling nobody wants), gated at both the payload and the refresh feed, reversible default-true key — sound and proportionate.

Suggestions

• The _gitlab_hosts_* lock/TTL/generation family now carries a third, unrelated value, read from state.py via lazy imports into handlers/source_providers.py; rename or relocate the snapshot to a neutral module before a fourth tunable piles onto a name that still says "gitlab hosts".
• The per-key publish_session_card_chips_now + push block inside the generic config PUT handler will be cloned by the next snapshot-backed key; the snapshot-expiry-on-write follow-up you already named would retire it — prioritize that over adding a second block.

[DESIGN-REVIEWED] 93c6d13

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 93c6d134656586722da9114224f619c5a920bf3c and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 93c6d13

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

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 93c6d134656586722da9114224f619c5a920bf3c — 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.

I've gathered what I need: consumer counts, sibling counts, and the existing mechanisms this change rides on. Writing the review.

First-Principles-Verdict: PASS

Every item removes a named cost — a reclaimed row, PR numbers off a shared screen, and credentialed polling nobody renders — and none of it duplicates an existing mechanism.

What this change ships

Intent: let someone stop their session cards advertising every pull request and issue their transcripts happened to mention. ADDITION.

1. New Settings → Chat toggle, default on — justified
2. Session cards draw no chip strip (and no +N) while off — justified
3. Periodic and turn-boundary credentialed provider status calls stop while off — justified
4. New config key dashboard.session_card_source_links, PUT/GET plus a 400 invalid_session_card_source_links — justified
5. The click applies on the next frame instead of within the 30s TTL — justified; publish_session_card_chips_now has 1 consumer (files.py:4054), singular form
6. _load_provider_hosts → _load_source_link_settings, now a 3-tuple sharing the off-loop snapshot — justified by the existing no-config-read-on-the-loop invariant
7. Every Chat-settings dashboard toggle now PUTs only the key it changed — rides along, declared
8. Shared website/scripts/lib/settings-row-shot.mjs; capture-verbosity-levels.mjs switched onto it — rides along
9. New capture harness plus 4 committed PNGs — justified (318 sibling harnesses; temp-screenshots/ is tracked)
10. A merely-quoted PR still reads as ownership by default — symptom-level, declared

Watch

• Default-true means the issue's headline complaint survives on every install; the cause the description itself names, _ChatSlot._pr_source_links (src/kiro_crew/dashboard/state.py), is untouched. Declared, so this is scope, not premise.
• Item 7 rewrites the request body for all 13 toggles on that page and reworked assertions in 4 test files. It is general (both updateDashboardConfig callers are now patch-only: BrowserPanel.tsx:94, ChatPanel.tsx:203, 0 unfixed siblings), but its blast radius far exceeds the toggle it shipped with.

[FIRST-PRINCIPLES-REVIEWED] 93c6d13

[github-actions]

UX Review (Fable 5) — ✅ PASS

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

UX-Verdict: PASS

Real toggle, honest copy: the label names the exact surface, "chips" matches the sibling "File Change Chips" vocabulary, the description states the off-switch's side effect, the config write pushes fresh slots so the chips vanish instantly, and the on/off screenshots prove both states.

[UX-REVIEWED] 93c6d13

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 93c6d134656586722da9114224f619c5a920bf3c — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 93c6d13

Verdict parsed from the review's SHA-scoped output markers for commit 93c6d134656586722da9114224f619c5a920bf3c.

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

[chenmingwei23]

Disposition: GPT 5.6 BLOCKING on state.py:2121 -- ADOPTED, fixed in 52f5aa5

The finding is correct and it was mine. session_card_source_links_enabled()
called _raw_config(), which does read_text() + json.loads() on every call
despite its docstring, and serialize_slots calls it on the event loop for every
slots push. Anchor no-blocking-call-on-event-loop applies as stated.

The prescribed remedy -- "read the setting from an in-memory snapshot populated
off-loop; never call _raw_config() from serialization or refresh paths" -- is
what shipped, using the mechanism that already exists for this exact hot path
rather than a new one:

• The switch now rides the snapshot ensure_gitlab_hosts_loaded() refreshes in a
  worker thread for the self-managed GitLab and Jira allowlists. Same config read,
  same TTL, same lock, same generation counter. That refresher exists because
  "slot extraction is synchronous and cannot load the allowlist itself", which is
  the same sentence about the same call path.
• _load_provider_hosts is renamed _load_source_link_settings and returns
  (gitlab, jira, chips) -- one read, since a second KiroCrewConfig.load() per
  round would double the cost and let the two halves disagree within a round.
• session_card_source_links_enabled() in source_providers is a pure snapshot
  read; state.py keeps a lazy wrapper of the same name (the _cached_check_status
  pattern in that file) so it has no import-time dependency on the handler module.
• The chip switch's own publisher bumps the SHARED generation on a real change, so
  the owner websocket's existing gitlab_hosts_generation() comparison pushes a
  fresh slots payload and the chips appear or disappear without a reload.

Two behaviours this changes, both stated in the PR body rather than left implicit:
the setting now takes effect within one refresh TTL instead of on the next push
(same as an allowlist edit), and the snapshot starts True where the allowlists
start empty -- an unknown host must fail closed, but a chip strip that predates
the switch must fail open or a cold snapshot would blank it for everyone.

Pinned by test_never_reads_config_on_the_event_loop in
test/test_session_card_source_links_knob.py, which fails the test if either
_load_source_link_settings or KiroCrewConfig.load is reached from the getter --
the companion to this file's own
test_gitlab_allowlist_never_reads_config_on_the_event_loop.

Separately: _raw_config()'s "cached per process" docstring is wrong for every
caller, not just this one, and effective_max_background_turns() still reaches it
from the turn-dispatch path. Noted as a follow-up in section 5 rather than folded
in here.

[chenmingwei23]

Disposition round 2 -- head 1a097f9

GPT 5.6 BLOCKING, ChatPanel.tsx:765 "stale full-config write can re-enable
chips" -- ADOPTED.

The finding is right, and the mechanism is worse than the title suggests: it was
never specific to this toggle. setDash did
dashMut.mutate({ ...dashCfg, ...patch }), so flipping ANY of the thirteen
switches on this panel PUT the whole config rebuilt from that tab's query cache,
and the handler applies whichever keys are present -- so every other setting was
written back at its cached value. Two dashboards open, or a concurrent
kirocrew config set, and one flip restored the panel to whatever this tab last
read.

Fixed by sending only the changed keys, which is what this repo already does
elsewhere: BrowserPanel.tsx's own dashboard mutation is
mutationFn: (patch: Partial<DashboardConfig>) => api.updateDashboardConfig(patch),
with the reason in a comment above it. ChatPanel was the outlier, so this is a
convergence rather than a new pattern. The optimistic onMutate write now MERGES
into the cached config instead of replacing it -- writing the patch alone there
would have blanked every other row until the refetch landed, which is the bug the
full-object body was presumably protecting against on the client side.

What this cost: eight assertions in SettingsChatPanelCoverage.test.tsx pinned the
old body shape (one was named "keeping siblings"). They now assert the changed key
alone. The requirement -- do not lose sibling settings -- is unchanged and is met
by the handler, which writes only the keys present in the body; the assertions had
encoded the mechanism, and the mechanism was the bug. 68/68 pass, and the
pre-existing capture-verbosity-levels.mjs harness still passes end to end --
its whole point is that the value re-read after the write is the one just chosen,
which is the round-trip this change had to preserve.

Frontend Lint & Type Check -- FIXED, and it was mine. eslint was clean (657
warnings, 0 errors); the failure was jscpd: my screenshot harness copied the
row-framing geometry out of capture-verbosity-levels.mjs, and the threshold is
0%. Extracted to scripts/lib/settings-row-shot.mjs, both harnesses call it, and
npx jscpd . is back to a single clone -- the pre-existing
capture-grid-divider-align / capture-sidebar-dragbar pair, which measures 0%
and passes. Verified locally by A/B: with my harness set aside the run reported 18
duplicated lines and exited clean, with it 35 lines and the error, so the gate was
tipped by this diff and is now clean again.

Screenshot Evidence was already answered in round 1 with four committed frames
and the harness that produces them; the re-pushed head only moved their pinned
URLs.

Gates re-run on this head: 22 knob tests, test_source_providers.py,
test_dashboard_source_links_expand.py, test_dashboard_state_ws.py,
SettingsChatPanelCoverage (68), tsc --noEmit, jscpd, isort, flake8, the black
gate. Both screenshot harnesses re-run against a fresh build; the four committed
frames came out byte-identical, so the evidence still matches the code.

[chenmingwei23]

Disposition round 4 -- head e17067066. Both advisory CONCERNS adopted.

UX Review -- "the sidebar keeps rendering chips for up to a minute, so the switch
reads as broken" -- ADOPTED. Correct, and correctly diagnosed: nothing invalidated
the snapshot on write, so the value only moved when the owner-WS loop re-ran
ensure_gitlab_hosts_loaded() after CHECK_STATUS_TTL_SECS. The switch is now
PUBLISHED rather than polled -- PUT /api/dashboard/config installs the new value
via _publish_session_card_chips and pushes the slots, so the chips go on the next
frame. The TTL refresh stays as the backstop for an edit made outside the dashboard.
Two things I made sure of, because "publish on write" is easy to get subtly wrong:
only a body that actually carried session_card_source_links republishes (a
link_previews save must not resurrect chips a second tab turned off), and a failure
in the publish path is swallowed -- the write already succeeded, and reporting a
saved setting as unsaved would be worse than a one-TTL delay. Both are pinned:
test_a_write_publishes_the_switch_and_pushes_the_slots and
test_a_write_of_another_key_leaves_the_snapshot_alone.

First Principles Review -- "the include_source_links plumbing threads a flag
through two signatures to avoid a per-slot config read that this diff's own getter
provably never performs" -- ADOPTED, and the subtraction is exactly right. The
threading was designed against the FIRST revision of this PR, where the getter did
read config.json. Once round 1 moved it onto the off-loop snapshot the cost it was
amortizing stopped existing -- and, worse, serialize_slots's docstring still
asserted that cost, so the code and its stated reason had come apart. That is the
kind of claim this lane exists to catch and I should have caught it myself when I
changed the read.

include_source_links is gone from _ChatSlot.to_dict and
DashboardState.serialize_slot, the bool | None sentinel with it, and to_dict
now calls session_card_source_links_enabled() at the point of use.
serialize_slots is byte-identical to main again. The lane's coherence argument
holds and is now recorded in the replacement test: publication happens on the loop,
so a synchronous slot loop cannot observe a flip mid-push.
test_the_switch_is_resolved_once_per_push_not_once_per_slot is deleted as the lane
suggested; test_the_switch_is_resolved_where_it_is_used replaces it, asserting the
payload follows the snapshot AND that none of the three signatures carries the
parameter, so the plumbing cannot creep back in.

Both changes also simplified the PR body, which no longer claims a one-TTL delay or
describes threading that does not exist.

Design Review PASS, GPT and Opus clean on the previous head; Opus's falsified
candidate (missing configKey on the new toggle) matches every sibling dashboard
toggle in this panel, so no change there.

Gates on this head: 25 knob tests plus test_dashboard_files_coverage.py,
test_dashboard_source_links_expand.py and test_dashboard_state_ws.py -- 205 green
together -- with isort, flake8, the black gate and mypy clean (mypy reports only the
2 pre-existing transcribe.py errors).

One note on how the publish tests got their teeth, because the first version of them
was weak and I would rather say so than let it pass as verified. Asserting only the
OUTCOME (snapshot unchanged, no push) could not tell a correct guard from a guard
that was entered and then raised, because the publish path deliberately swallows.
Both mutations I tried passed. So the value is now read outside the try -- an
absent key cannot reach the publisher at all -- and the tests spy on the publisher
itself. Re-checked against both mutations: widening the guard to fire on a body
without the key reds the sibling test, and dropping the slots push reds the publish
test.

[chenmingwei23]

Disposition round 6 -- head 6a1854792. First Principles CONCERNS, item by item.

Watch 1 -- "the lead harm stays on for everyone, because the cause is the extractor,
not the missing knob. Say so, or narrow the extractor." -- ADOPTED (said so).

Correct, and the honest version is now a section in the PR body: mechanism-level for
the extra row and the credentialed polling, symptom-level for the mis-attribution,
because _ChatSlot._pr_source_links still accepts every /pull/,
/merge_requests/, /issues/ and /browse/ URL with no ownership signal, and
default-true means every existing install keeps that. Narrowing the extractor changes
what a chip MEANS -- which sessions are entitled to advertise which links -- and that
is a decision to take on its own, not a rider on an off switch.

Subtraction 2 -- "drop the empty-payload branch at chat_handlers.py:992; the
immediate push removes the +N pill that would reach it." -- HALF ADOPTED. The
critique of my REASON is right; the branch stays for a different one, and the comment
now says so.

The stale-tab story I wrote is indeed weakened by the same round's publish-and-push: a
connected owner window loses its pill at once. But the branch is not only reachable
from a pill. An app token that owns the slot reaches this endpoint DIRECTLY -- the
app-isolation audit right above it logs exactly that allow -- and never had a pill to
lose. Dropping the branch would make "off" mean two different things depending on who
asks, while the switch is meant to gate all three read paths: the slots payload, the
status refresh, and this one. Kept, with the comment and the test docstring rewritten
to the real reason instead of the one that stopped being true.

Subtraction 1 / Watch 2 -- "defer the all-13-switches partial-PUT fix out of this
commit; the defect predates the toggle." -- REBUTTED, with the sequencing as evidence,
and the alternative left on the table.

I agree it is separable on the merits and I would have kept it out by choice. What
makes it not separable HERE is that the GPT lane BLOCKED this PR on it, anchored at
ChatPanel.tsx:765 -- the new toggle's own onChange -- with the remedy "send only
the changed key, or revert this toggle until it uses a partial mutation"
(#7005 (comment)). Deferring
leaves two options: ship a line that lane blocks, or drop the feature the issue asked
for. So it rides along, declared in the body, with the eight expectation edits
explained as mechanism-vs-requirement rather than accommodation.

If you would rather have the clean split, the shape is: a small PR that changes
ChatPanel's mutation to patch-only and retargets the eight assertions, then a rebase
of this one on top of it. That is one extra round trip and a dependency between the
two, which is why I did not choose it unprompted -- say the word and I will do it that
way.

All other lanes are clean on this head: GPT no blocking findings, Opus no blocking
findings, Design PASS, UX PASS (its round-4 concern about the inert-looking switch was
adopted). CI was 69/69 green with PR Readiness and CodeQL passing on the previous head;
this round is a comment, a test docstring and a body section, so the same suites were
re-run locally (40 tests across the knob and expand files, flake8, black) and CI is
re-running now.

[chenmingwei23]

Disposition round 7 -- head 2f37a413a. Both new subtractions adopted.

Subtraction 1 -- drop _session_card_chips_seq; the lock already orders the two
writers. -- ADOPTED, and the counter is gone.

The premise as written was not yet true -- the PUT handler published WITHOUT taking
_gitlab_hosts_lock, which is why I reached for a counter -- but it names the smaller
fix, and it is the right one. The publish now goes through
publish_session_card_chips_now, which takes that lock; since
ensure_gitlab_hosts_loaded holds it ACROSS its threaded load, a write that races an
in-flight poll simply waits and lands last. No module global, no writer-priority
concept, and the ordering argument lives in the mechanism instead of in a comment.

The bounded cost, stated because it is real: a config PUT carrying this key now waits
on an in-flight refresh, which is one config read.

The race test stays but was rewritten to exercise the new path -- it parks a real
worker-thread load, calls publish_session_card_chips_now(False) while it is parked,
then releases. Mutation-verified: publishing without the lock reds it.

Subtraction 2 -- drop the "+N" expand gate; its app-token caller has zero counted
consumers. -- ADOPTED. Gate, import and test removed.

Last round I kept it on caller-uniformity grounds. Counting the consumers settles it
against me, and there is a stronger reason to drop it that I should have seen: an app
token that owns the slot can already read that slot's MESSAGES, and these URLs are
extracted from those messages -- so the gate withheld nothing the caller did not
already have. It was uniformity for its own sake. The endpoint keeps its 404, which is
the one refusal it does owe, and the code comment now explains why it is deliberately
ungated rather than leaving the asymmetry unexplained.

Watch -- item 9 (all 13 switches now PUT only the changed key) should land
separately. -- NEEDS YOUR CALL. Two lanes want opposite things and I will not pick for
you a second time.

• GPT 5.6 BLOCKED this PR on the toggle's own onChange line, remedy "send only the
  changed key, or revert this toggle until it uses a partial mutation"
  (feat(dashboard): let the session-card PR/issue chips be switched off #7005 (comment)).
• First Principles cites AGENTS.md "one logical change per commit" and wants that fix
  out of a feat commit.

Both are right about their own axis. Deferring means shipping a line GPT blocks or
dropping the feature; keeping it means a feat commit that also fixes a pre-existing
clobber across 13 switches and 6 test files. My default is to keep it (declared in the
body, tests explained as mechanism-vs-requirement) because that is the state that is
green right now. The alternative, if you prefer the clean history: I split the
mutation fix into its own PR, land it, then rebase this one on top -- one extra round
trip and a dependency between the two.

Everything else is clean on this head: GPT and Opus no blocking findings, Design PASS,
UX PASS. CI was 69/69 with PR Readiness and CodeQL green two heads ago and is
re-running now; locally 205 tests across the knob, expand, files-coverage and
state-ws suites, plus flake8, isort, the black gate and mypy (only the 2 pre-existing
transcribe.py errors).

[chenmingwei23]

Disposition round 8 -- head 5dcfbad95

Subtraction -- "delete session_card_source_links_enabled in state.py: a second
symbol with the same name as the one in source_providers; its 3 call sites can
lazy-import it the way gitlab_hosts_generation is." -- ADOPTED.

Two symbols with one name is worse than the duplication it saved: a reader who greps
the name finds two definitions and cannot tell which one a call site means, and the
wrapper's docstring had become a second place where the mechanism was described. The
wrapper is gone; to_dict, source_link_urls and source_link_urls_for_slot now
lazy-import the canonical getter at the call site, which is what this file already
does five times over for gitlab_hosts_generation, parse_source_url,
request_check_refresh_now and get_cached_check_status. Verified no reference to the
removed symbol survives anywhere in src/ or test/.

Worth noting the one wrapper that stays and why it is not the same case:
_cached_check_status wraps get_cached_check_status under a DIFFERENT name, so it
shadows nothing.

Watch -- item 9 (the patch-only dashMut change) should be its own commit.

Unchanged from my round-7 answer, and it is now with the maintainer rather than with
me: GPT 5.6 blocked this PR on that exact line and offered only "send only the changed
key, or revert this toggle", so deferring means shipping a blocked line or dropping the
feature. I have asked whether to keep it here or split it into a prerequisite PR and
rebase this one on top; the split is a mechanical change I can do on request. Until
then it stays, declared in the body.

Gates on this head: 205 tests across the knob, expand, state-ws and files-coverage
suites, flake8, isort, the black gate, mypy (only the 2 pre-existing transcribe.py
errors). The previous head was 69/69 in CI with PR Readiness and CodeQL green, and all
five lanes have re-run on every head since: GPT and Opus no blocking findings, Design
PASS, UX PASS.

[chenmingwei23]

Disposition round 9 -- head 5dcfbad95. No code change this round; here is why.

Subtraction -- "delete two of the three gates: gate inside _pr_source_links
instead, it has 4 call sites and covers all three." -- REBUTTED, and it conflicts with
last round's subtraction.

The mechanics work: all three gated readers go through _pr_source_links, so one early
return would cover them and skip the same scan. Two reasons not to take it.

1. The 4th call site is source_links_payload -- the "+N" expand. Gating inside
   the extractor re-gates that endpoint, which is the branch the PREVIOUS round of this
   same lane told me to delete for having no counted consumers, and which this round's
   own item list marks "justified" as ungated. The two subtractions cannot both be
   taken: one says the expand must not be gated, the other gates it as a side effect.
   Reading "the diff already argues it would withhold nothing" as permission inverts
   that argument -- it was the reason the gate has no VALUE, not a reason gating is free.

2. It makes a transcript property depend on a display preference.
   _pr_source_links answers "what does this session's transcript mention", ordered by
   last mention, bounded by _MAX_SOURCE_LINKS_PER_SLOT, cached behind a content
   revision -- all of which its docstring documents as extraction semantics. A
   config-dependent early return makes that name and docstring untrue, and the next
   caller that wants mentions for some other purpose silently gets an empty list. The
   current split is the honest layering: the extractor says what exists, each caller
   decides whether to render or poll it. That is also why the three gates read
   differently -- one skips a scan for a payload, two withhold URLs from a credentialed
   subprocess.

There is a cache hazard in the proposal too, avoidable but worth naming: the result is
memoized under a content revision, so an implementation that cached the empty list
would keep serving it after the switch went back on, until an unrelated message
mutation moved the key.

If you prefer the single gate, I will take it -- but as a pair with re-gating the
expand, since that is what it does. Your call, and it is the second of two open scope
questions on this PR (the other being whether the setDash patch-only fix rides along
or lands first as its own PR).

Watches -- both known and unchanged. The mis-attribution is declared in the body
under "What this does not fix"; narrowing _pr_source_links changes what a chip means
and wants its own decision. Item 7/9 (setDash) is with the maintainer, per round 7.

Status on this head: GPT and Opus no blocking findings, Design PASS, UX PASS, First
Principles CONCERNS (advisory -- its own header says only BLOCK gates readiness). CI is
mid-run; the last complete pass was 69/69 with PR Readiness and CodeQL green. I am
deliberately not pushing a commit for this one: every push re-rolls all five lanes, and
three consecutive rounds of accepted shavings have each produced a new head for the
next round to shave.

[bolichen97]

Full-diff maintainer review passed: change matches its stated scope, no regressions or trust-boundary weakening found, checks green and no outstanding change requests. Approving.

[chenmingwei23]

Rebased onto main at maintainer request -- head 93c6d1346, MERGEABLE again

150 commits behind, CONFLICTING/DIRTY. Two files conflicted; everything else
auto-merged, and the three generated files (config-baseline.json,
settingsRegistry.gen.ts, en-XA.json) were re-generated on the new base and came
out byte-identical to the merged result, so no drift is hiding in them.

No push-forbidden state existed to supersede. No /ai-review override was ever
posted on this PR -- the two grep hits for it are the review bot's own boilerplate
footer offering the command, placeholder reason still in them. Every finding here was
fixed or rebutted on the merits, so this rebase voids nothing.

state.py -- main refactored to_dict out from under the gate. Main extracted
the ~280-line body into self._projection.to_dict(...), which now receives
source_links= plus injected budget_source_links / project_source_links. My
version of that function no longer exists, so I took main's file wholesale and
re-applied the three gates onto it rather than hand-merging a function main deleted:
the extraction skip now sits on main's own source_links = self._pr_source_links()
line, and the two URL feeds keep their early return. Verified the projection derives
BOTH payload fields from the list it is handed (project_source_links(budget(...))
and len(source_links)), so an empty list still yields an empty strip and a zero
total -- the gate composes with the refactor instead of duplicating it.

ChatPanel.tsx -- main's new optimistic overlay vs this PR's patch-only write.
Main landed useOptimisticConfigPaths (shown-value overlay, monotonic token,
per-path save errors) and built it around whole-object writes: "a toggle builds its
payload from the SHOWN config, a second toggle therefore already carries the first
one's in-flight value". This PR sends only the changed keys. Both intents are kept:

• main's overlay machinery stays verbatim -- token, supersede, setPathSaveError,
  cache guard. It is strictly better than the hand-rolled onMutate/onError this
  PR carried, which is now deleted.
• the wire payload is a Partial<DashboardConfig>, so a save cannot write back
  another tab's key at this tab's cached value.
• displayValue: patch => ({ ...dashCfg, ...patch }) merges onto the SHOWN config,
  which preserves main's own property -- a second toggle during an in-flight save
  still composes on the first one's value -- and applyToCache merges for the same
  reason. Main's comment describing whole-object semantics is rewritten, because it
  would otherwise describe a payload the code no longer sends.

One of main's tests asserted the mechanism I changed, so I retargeted its assertion
and left its property alone. ChatPanel.optimisticWrites.test.tsx::"a slow earlier save's success does not clobber a newer toggle's display" asserted the second PUT
body carried BOTH keys. Under patch-only it carries one, and the two saves compose on
the server instead. The property the test exists to pin -- both toggles still read
checked while an older save settles, and after the newer one settles -- is untouched
and still asserted; only the wire-shape line moved, with its comment updated to say
where composition now happens. Flagging it explicitly because it is a
maintainer-authored test, not one of mine: if you would rather the whole-object write
stayed, the alternative is dropping the patch-only change, which the GPT lane blocked
this PR on earlier
(#7005 (comment)).

Gates on the rebased tree. Targeted pytest -n 4 over the touched modules'
suites -- knob, source-providers, source-links expand, dashboard state/ws,
files-coverage, and main's new test_chat_slot_project.py: 678 passed. The only two
failures are test_provider_executable_accepts_user_owned_install /
_symlinked_install, which fail identically with main's own copies of both files
checked out (SourceProviderError: KIROCREW..., the agent sandbox), so they are
pre-existing on this base, not this diff. Frontend: tsc --noEmit clean, jscpd
clean (1 clone, 0%, the pre-existing pair), i18n key + DNT gates clean, catalog
parity / duplicate-keys / settings-registry specs 109 passed, and the five ChatPanel
specs 105 passed. isort, flake8 and the black gate clean; mypy reports only the 2
pre-existing transcribe.py errors. No full suite was run.

Evidence re-captured against a fresh build of the rebased tree -- the harness asserts
its own frames, so this also re-verifies the feature end to end on new main (chip
present with the switch on, the PUT carrying false, aria-checked=false, no chip
after). The two settings-row frames came out byte-identical; the two sidebar frames
differ only in relative timestamps, since the fixture is clock-relative. Body image
URLs re-pinned to the new head.

Push used the audited SCRUBGATE_OVERRIDE=1: all 47 hits are non-ASCII punctuation
(em dash, section sign, arrow) inside main's own already-public commit messages, which
the rebase pulled into the hook's scan range. My commit message is ASCII-clean and is
the only commit beyond main.

[bolichen97]

Rebased this onto current main as part of a maintainer conflict sweep and stopped short of pushing, because one half is an author decision rather than a conflict resolution.

src/kiro_crew/dashboard/state.py resolved cleanly. Main replaced to_dict's whole inline body with a delegation to SlotProjection.to_dict (#6915), so your side of that conflict was the pre-refactor body. Your actual contribution there is the chips gate, which drops straight into main's delegation:

    def to_dict(self, *, include_check_status: bool = False) -> dict:
        # Skip extraction itself when the chips are off ...
        from kiro_crew.dashboard.handlers.source_providers import (
            session_card_source_links_enabled,
        )

        source_links = self._pr_source_links() if session_card_source_links_enabled() else []
        return self._projection.to_dict(
            self,
            ...
        )

Your other two gates (source_link_urls, source_link_urls_for_slot) merged with no conflict, and test_session_card_source_links_knob.py passes against that resolution.

website/src/pages/settings/ChatPanel.tsx is the blocker, and it is a contract collision. Main rewrote this panel's config plumbing onto an overlay abstraction whose design assumes the mutation writes the config WHOLE:

path: () => 'dashboardConfig',
applyToCache: (_cached, next) => next,
mutationFn: (next: DashboardConfig) => api.updateDashboardConfig(next),

Your PR converts the same mutation to patch-only (Partial<DashboardConfig>), and the two are pinned by directly opposing tests:

• main, ChatPanel.linkPreviews.test.tsx:74 — "persists the change through updateDashboardConfig, preserving sibling fields", asserting toHaveBeenCalledWith({ ...BASE_DASH, link_previews: true })
• yours, same file and line — "persists the change through updateDashboardConfig, sending only that key", asserting toHaveBeenCalledWith({ link_previews: true })

Taking main's side compiles and typechecks, but then 12 of this PR's own ChatPanel tests fail, because the test files merged without conflict and therefore carry your assertions. Taking your side would require reworking the overlay's path / applyToCache to be per-key. Either direction is a design decision about the overlay, so I am leaving it to you instead of guessing.

Worth separating out: the behaviour your patch-only change fixes is real and main's overlay does NOT subsume it. The overlay's own comment describes protecting against a second toggle in this tab during an in-flight save; a whole-object PUT built from this tab's shown config still writes back every other setting at its cached value, so a change a second dashboard tab (or kirocrew config set) made after this tab cached is still clobbered. Main's "preserving sibling fields" test is pinning that behaviour, which means the cross-tab case is currently untested in either direction. That may deserve its own issue regardless of how this PR lands.

Nothing was pushed to your branch, so it is exactly as you left it.