refactor(chat): split chat page controllers - #7255
Conversation
github-actions
Bot
added
the
readiness: checking
aa12b26 to
ce52c46
Compare
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — ✅ PASSUX-level review of The diff stat showed no image files, so no screenshots to review. I have everything needed: this is a behavior-preserving refactor; the only two new i18n keys are extractions of strings that existed verbatim in the base, translated into all 12 locales but deliberately pinned to English at both call sites to keep legacy notification bytes. UX-Verdict: PASS Pure code-motion refactor; every user-facing string and behavior is preserved byte-for-byte, so there is no new experience to break. Suggestions
[UX-REVIEWED] 2c3df4a |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All checks converge: shared helpers defined once (3 production consumers, no duplication), the feature-map claim verified, i18n entries gate-forced, and the two riskiest spots (string localization, facade removal) handled conservatively and declared. Final review: First-Principles-Verdict: PASS An 8k-line page becomes seven owned modules; every rider is a deletion, a CI-gate mandate, or a declared conservative choice — users learn nothing new. What this change shipsIntent: make the chat page maintainable by giving its interleaved concerns explicit owners, with zero behavior change — a MOVE (refactor), neither fix nor addition. The harm is measured, not asserted: three unrelated behaviours landed on
Counts run: controllers have 1 production consumer each (inherent to a split, not generalization); [FIRST-PRINCIPLES-REVIEWED] 2c3df4a |
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS A real monolith problem, solved by the honest strangler shape: ownership boundaries with behavior byte-preserved, and every trade-off named, tested, or tracked as a follow-up. Suggestions
[DESIGN-REVIEWED] 2c3df4a |
412d33d to
5f0de88
Compare
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
5f0de88 to
3cb0475
Compare
github-actions
Bot
added
readiness: action required
3cb0475 to
6a6d875
Compare
github-actions
Bot
added
readiness: checking
6a6d875 to
aece56d
Compare
|
Follow-up on
|
|
Follow-up on the current First Principles review for
These are deliberate scope/compatibility decisions, not unresolved defects in the extracted ChatPage surface. |
aece56d to
be635e0
Compare
|
CodeQL follow-up for current head
A real remediation is a small, separately scoped change to that hook (with a direct terminal-ID test); it requires explicit scope authorization rather than being folded silently into this behavior-preserving refactor. |
|
Follow-up on the current
The extraction itself, its docs, and its characterization coverage remain the smallest behavior-preserving change; none of the suggested deletions is appropriate in this PR. |
github-actions
Bot
added
readiness: action required
be635e0 to
c93f022
Compare
|
Coverage Gate follow-up for current head
The new head is rebased onto current |
github-actions
Bot
removed
the
readiness: action required
buluoray
left a comment
buluoray
left a comment
There was a problem hiding this comment.
Review of be070c3 — refactor(chat): split chat page controllers
Reviewed as a behaviour-preserving controller split. I did not attempt a line-by-line pass of all 11k lines; I separated moved code from rewritten code first, then reviewed only the rewritten surface and the structural invariants a controller split can break.
Move vs rewrite
The pre-split ChatPage.tsx was 8909 lines; it is now 2099 lines plus seven extracted modules (~11.5k total). Of ~6576 substantive (non-import/non-trivial) lines in the eight head files, ~4192 (~64%) are byte-identical (modulo indentation) to the base monolith. Per-file verbatim-move share: ChatPageMessageContent 95%, Composer 78%, Transcript 70%, ChatPage 65%, Session 65%, Resources 65%, View 59%, Actions 41%. Of the remaining ~36%, roughly 70% is mechanical controller boilerplate — Pick<> port typedefs, prop-bag destructuring, return objects, and JSX prop-threading (foo={ctrl.foo}). The genuinely rewritten logic surface is small and concentrated in useChatPageActionsController.ts (the send callback with references re-plumbed through prop bags) and the View's JSX rethreading.
Defect-class checks (the ones a controller split introduces)
- Duplicated state / two sources of truth: none. No
useState/useRef/useReducername is declared in more than one controller; the ports/Pick<>pattern keeps single ownership. - Effect cleanup lost: none.
useEffectcount 96/96,return () =>cleanups 28/28 (base vs head). - Listener registered twice or never: none.
addEventListener18/18,removeEventListener17/17, and the per-event-name multiset is identical. The 18-add/17-remove asymmetry is pre-existing in base, not introduced here.setTimeout/clearTimeout/requestAnimationFrame/ResizeObserver/MutationObserver/.subscribecounts all identical. - Mid-turn steer / turn-boundary reorder: preserved. The steer decision block (
outcome === 'accepted' && steerNow && ... && !body.queued && !body.steered) is identical apart from_busybeing renamedbusyAtSend. The one new cross-module ordering invariant —syncSlotRunningFromServerdispatch must be registered before the actions controller's auto-send effect that readsslotRunningvia a livestore.getState()— is explicitly pinned by the newChatPage.runningReconcileOrder.test.tsand holds on head (base ordering 4320→4906 preserved as 603→609). Frontend tests are green. - Ref/callback identity churn: the load-bearing optimistic-bubble read
_busy = selectComposerBusy(store.getState(), slot ?? null)was already a live store read in base (line 4703); head renames itbusyAtSendand reuses it in both the bubble gate and the steer gate. Pure rename, same value. - Guard / refusal survival: all early-return refusals survive.
outcome === 'refused'1/1,restoreComposerAfterFailedSend3/3,if (!raw2/2,sendingRef.current4/4,disabled19/19. Elevated counts forsttDisarmedRef/frozenInputRef/postStopEditedRefare port-plumbing (thePick<>unions plus receiving-controller destructuring add references), i.e. the refs are threaded across controllers, not dropped. - Externally observable behaviour: the default export signature
ChatPage({ mode, embedded, embedMode, popout, noUrlSync })is byte-identical (same props, same defaults). All production consumers (App.tsx,ChatPanel.tsx,CoAuthorPanel.tsx,ArtifactChatPanel.tsx) use the default import; the removed named re-exports had zero production consumers.
CodeQL (human-flagged)
The failing CodeQL check is js/insecure-randomness on the relocated mintSendId, which is byte-identical to base (s-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}). The Math.random() sink is pre-existing and merely moved; the value is a client-side correlation id for optimistic bubbles, not a security token. Not introduced by this PR and not a reachable security hole, so not blocking under the blocking criteria.
Verdict
0 blocking findings. No reachable security hole, crash/data-loss, user-visible correctness defect on the chat path, removed guard, or AUTOSDE blocking-rule violation introduced by this diff.
Non-blocking (advisory, already dispositioned): the actions controller consuming a large (~45-field) composer prop bag is structural coupling, not a behaviour defect; the 12 shipped i18n translations pinned to { lng: 'en' } preserve the pre-refactor English bytes by design and are a deferred localization follow-up.
What I could not verify
I did not diff every one of the ~4200 moved lines individually — I relied on structural invariants (effect/listener/timer/state/guard counts and the key decision blocks) rather than statement-by-statement equality, so a subtle reference change inside a line my matcher counted as identical is theoretically possible but unlikely given every structural check held. I did not run the suite locally; I relied on the green CI (frontend tests, E2E, coverage) and on the UX/Design lanes' byte-identical verification for pixel-level render equivalence of the View's relocated JSX.
|
Head moved to Recording what changed since 1. Re-rebased onto the current tip. 2. Fixed a stale source pointer on a row this PR itself edits. Why it was wrong: at the original merge base the Worth being straight about the provenance: the row was already stale on main for the same reason -- it names I audited the class rather than the instance, and this is the only occurrence: the other 16 source paths this PR's doc changes add all resolve and are factually correct. Gates re-run green at this head locally: Unchanged from the previous head: the |
Your dead-code finding is correct and I reproduced it exactly. Why I am still not deleting them here: unlike the two you found before, these are not this PR's code. That distinction is the whole disposition, so here is the proof rather than an assertion. Both statements, and both of their explanatory comments, are byte-identical on The framing they are "the two surviving siblings of the facade class this PR removed" is what I am pushing back on, and it matters for scope. The class you closed twice was real but narrower than it looks: the six-name re-export block and The cost of folding it in is specific rather than theoretical, which is why the answer is a separate PR and not "later". This head has four lanes freshly stamped green at I am recording it in the PR body's follow-up list next to the |
Open PR relationship auditThis 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
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
|
UX CONCERNS —
The Disposition: rebutted on proportionality — the reviewer's fix is a genuine UX improvement but it is a behavior change, so it belongs in a dedicated follow-up (un-pin |
|
First Principles CONCERNS — leave
Both surviving re-exports pre-exist on The asymmetry the reviewer flags is real but the fix is disproportional to a behavior-preserving split and not this PR's to make: (1) Disposition: rebutted on scope/proportionality — the cleanup is welcome as a dedicated follow-up (delete both pre-existing re-exports and reconcile the eight artifact-page mocks in one focused PR), not folded into a byte-preserving refactor. No code change on this head. |
|
CodeQL
I am pushing back on the security-context classification, not dismissing the scanner. Evidence: 1. It is pre-existing on 2. This PR only RELOCATED that byte-identical code — and not even in a file this diff touches. The value CodeQL flags is the optimistic-send correlation id 3. It is not a security context. Disposition: rebutted — not a reachable security use (cosmetic UI de-dup id), and pre-existing on |
|
GPT 5.6 BLOCKING —
The finding is correct about the code. It is wrong about who wrote it, and the remedy it prescribes cannot land in this PR without breaking the contract this PR exists to keep. The markup is Why the migration does not belong in this diff.
What I did instead. Recorded it as Follow-up 5 in the PR body, with the concrete line ranges on both sides, the two-of-three scoping, and the This needs a human call and I am not making it. Two honest ways forward:
Until one of those happens this lane stays red, and I would rather report that accurately than clear it by weakening the gate or by smuggling a UX change through a refactor. |
Round on
|
ChatPage.tsx had grown past 8,500 lines and owned every concern the chat surface has: session and slot lifecycle, the composer and its per-slot drafts, transcript grouping and scroll, attached resources, every action callback, and the whole JSX tree. Nothing could be read, tested, or changed in isolation, and any two edits to unrelated concerns collided. The page is now a thin host that composes seven purpose-scoped units under pages/chat/: session, composer, transcript, resources and actions controllers, plus ChatPageView for the render tree and ChatPageMessageContent for user-content rendering. Behaviour is unchanged — this moves code and draws boundaries, it does not redefine what the page does — so the existing suites keep asserting the same outcomes against the new seams, and the controllers that gained a directly reachable surface gained coverage for it. Behaviours the monolith gained independently of this split are placed in the module that now owns each, rather than reverted to a pre-split spelling or left behind in the host: - The queue-card recipe stays on the shared useQueuedMessageActions hook, so the actions controller merges a cancelled card's text into the draft instead of assigning over it, and threads the in-flight latch through to QueueStack. - Follow-the-output stays gated on live follow state read through vGetFollowRef, so the transcript, session and actions controllers do not force-arm an at-bottom flag and cannot yank a reader who scrolled up. - The opt-in bubble-vanish probe moves with the display-items mirror it measures. - The invisible-only assistant row skip is applied at all five of its sites: the render anchor on the page, the renderer, the footer-host scan and the turn-item host in the transcript controller, and the loose-single host in the view. Regenerate keeps scanning by role, because that scan mirrors the history rewrite the server persists and must not inherit the renderer's skip. - The backend's model-withhold verdict stays on the page, beside the rest of the model-display derivation. - The unresumable-resume gate moves to the session controller with the swap it guards; its notice renders in the view alongside the other pane-level banners, still outside the split / no-slot / transcript ternary so a resume arriving with no active slot can be narrated. - The shared scroll chrome is adopted in the view, where the page's own copies of the header fade and jump-to-bottom pill were. - The composer status stack's re-anchor observer lands in the transcript controller that already holds scrollBottom, the follow ref and the sibling tip/survey compensation effect its rationale points at, while the ref attribute stays on the stack wrapper in the view. - The composer footer's working-tree badge stays beside the project-git query whose `repo` flag gates it and whose cache key it shares, so the page keeps that derivation and hands the view three already-reduced counts. - The session MCP report moves with ChatHeaderMenu into the message-content module. - The mobile drawer's history entry splits across two owners because it is two concerns: the mint/spend pair, the phase machine and the POP-closes-the-drawer effect belong to the drawer in the page, while the flag that distinguishes a bookkeeping pop from a Back the user asked for is owned by the session controller alongside the other pop refs, since that controller's sid effect is what reads it. Registration order holds end to end — the sid and URL-sync effects run before the drawer's POP effect, so consuming the entry cannot resurrect the outgoing session. - The session-switch history bookkeeping a POP is checked against — the pushed-entry key set and the pending-key claim, plus the shared stale-sid repair both POP paths call — lands in the session controller with the two effects that write and read it, because a reader that guessed from the viewport instead is the defect that bookkeeping replaced. - The transcript's deferral is keyed by slot in the transcript controller that owns the display-items mirror, so a session switch renders the incoming transcript in its first commit and only same-slot updates are deferred. - The queued-send stash is written where the receipt is read, in the actions controller's send, and consumed by the same controller's restore, which now merges recovered paths into the staged chips as well as the text. - The user-content render helpers take one options object in the message-content module, so a user row's `/chat?sid=…` link switches session in place like every other row kind; the transcript controller supplies the session triple below the slot-title map it reads, since a dependency array is evaluated in the render body. - The mobile drawer's visual-viewport inset splits the same way the drawer itself does: the page reads the viewport and derives the covered band, because it already owns the drawer and the view declares no hooks of its own, and the two boxes those numbers inset are the view's. Its source contract reads the owning module per clause. - The social-share entry's governance verdict stays beside the rest of the dashboard-config derivation on the page and reaches the assistant row through the transcript controller that renders it. The transcript controller inherited enough moved code to fall under the per-file coverage floor, so the paths that arrived uncovered gain direct tests through a real page render: a pinned message behind the loaded window pages history and lands the jump on it, reports unavailable only once the walk has run out, and is abandoned when the chat it belongs to is left; and a search hit reports its position, with a re-click on the selected result travelling back to it rather than advancing. Source contracts that pin a moved invariant read the owning module per clause instead of one page file. The moved helpers keep no compatibility re-export on the page: the website is a bundled SPA, so the only importers of the old path were repository tests, which now read the owning module like their neighbours. No extraction leaves a zero-consumer export behind either — the seams the split needs are exported, and nothing beyond them. One ordering invariant the split creates is explicit rather than incidental. The running-state reconcile effect is registered above the actions controller, because the controller's auto-send effect reads that reducer's output live: `send` asks `selectComposerBusy(store.getState())`, which reads `chat.slotRunning`, to decide whether to draw an optimistic user bubble. Arriving with `?autoSend=1` on a slot the server already has running is the racing commit, and reading the stale value there draws a bubble for a message the backend answers with its own queued row, leaving the turn in the transcript twice. A source-position contract pins the order and every link that makes it load-bearing, so a later move of either end fails loudly instead of silently duplicating a turn. The feature map's Sessions and Worktrees rows name the extracted owners, since ChatPage.tsx alone is no longer the answer to "where does this live". Co-authored-by: Kiro Crew <noreply@kirodotdev.github.io>
Stand-down: this PR is superseded by an incremental split, not by
|
Split plan — the stack is openFollowing up the stand-down note above with the concrete PRs. They are stacked: each targets the previous one's branch, so read each diff relative to its own base. Merge order is 1 → 2 → 3 → 4; after each merge the next PR is retargeted to
Every slice moves Still to come, in dependency order once the stack lands: the post-virtualizer transcript hook (needs slice 4 and slice 1), the composer controller (its Recommendation stands: close this PR and let the stack carry the goal — your click, not mine. |
|
Stack order update: after #8978 merged, the remaining order is now #9079 (pre-virtualizer transcript state) → #9072 (session controller) → #9078 (resources controller). Reason: the per-file coverage floor on |
4 participants
Problem / Motivation
ChatPage.tsxhad grown into an approximately 8k-line page where URL/session synchronization, composing, sending, transcript rendering, resources, and view chrome were interleaved.Why it matters
The page now has explicit ownership boundaries without changing its route, public props, Redux/WebSocket contracts, streaming order, IME handling, scroll anchors, or responsive composition.
What changed
ChatPage.tsxto a readable composition layer and extracted cohesive session, composer, action, resource, transcript, message-content, and view modules.sendIdwire expression verbatim:s-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}; direct characterization locks its legacy output.playwright-clievent characterization to keep the extracted page above the enforced per-file coverage floor without changing production behavior.ChatSidebar,ChatInput,chatSlice,useWebSocket, backend, Electron production code, dependencies, lockfiles, and CI configuration out of scope.origin/mainand placed the three behaviours that landed onChatPage.tsxwhile this was open by ownership rather than by where they were written. The composer footer's working-tree badge stays beside theproject-gitquery whoserepoflag gates it and whose cache key it shares, so the page keeps that derivation and handsChatPageViewthree already-reduced counts; the per-session MCP report moves withChatHeaderMenuintoChatPageMessageContent. The mobile drawer's history entry splits across two owners because it is two concerns — the mint/spend pair, the phase machine and the POP-closes-the-drawer effect stay with the drawer in the page, while the flag distinguishing a bookkeeping pop from a Back the user asked for is owned byuseChatPageSessionControlleralongside the other pop refs, since that controller's?sideffect is what reads it. Effect registration order is preserved end to end: the sid and URL-sync effects still run before the drawer's POP effect, so consuming the entry cannot resurrect the outgoing session.ChatPage.drawerBackClose.test.tsx(5/5),ChatInput.gitBadge.test.tsxandmcpSessionReport.test.tsxpass unchanged.docs/feature-map/README.md). The Feature Map Gate landed onmainafter this branch was cut and fires on a file appearing underwebsite/src/pages/;ChatPage.tsxalone is no longer the answer to "where does this live". The Worktrees row is the same stale-pointer class as Sessions —api.createWorktreenow lives only inuseChatPageActionsController.ts(verified: that is the sole hit acrossChatPage.tsxandpages/chat/*) — so fixing one row and not the other would have left the map half-true.knowledge_context_was_not_keptandserver_did_not_respond.ChatPageDrafts.test.tsxcarries no timing change at all. An earlier round had raised the rapid-handoffwaitForfrom testing-library's 1s default to 5s, attributing the latency to a saturated coverage worker pool. That attribution was wrong, and it was worth measuring rather than asserting: instrumenting the FIFO handoff shows 244-267 ms on this branch versus 280-383 ms formain's monolith (3 runs each, same host, same file). The split added no render/commit boundary — the extracted page is if anything marginally faster — so the default bound has ~4x headroom and the hunk is gone. That removes the only place a reviewer could reasonably have read this diff as loosening a test.AGENTS.mdComments rule (no "previously / used to / we now" narration, no task-log markers). Five sites: the composer controller's rejected-send fallback, the actions controller'sswitchStabilityport docblock and itsexhaustive-depsdisable, theChatPageViewownership note on the page, and the resources controller's docblock. Each now states the invariant rather than its history. TheswitchStabilityport itself is gone as of the latest rebase —main's refactor(frontend): burn the eslint warning ceiling to zero #7569 narrowedswitchAgent's dependency list to what its body actually reads, which made that port and itsexhaustive-depsdisable dead code, so both are deleted rather than re-documented. I checked the whole diff for this class rather than the named instances, and deliberately left the narrating comments that are byte-preserved moves ofmain's own text (reflowed wrap points only) — rewriting those would enlarge the diff and break the byte-preservation contract that is this PR's point.Tests
npm exec -- vitest run --coverage --pool=forks --maxWorkers=1 --no-file-parallelism ChatPage— 55 files, 489 tests passed.sendId(1/1), and message-content compatibility imports all passed.npm run typecheck, final ESLint budget gate,jscpd, i18n catalog checks (19 checks), and docs lint passed.origin/maintip, all exiting 0:npx tsc -b,npx eslint src/ --max-warnings 0(0 warnings, 0 errors —mainburned the ceiling to zero in refactor(frontend): burn the eslint warning ceiling to zero #7569; see the rebase note below),npx jscpd .(0 clones — the split introduced no duplication),npm run i18n:check(withI18N_BASE_REFat the merge base),npm run lint:phantom-classes(self-test + gate),check_feature_map.py,check_brand_name.py, and 86 vitest files / 872 tests passed covering every changed spec plus everyChatPage*spec. Two gates I did not re-run and am not claiming: the project-wideFRONTEND_MIN: 90average (needs the full ~980-file suite) andcheck-bundle-size.mjs(needs an analyze build; an earlier round ran it on this tree at 739 chunks in budget). Both are low risk here — every touched module measures 83.0-97.6% line coverage against the 80% per-file floor, no baselined file graduates, the diff adds zero production dynamic imports, andvite.config.tsis untouched.1799bf192), everything below exiting 0 on this tree:npm run typecheck(tsc -b),npm run lint(eslint, 0 errors),npx jscpd .(0 clones),npm run i18n:checkwithI18N_BASE_REF=origin/main(19 checks · PASS),npm run lint:phantom-classes,check_feature_map.py,check_brand_name.py,check_harness_parity.py,check_black_formatting.py,check_subprocess_encoding.py,check_changelog_history.py,check_focus_cue.py,check_testpaths_coverage.py,check_builtin_skill_scope.py,check_loop_bound_locks.py,flake8 src/kiro_crew test,mypy --platform linux src/kiro_crew(1288 files, no issues),scripts/docs-lint.sh, andscripts/scrub-lint.sh --no-historyunderLC_ALL=C(see the inherited-failure note below — under a UTF-8 collation locale the[A-Za-z]range in its identity scan admitsé, which is what makesmain's/home/téstfixture look like a hit).ChatPage*slice, because the two source contracts fixed in this rebase are ones aChatPage-scoped selection would have missed.fork.spec.ts(passes 3/3 at--retries=0; reproduces the CI failure when the ported deferral is unscoped).check-bundle-size.mjson a real analyze build — 806 chunks within budget, App chunk 3255.3 KB against the 3360 KB ceiling. This retires the "did not re-run" caveat above.Manual verification
Automated characterization and source-contract coverage exercise the extracted seams. ChatPage Playwright cases require an authenticated agent gateway and can create sessions/send messages; this host has no safe isolated credentialed gateway, so they were not run.
Screenshots / video
No screenshot is included. This controller/view extraction preserves the existing layout and interaction surface; the extracted legacy notification sites are characterized to preserve their original English bytes across non-English and pseudolocale settings. No screenshot was fabricated against the user-owned gateway.
Why no screenshot: This behavior-preserving controller/view extraction leaves rendered layout, interactions, and legacy notification text unchanged. Screenshot-oriented ChatPage Playwright cases require a user-owned authenticated gateway, so no visual evidence was fabricated.
Rebase onto
main(third time main moved under this PR)mainadvanced 137 commits, 7 of which touchedChatPage.tsx— a semantic conflict, not a three-waymerge, because
mainkeeps adding to a file this PR has emptied into controller modules. Every hunkmainadded was re-placed by RESPONSIBILITY, carrying its WHY comments with it. Nothing was dropped: each landing
site is pinned below and verified present.
mainchangesettingsPath()for the model picker's "set as default" (#8098)chat/ChatPageView.tsxTranscriptScrollShellextraction (#7977)chat/ChatPageView.tsxchat/useChatPageComposerController.tsx.mutate({ slot })call sitechat/useChatPageActionsController.tsautoSendTickbump so an already-mounted page still auto-sendsautoSendTicklives there)chat/useChatPageComposerController.tsxswitchAgentdependency narrowing (#7569)chat/useChatPageActionsController.tschat/useChatPageTranscriptController.tsxembedded/popoutargs)chat/useChatPageTranscriptController.tsxhandleFork/handlePlanFromHeretakemessageIdpages/ChatPage.tsxrevealAppInPanelcloses the find pane unconditionallypages/ChatPage.tsxcloseSidebarpages/ChatPage.tsxTwo of
main's hunks needed no carrying — both sides had already converged.main'sexhaustive-depsdisables onhandleFileOpen/handleFolderOpenare unnecessary here because theresources controller hoists
const closeSearch = search.closeonce, so the dep is a bare identifierrather than a member expression; and
main's drop ofscrollToDisplayIndexfrom the nav-scroll deparray is already how the transcript controller writes it. The same hoist is now applied in
ChatPage.tsxforrevealAppInPaneland the source-reveal handler, which is why neither needsmain's disable comment either.One comment was rewritten rather than copied.
main's WHY on the preview-expand deps assertscloseSidebarcloses over onlydrawerXanddrawerTravel; on currentmainit is[runDrawerClose, consumeDrawerEntry], so that text is already stale there. The carried comment statesthe real chain (both links are over a
useMotionValue, auseCallback([]),navigateand a ref, sonothing in it churns) instead of importing a false claim.
Render/effect ordering was reasoned about explicitly, since
tsccannot see it.TranscriptScrollShelldeclares no hooks at all, so interposing it adds nothing to the effecttree and cannot reorder mount effects; its children are still constructed by the parent, so
virt.measureRef/ sentinel refs attach in the same commit and the same document order. DOM order isbyte-identical (header spacer →
aboveRows→ top sentinel → spinner → top spacer → rows → bottomspacer → bottom sentinel →
belowRows),scrollerRefandonScrollstay on the same element, andscrollerStylemerges first so the shell's scroll contract still wins whilepaddingBottomsurvives.The
autoSendTickbump crosses a module boundary (set in the composer controller, consumed by theactions controller's send effect) but is only ever read as an effect dependency, never synchronously in
the same render, so it cannot race.
closeSearchis declared immediately after the resourcesdestructure, above every consumer.
Also in this rebase
main's eslint ceiling is now 0 (refactor(frontend): burn the eslint warning ceiling to zero #7569), which surfaced 6 pre-existingreact-hooks/exhaustive-depswarnings the split had created: refs and setters that were localuseRef/useStatebefore extraction, and which eslint can no longer prove stable once they arrive ascontroller properties. All 9 missing deps are completed, not suppressed — each is a raw
useRefresult or a raw
useStatesetter, so listing them is free and adds no churn.npx eslint src/ --max-warnings 0exits 0.main's scroll-shell characterization net follows the code.ChatPage.scrollShell.recipe.test.tsxand
.render.test.tsxread source TEXT out ofpages/ChatPage.tsx; the invocation they pin now livesin
ChatPageView.tsx. The recipe suite's own header says it "follows the CODE, not the file", soChatPageView.tsx(and the transcript controller, which owns the secondloadingOlderconsumer) joinedits file list, and the render suite's one
readFileSyncwas repointed. Every assertion is unchanged— including the exactly-one-invocation pin, the fail-loud read, and the
loadingOldercount pin.scripts/mutation-check-scroll-shell.mjswas repointed the same way; it now reports the byte-identicalverdict
origin/maindoes (67/83 caught, the same 16 survivors, all of them type/JSDoc lines inmain's ownTranscriptScrollShell.tsx), where before the retarget it aborted at "anchor not found".asserted: across all 14 files,
main's 18-42 added keys and this branch's 2 are all present, with 0dropped from either side and 0 extra.
CodeQLDev Fleet restart/sync give no progress feedback, so users fire them twice #639 is unchanged and stillmain's. Re-verified against the three-dot diff on this head:website/src/hooks/useBottomTerminal.ts— which holds the real tainted source,const mintId = () => Math.random().toString(36).slice(2, 14)at line 58 — is touched zero times bythis branch. The only
Math.randomthe diff adds is thesendIdexpression, a verbatim move locked by acharacterization test. The existing rebuttal disposition stands; no suppression was added.
./scripts/scrub-lint.sh --no-historyfails ontest/test_atomic_write_named_duplicates.py:155,157(
/home/tést). Inherited: that file has zero hits in this branch's three-dot diff and both lines existbyte-identically on
origin/main.Rebase onto
main(fourth time main moved under this PR)mainadvanced 45 commits past the base this branch last carried, 5 of which touchedChatPage.tsx— again a semantic conflict rather than a three-way merge, becausemainkeeps adding to the file this PR has emptied into controller modules.
ChatPage.tsxresolved tothis branch's thin host and each of
main's five hunks was re-placed by RESPONSIBILITY, carryingits WHY comment with it. Nothing was dropped: every landing site is named below and verified
present in the tree.
mainchangeuseSlotDeferredValue, #8581)chat/useChatPageTranscriptController.tsxchat/useChatPageSessionController.tschat/useChatPageActionsController.ts/chat?sid=…link switches in place (#8510)chat/ChatPageMessageContent.tsxchat/useChatPageTranscriptController.tsxsettingsPath({ tab: 'voice' })for the voice-setup modal (#8261)chat/ChatPageView.tsxOne placement is not a straight copy, and the reason is a language rule rather than a taste
call.
renderUserContentCbnow readssessionTitles, so it had to move BELOW that memo: auseCallbackdependency array is evaluated in the render body, and declaring the callback abovethe memo would read the binding before its initializer runs. Both consumers (the row renderer and
the controller's return) are further down, so this is a pure relocation, and the position carries a
comment saying why it is load-bearing rather than incidental.
Two source contracts had to follow the code — the same class the third rebase already handled
for the scroll-shell net. Both landed on
mainafter this branch was last rebased, so CI has neverrun either of them against the split:
pagination.earlierAdmission.test.tsreads all four automatic older-history triggers out of onesource file. All four, and the admission ref they share, now live in
useChatPageTranscriptController.tsx, so that is the file it reads. Every assertion isunchanged, including the two ordering pins and the exact
gated === 2count. One end anchor isnow the deps-list PREFIX (
}, [dispatch, earlierBarInView) instead of the whole array, so a depadded for an unrelated reason cannot read as a gate violation. 12/12 pass.
injectBubbleWhitespace.test.tsfinds the transcript's single warn-tinted bubble by class andasserts it carries no
whitespace-pre-wrapand does passsoftBreaks. That bubble is now in thetranscript controller's row renderer. 3/3 pass — including the "exactly one container" positive
control, which is what keeps the other two from passing vacuously once markup moves.
Every previously-red lane on
847ecdcf, attributedTen checks were red. Nine are answered by the rebase or by this branch's own fix; the tenth is
dispositioned below.
Backend Tests (3.12, 4)main's, already fixed upstream101125494842: all 10FAILEDlines nametest/test_slot_close_recreation_race.py— 9 ×Failed: Timeout >120.0splustest_cleanup_ordinary_archive_still_saves_and_removes - AttributeError: '_Req' object has no attribute 'can_read_body'raised atsrc/kiro_crew/dashboard/handlers/_shared.py:142. That file arrived onmaininc477f0b9dwith a request double that never modelledcan_read_body;5bc2fc796(#8536) and35426b9ea(#8583) fixed and de-duplicated it, and both are ancestors of this head (exactly onecan_read_bodyproperty remains, attest/test_slot_close_recreation_race.py:127). This diff contains no Python at all.Backend Tests (Windows) (4)101125212737: 4 ×worker 'gwN' crashed while running 'test/test_slot_close_recreation_race.py::…'. Windows overrides--max-worker-restart=0and has noSIGALRM, so the same hang kills the worker instead of timing out.Frontend Tests (4)101125494986:pagination.earlierAdmission.test.ts > the top sentinel checks it first,expected '' to match /if \(!earlierBarInView\(\)\) return/— the contract was still readingpages/ChatPage.tsx. Migrated above. The whole website suite is now green on this tree: 1850 files, 29022 passed, 1 expected fail, 2 skipped, exit 0.Frontend Coverage Mergeci.ymldeclaresneeds: [frontend-test, changes]and merges the shard blobs, so a red shard exits it non-zero by construction.Coverage GateBundle Size Gatemain's drifted ceiling101125494908:FAIL assets/App-DE1AGs-8.js: 3.15 MB exceeds its 3.13 MB budget by 20.9 KB (chunk 'App').449425a50(#8519) re-measured that ceiling toApp: 3360 * KB; on this head an analyze build measures the App chunk at 3255.3 KB andnode scripts/check-bundle-size.mjsexits 0 with "806 chunks within budget". ~105 KB of headroom; the split's own contribution overmain's recorded 3201 KB is ~54 KB of module scaffolding, and no ceiling was raised.E2E (stub ACP backend, offline)main's, and proven so on this tree101125213011:fork.spec.ts:49expect(more).toBeVisible()→element(s) not found, waiting onassistant-more-actions. That ismain's #8526, whose fix commit is titled "(E2E fork.spec red on main)". Verified causally rather than argued: with the ported deferral scoped to the slot,playwright/fork.spec.tspasses 3/3 at--retries=0against the harness gateway + fake ACP backend; changing that one call to an unscoped deferral reproduces the CI failure exactly — same assertion, same locator, same line — and scoping it back makes it pass again.CodeQLmain, line-shiftedjs/insecure-randomnessatwebsite/src/pages/ChatPage.tsx:981.main's open alert #639 is the same rule in the same file at:5894, and both lines are the identical sinkconst sessionId = addDockTerminal(currentProjectRef.current ?? undefined). The tainted source,mintIdatwebsite/src/hooks/useBottomTerminal.ts:58, is touched zero times by this diff. ShorteningChatPage.tsxmoved the reported line and nothing else. No suppression added, no alert dismissed; Follow-up 1 still owns the real fix.GPT 5.6 ReviewPR ReadinessGPT 5.6 Review—errors-use-error-noticeonChatPageView.tsx:1082The finding is correct about the code and wrong about the author, and I am not going to fix it
inside this PR. The three notices it names —
uploadError,sidError,pinStatus— existbyte-identically on
origin/maintoday atwebsite/src/pages/ChatPage.tsx:8672-8688; this diffmoves them into
ChatPageView.tsxand changes not one character of them. The lane charges thembecause a new file's every line is an added line, which is the structural reason a
behaviour-preserving extraction cannot clear a whole-file rule scan without also paying off the
debt it relocated.
Two reasons that payoff does not belong here:
ErrorNoticerenders different chrome and, withaskAgent, navigates to the chat and unmountsthe tree that raised the banner. That is a UX change requiring its own screenshot evidence —
this PR carries a
no-visual-deltadeclaration precisely because it has none to show.askAgentis a per-surface product judgment the rule itself assigns to the author ("TheAUTHOR decides per surface"), and an upload failure that happens while a composer holds an
unsent draft is exactly the case the rule warns can destroy unsaved state. Deciding it in a
12k-line refactor, unreviewed on its own, is the wrong place for it.
One of the three is also not an error at all by the rule's own definition:
pinStatusisrole="status"and reports a pin that succeeded, and the rule explicitly excludes "status textabout something that has not failed".
This needs a human call, and I am deliberately not making it. Either a repository writer
records
/ai-review override gpt <head>: pre-existing markup moved verbatim; ErrorNotice migration tracked separately, or theuploadError/sidErrormigration lands as its own PR with its ownvisual evidence and its own
askAgentdecision — added as Follow-up 5 below. Until one of thosehappens this lane stays red on merit, and I would rather report that accurately than clear it by
weakening the gate.
Rebases five and six, and the one red that was mine
mainmoved twice more while this round was being verified — 45 then 32 commits — andeach time on
ChatPage.tsx, so the same semantic resolution ran again. Five more hunkswere re-placed by responsibility:
mainchangepages/ChatPage.tsx+chat/ChatPageView.tsxcapabilities.social_sharegating the share entry (#8565)pages/ChatPage.tsx+chat/useChatPageTranscriptController.tsxchat/useChatPageSessionController.tschat/useChatPageActionsController.ts+chat/ChatPageView.tsxmain's newhoverHold.tspointer andPOST /api/spawn/stop-allalongside this branch's extracted ownersdocs/feature-map/README.mdThe drawer inset is the only one whose placement is a judgement rather than a lookup, and
the reason is worth stating:
ChatPageViewdeclares no hooks at all, and the pagealready owns the drawer (the mint/spend pair, the phase machine, the POP effect). So the
page reads
useVisualViewport()and derives the covered band, and the two reduced numberstravel to the boxes as layout view-model fields.
mobileSessionsDrawerViewport.test.ts—which
mainadded after this branch was last rebased, so CI has never run it against thesplit — now reads the owning module per clause: the hook and the
Math.max(…)derivation from
pages/ChatPage.tsx, the scrim and panel style clauses fromchat/ChatPageView.tsx. Every regex is unchanged, including the raw-sourcekey="sessions-backdrop" … z-[46]adjacency guard that keepsChatPage.composerChromeOcclusion.test.tsxfrom silently comparing againstundefined.ChatPage.sid.test.tsxtook the union of both sides' cases and both sides' harnessoptions (
connectedandslotsLoaded), somain's two slot-list-arrival tests and thisbranch's offline-deep-link test all run.
Coverage Gatewas real, and it was this PR'sThe one red on
96883a23that was neithermain's nor downstream of anything:Where it came from, measured rather than guessed. I pulled the
coverage-frontendartifact from the last run whose Coverage Gate actually completed on this branch
(33851937200, head
9e5201367) and compared it with the current one: that file measured463 lines / 83.8% then and 655 lines / 77.4% now. The +192 measured lines arrived
with the THIRD rebase, whose Coverage Gate never ran — it failed closed on
main'spoisoned
test_slot_close_recreation_race.pywithcoverage-combine=skipped, so theper-file check was never reached and the shortfall sat unmeasured for a round. My own
delta to that file is +32/−8 and is fully covered, so the debt is inherited, but it is
this PR's to pay and I have paid it rather than baselining it.
Five direct tests through a real
render(<ChatPage />), in the suite that alreadydrives the page's pin and search seams:
still outstanding;
the same exhausted-walk mock that does reach the notice without the switch, so it is
not vacuous;
code paths behind one click.
That is 40 previously-uncovered lines: 77.4% → 83.51%,
check_per_file_coverage.pyexits 0 with 1223 files at or above the floor and none below, and the project average
is 92.87% against the 90% minimum.
One case I wrote and then deleted rather than ship. A sixth test aimed at the
older-fetch rejection arm could not be made deterministic: the transcript's own
older-history pollers page the same offset, and when one of them wins the race the jump's
own dispatch is refused by the thunk's
condition, which reads as a supersede and isdeliberately silent — so the arm under test never runs. Three shapes were tried
(reject-always → an unbounded retry loop that OOM-killed the vitest worker; reject-once →
the poller took the failure; reject-N-then-park → same). It is deleted, that arm stays
uncovered, and I would rather say so than leave a test that passes for the wrong reason.
Verification on this head
Everything below run locally on this tree, all exiting 0:
tsc -b;eslint src(0errors);
jscpd(0 clones);npm run i18n:check(19 checks · PASS);lint:phantom-classes; the elevencheck_*.pyratchets (black,subprocess_encoding,changelog_history,focus_cue,testpaths_coverage,builtin_skill_scope,loop_bound_locks,brand_name,harness_parity,feature_map,per_file_coverage);flake8 src/kiro_crew test;mypy --platform linux src/kiro_crew(1288 files, noissues);
docs-lint.sh;scrub-lint.sh --no-history(underLC_ALL=C— see theinherited-failure note above). Plus:
ChatPage-scoped slice: 1853 files, 29070passed, 1 expected fail, 2 skipped;
ACP backend: 253 passed, well above the suite's own executed-spec floor, 0 skipped;
check-bundle-size.mjson a real analyze build: 807 chunks within budget, App chunk3259.1 KB against the 3360 KB ceiling — no ceiling raised.
Follow-ups (deliberately not folded into this refactor)
Recorded here so they are not lost with the PR:
CodeQL alert Dev Fleet restart/sync give no progress feedback, so users fire them twice #639 (
js/insecure-randomness, high) ismain's, not this PR's. Re-verified on this head: the alert isstate: open,created_at: 2026-08-20(before this branch existed), and itsmost_recent_instance.refisrefs/heads/main. The tainted source isconst mintId = () => Math.random().toString(36).slice(2, 14)atwebsite/src/hooks/useBottomTerminal.ts:58, and this branch's copy of that file is byte-identical tomain(the three-dot diff touches it zero times). The extraction only moves the reported sink line by shorteningChatPage.tsx. The oneMath.randomthe diff adds is thesendIdexpression, which exists byte-verbatim onmainatChatPage.tsx:432and is a pure move locked by a characterization test. No suppression was added and no alert dismissed. The fix belongs in its own PR: replacemintIdwithcrypto.randomUUID(), which closes Dev Fleet restart/sync give no progress feedback, so users fire them twice #639 formainand for every PR that inherits it.Unpin
lng: 'en'on the two moved notification strings.knowledge_context_was_not_keptandserver_did_not_respondare translated into 13 catalogs that no locale can currently render, because both call sites pin the English bytes. That pin is correct here — onmainthese were bare English literals, so unpinning would change visible language selection and break the byte-preservation contract this PR is — but it is a real wart. The follow-up unpins both and deletes the two assertions that lock the English bytes, as a deliberate behaviour change reviewed on its own.Shrink the
ChatPageViewoption bag. The view takes whole controllers through a wide props surface. That is the honest shape for a behaviour-preserving strangler step, and narrowing it is a follow-up refactor with its own risk, not part of this move.Delete two dead re-exports
mainleft inChatPage.tsx.export { isBrowseCommand }(:14) andexport { PREFILL_STORAGE_KEY } from '../utils/navIntent'(:47) have zero importers of any kind —grep -rnE "import \{[^}]*\} from '[^']*(pages/)?ChatPage'" src/returns nothing, not even a test; every consumer already imports fromutils/. Both statements and both of their explanatory comments are byte-identical onorigin/main(:17and:147), and the three-dot diff contains no+line for either, so this is inherited debt rather than something the extraction invented — unlike the six-name re-export block andChatPageSourceLink, which this diff did create and which were therefore deleted in it.main's own comment claims the second one serves "this page's historical importers"; that claim is false onmaintoday, independent of this branch. The fix belongs in its own PR: two provably-unimportable lines are trivially reviewable on their own evidence, and folding them in here would force-push over four freshly-stamped green lanes to change nothing a user or a compiler can observe.Migrate the chat pane's two error notices to
ErrorNotice.uploadErrorandsidErrorrender as hand-written bordered<div>s (nowchat/ChatPageView.tsx:1082-1093, byte-identical toorigin/main'sChatPage.tsx:8672-8688), which theerrors-use-error-noticerule inwebsite/AUTOSDE.yamlforbids. The migration is a visible change to chrome plus a per-surfaceaskAgentdecision — an upload failure can coincide with an unsent composer draft, which is the state the hand-off destroys — so it needs its own screenshot evidence and its own review, not a hunk inside a behaviour-preserving split.pinStatus, the third box at that site, isrole="status"for a pin that succeeded and is outside the rule by its own wording.Rebase seven —
main's registry migration folded in, and the one blocker leftmainmoved three times underChatPage.tsxsince the last round, and two of thosecommits landed in the exact code this split relocates, so the rebase was a real merge
rather than a replay. Base is now
a540e334b.What
maincontributed, and where each piece now livesf1f6fb3faand73d60a83d(chat-core P5-a / P5-b) replaced the page's inlinerenderMessagerole chain with registry dispatch, and7225509f6reworked everyautomatic older-history door. Both landed in code this PR moves, so each hunk was
re-homed onto its new owner rather than replayed onto a page that no longer holds it:
useChatPageTranscriptController.tsxnow owns the registry host list —mergeRenderers([...createTranscriptRenderers(…), stop_event, notice, permission, undrawn, mcp_oauth, hidden_invisible_assistant, bubble])withresolveRendererandthe bubble as the by-reference fallback — because the controller is what owns the
page's row dispatch. The if-chain,
hasReasoningContent/isReasoningRole, and thenine row-component imports are gone from it;
REASONING_ROLESandNO_AUTO_DENIEDreplace them. It also takes the whole of fix: stop the chat transcript moving on its own #8574: the idle-prefetch effect is deleted,
sawRealInputRefis replaced by the expiringREAL_GESTURE_AUTH_MSwindow,sentinelPagesSinceInputRef/walkPagesSinceInputRef/walkLastInputAtRefbecomerefs with a slot-change reset, the sentinel gains the empty-transcript guard and the
OLDER_WALK_MAX_PAGES_PER_INPUTbound,keydown/pointerdownjoin the gesturelisteners, and every door logs through
scrollInspector.ChatPageMessageContent.tsxgainsanchorAltIdFor(the lead-message anchor thedual-identity restore needs) and loses
toolDisclosureKey, whichmainmoved intothe shared row set — so the disclosure key now has exactly one definition, in
chat/transcriptRenderers.tsx, for every surface.ChatPageView.tsxtakes the restore cover: the centredslotLoadingspinner isgone,
ChatTranscriptSkeletoncovers both waits, and the shell'sscrollerStylecarries
visibility: hiddenwhilevirt.restoreGateis up.ChatPage.tsxkeeps thedevWatchMessagesprobe beside itsmessagesselector.lastErrorIdxis deleted from the actions controller and from the two hand-offsthat threaded it, because the shared row set derives the newest-error index itself
(
transcriptRenderers.tsxlastErrorIndex).The six source-contract tests that pinned the old spelling were repointed at the module
that now owns each clause, keeping
main's assertion and not the pre-registry one:chatRolesParity.contract.test.ts(taken frommainwholesale, paths repointed),transcriptRenderers.test.tsx,NoticeCard.test.tsx,invisibleText.test.ts,ChatPage.mcpOAuth.test.tsx,toolDisclosureKey.collision.test.ts, plusRecoveryCard.test.tsxwhose auto-merge had mixed one side's path with the other's.Verification on this head
tsc -b --force,eslint src,npm run build, the elevencheck_*.pyratchets,flake8 src/kiro_crew test,mypy --platform linux src/kiro_crew(1296 files, noissues),
docs-lint.shandscrub-lint.sh --no-history(underLC_ALL=C.UTF-8, CI'slocale — see the inherited-failure note above) all exit 0. The full website vitest
suite: 1865 files, 29308 passed, 1 expected fail, 2 skipped, 0 failed.
The three previously-red lanes the rebase cleared
Backend Tests (3.12, 3),Backend Tests (Windows) (3)andCoverage Gateare allgreen on this head with zero Python in the diff. They were
main's push-gatebreakage, repaired upstream by
c791f0f1d("hand the push arity scan raw words, not cutones"), and the rebase is the whole fix.
GPT 5.6 Reviewis green here too, and themerge conflictlabel is cleared (mergeable: true).Opus 4.8 Review— the lane cannot read a diff this size, and that is the blockerThis is the only item
PR Readinesscounts as blocking. It is not a finding: thelane never produced one. Its own steps say why, in order:
Flag oversized reviewable diffprintsreviewable changed lines (excluding src/kiro_crew/_vendor): 21290against its declaredMAX_REVIEWABLE_LINES: 15000,and warns "likely too large for one reliable review pass. Consider splitting it into
smaller, self-contained PRs."
Prefetch the reviewable diffwrites 1,335,611 bytes to one file, and thediscovery prompt's only instruction for obtaining the diff is to
Readthat file.--allowedTools "Read,Grep,Glob"and a 200,000-token contextwindow. It cannot read 1.34 MB and has no shell to chunk it, so it ends
subtype: success,is_error: false,num_turns: 16,permission_denials_count: 36— without emitting[OPUS-DISCOVERY].Capture discovery candidatesthen fails closed on the missing marker, correctly:"validation against a truncated or empty list would emit a clean
[OPUS-REVIEWED]verdict and pass the gate on a review that never happened."
Four attempts across two heads (
745a3f1d0×3,2c3df4a32×1) are byte-for-byte thesame outcome, and the rebase made the diff larger, not smaller. Nothing on this branch
can change that: the lever is either the workflow (out of this PR's scope, and behind the
workflow-change guard) or the diff size.
Recommended: split this PR. One controller per PR, extracted from the then-current
ChatPage.tsx, in dependency order —ChatPageMessageContent(~1.5k changed lines, andfirst because every controller imports it), then
useChatPageSessionController(~1.4k),useChatPageResourcesController(~1.7k),useChatPageComposerController(~3.5k),useChatPageActionsController(~3.8k),useChatPageTranscriptController(~4.6k, whichcarries the registry host list and the source-contract repoints), and
ChatPageView(~4.5k) last, once the page is only composition. Every slice is then wellinside the lane's own 15,000-line budget and reviewable on its own evidence. A
two-way split (the six extracted modules ≈12k, then the view plus the final page
reduction ≈9k) also clears the threshold, but each half is still a large read.
The other route is a maintainer
/ai-review overrideon this head, which is arepository-writer judgment and not mine to record. Worth weighing either way: every other
AI lane —
GPT 5.6 Review,Design Review,UX Review,First Principles Review,Code Review— is green on2c3df4a32, so an override skips one lane rather than thereview as a whole.
CodeQL— red in the UI, and not a readiness blockerUnchanged from follow-up 1 below, re-verified on this head. Alert #639
(
js/insecure-randomness, high) isstate: openonrefs/heads/mainata540e334b— this PR's own base — with the same alert number, the same rule, and thesame tainted source,
const mintId = () => Math.random().toString(36).slice(2, 14)atwebsite/src/hooks/useBottomTerminal.ts:58, a file this diff touches zero times. Thereported sink is
const sessionId = addDockTerminal(…), byte-identical tomain's; onlyits line number moved, from
ChatPage.tsx:5959onmainto:1017here, because thefile shrank. The check's own summary says the rest: "Alerts not introduced by this pull
request might have been detected because the code changes were too large."
It also does not gate the merge.
PR Readinessreads thedynamic/github-code-scanning/codeqlworkflow run, whose conclusion on this head is
success; the redCodeQLcheck-runis the advanced-security alert report, which readiness does not consume. Hence
1 blocking readiness item(s), and that one is the review lane above. No alert wasdismissed and no suppression added.
Related Issues
None — planned behavior-preserving refactor.
Checklist
origin/main