feat(apps): identity seam, view-state store, and cache retention

[chenmingwei23]

Part of the app view-state and cache-retention work, now delivered as one PR. The two follow-ups were merged into this branch rather than into main, so this PR carries all three pieces:

• identity seam (this PR, base: main) -- the prerequisite
• view-state store -- merged here from feat(apps): restore where a builtin app page left off #8407
• cache retention -- merged here from feat(apps): keep a builtin app's cached data across leaving its page #8404

Related: #8412 (stale bundle ceiling on main, merged), #8394 (external app cache
isolation, separate track), #8401 (convert the remaining AWS Control key sites,
follows this PR).

Problem / Motivation

App state does not survive navigation. Leave an app, come back, and it has reverted to its initial state: the folder you had open is back at the top level, and the data that was on screen is fetched again behind loading placeholders. AWS Control's drive is the reported case -- descend into a folder, navigate away, return, and you are at the bucket root with a skeleton.

The behaviour is platform-wide rather than a defect in any one app. Eighteen of nineteen apps that issue queries are affected. The one exception fixed it in app-local code no other app could reuse: issue-radar calls setQueryDefaults(['issue-radar'], { gcTime }) at module scope.

Underneath both halves sits a third problem that has to be solved first: a builtin page has no app identity in the React tree. BuiltinAppRoute resolves a route to a component and renders it with no provider, so nothing tells a mounted page which app it belongs to, and there is nothing to scope persisted state or cache retention to. Six builtin app files carry comments documenting exactly that.

Why it matters

An app that returns to its initial state on every visit cannot be a destination a user revisits. It pushes usage toward one long-lived session and penalises navigation, which is the opposite of the intended experience for a dashboard composed of many small apps. Unchanged data is also re-fetched on every return, so the cost falls on the backend as well as on the user's attention.

What changed (motivation -> approach -> change)

Two commits, matching the two layers.

Commit 1 -- the identity seam

builtinRegistry goes from route -> component to route -> {component, appId}, and BuiltinAppRoute publishes that appId as React context.

appId is explicit data, not derived from the route, and there is a concrete case rather than an abstract one: /worlds belongs to the app agent-worlds. A route.slice(1) derivation would mint worlds, which is not an app on this platform. Since the appId becomes a localStorage key segment and a query-key prefix, a derived one would be a permanent namespace nothing else addresses. A test asserts the pairing against the shipped app.json manifests in both directions.

Published from the render body, not an effect. It has to land before the page's first child query mounts -- the ordering issue-radar solves by putting its call at module scope. Under a React.lazy child Suspense hides the difference, but a repeat visit finds the module already loaded and renders the page in the same pass, where an effect is a render too late.

origin is the literal 'builtin', and the proof is registry membership: the registry holds only module code compiled into this bundle, which an external app cannot reach. Reading origin from the ['apps'] query cache there would be weaker -- absent on a cold load -- and would break the synchronous publication. The origin !== 'builtin' refusal lives in useTrustedAppId, where AppHost supplies an origin that is genuinely data. That matters because a namespace is granted by id: an external app can self-register under the name aws-control, and gating on the name alone would hand it the builtin's keys.

getBuiltinComponent is renamed getBuiltinApp rather than kept as a shim -- a caller left on the old name would receive an object where it expected a lazy component and render nothing, so a compile error is the better failure.

The provider then splits. app-sdk/scopedApi.ts owns the sandbox and is imported by path, deliberately off the app-sdk barrel, which is held in exact agreement with the third-party vendor stub: a name placed there is published, and publishing later is additive while un-publishing is a break. AppApiProvider stays on the barrel and composes identity with the scoped layer, publishing identity only when there is none in context -- shadowing a host-minted builtin identity with its external default would revoke that page's namespace silently. Three props every caller hand-wrote identically now default; navigateFn deliberately does not, because a default would mean the SDK importing a router.

Commit 2 -- view-state store and cache retention

View state. An app declares the few coordinates worth restoring; the host owns the key (kc:app:<appId>:view, appId from the identity context) and decides what happens when a record cannot be read back. The declaration is a filter, not documentation: pickDeclared runs on every write, so a field the app did not declare cannot reach storage -- which is what makes "do not persist the drive's contents" an enforced property rather than a convention. The record is read in a useState initializer, so the restored value exists on the consumer's first render and the drive's query is keyed to the right folder from its first request: no wasted root listing, no skeleton flash. scope is first-class, because a prefix means nothing outside the bucket it was taken in, so a mismatched scope resolves to defaults through the same path as a mismatched revision.

Cache retention. BuiltinAppRoute already knows which app owns the route before the page's first query mounts, so a sibling ahead of its Suspense boundary reads the appId and registers gcTime for the [appId] key prefix. react-query matches query defaults by prefix, so one registration covers every key the app already writes by hand -- AWS Control's accounts, drive and costs queries are all under ['aws-control', ...], so the reported symptom is fixed with no change to the app.

useAppQuery is the other half: the host authors the prefix, so an app cannot forge its own namespace, and code that wants the host's cache stays on plain useQuery where the difference is greppable. The prefix is exactly the appId and not ['app', appId, ...], which is the only shape that leaves existing keys untouched -- five prefixes are shared between an app and the host deliberately, and renaming the workflow ones would split the workflow cards in chat off the list in the app. What changes is who authors the prefix, not what it is.

Two call sites are converted, mixed in opposite directions on purpose: one query host-built with a hand-written invalidation, one invalidation host-built with a hand-written query. Either mismatch would stop a consent grant refreshing what it changed, so the pane exercises the byte-identity claim instead of asserting it. The remaining 33 follow in #8401.

issue-radar's own registration is deleted, and not merely as redundancy: setQueryDefaults is a Map keyed by the hashed key, so both wrote the same entry and the last writer won, decided by whether the app's lazy chunk had evaluated yet. Identical values hid it; a future change to either number would not have.

Tests

Identity: appId charset refusals; appId parity against the shipped manifests in both directions; the /worlds anti-derivation pin; the namespace refusal for a non-builtin origin; first-render publication for a cold and a warm page module; a source ratchet asserting the registry has no data-ingestion path, since that is the property the builtin literal rests on; and for the provider split, name resolved from identity, an explicit name overriding it, the loud refusal when neither is available, all three defaults, and the no-shadow rule.

View state: the write filter, every parse rejection, the builtin gate, first-render publication, the scope re-read, the three reporting tiers, and the drive's first request.

Retention: refusals for a host page and an external origin; keys byte-identical to the literals they replace, in both directions; degrade to a plain query with no namespace; already-prefixed keys collapsing with one warning; one registration per client and app; cold and warm ordering, each self-contained; the namespace surviving both api-layer providers nested inside a builtin page, which is the shape spec-builder and IncidentChat use and where a shadowed identity would put an app's data outside the very namespace being retained with no error to see; and an A/B on a faked clock showing the data present at six minutes and collected at thirty with an identity, and gone at six without one -- the reported symptom reproduced.

Mutations were checked individually throughout, each failing a named test, including putting AppScopedApiProvider back on the barrel (fails the stub-parity gate) and removing the i18n exemption described below (fails the diff-scoped i18n gate).

Local gate on the rebased head: tsc -b, eslint src/ --max-warnings 0, jscpd, npm run i18n:check (19 checks PASS), and 2036 tests green across 81 files.

Manual verification

Verified in an isolated pod, with the served bundle grepped rather than assumed, because pod up --provision skips the SPA rebuild on an already-provisioned pod and this repo has previously captured screenshots of a stale bundle. The pod served the freshly built chunk, hash-matched to disk, containing AppScopedApiProvider, agent-worlds, builtin-only, could not resolve an app name and not a valid app id, with the removed getBuiltinComponent accessor absent (0 hits). The /worlds pairing survives minification as a literal, which is the anti-derivation claim in compiled form.

The end-to-end path for the reported symptom was walked in a pod for the retention half: open AWS Control, navigate away, wait past the old 5-minute window, return, and the page repaints from cache with no skeleton.

Screenshots / video

View state -- the dashboard and AWS Control with the store active:

Cache retention -- the reported symptom, before and after leaving the page:

Rebase note

The two follow-ups were merged into this branch rather than into main, which left it at four commits against a MAX_COMMITS: 2 gate and conflicting with a main that had moved 37 commits. It is squashed to two commits along the layer boundary -- identity seam, then the two consumers -- with both trees verified byte-identical to the originals before the rebase, and the commit messages assembled from the original bodies rather than rewritten, so the reasoning that earned the earlier reviews is intact.

Two conflicts, both unions of unrelated additions rather than contested decisions. In DrivePage.tsx, main's type Failure and this branch's DRIVE_VIEW declaration landed adjacent; both are kept. In the App SDK spec, main inserted a paragraph immediately above a heading this branch renamed; the paragraph and the rename are both kept, and the section was re-read afterwards to confirm the prose still reads correctly against the new heading -- it closes the preceding section and carries no cross-reference, and no reference to the old heading text exists anywhere in the repo.

One new finding surfaced from rebasing onto current main: the i18n diff gate flags the view-state record's JSON serialization at app-sdk/viewState.ts. It is a latent failure rather than a rebase artifact -- it reproduces identically against the branch's earlier base, so it was present when that work merged. The line is JSON syntax written and compared by value, never rendered. It cannot take a narrower exemption: it is returned rather than passed, so no callee exemption reaches it, and an inline disable fails because the two gates register the rule under different names and naming both makes each run fail on the one it does not know. So it takes a file-scoped exemption in the shape the SDK's own protocol module already uses, with the module audited as copy-free first, and the exemption is mutation-checked.

The app-core bundle ceiling is NOT touched by this PR. An earlier revision bumped it here; that was the wrong home for it by this PR's own argument -- a ceiling main has drifted into is main's defect, and a separate fix unblocks every open PR rather than one -- and main has since re-measured it independently in #8519. This branch takes main's number unchanged, and the measurement confirms it covers this diff: the branch builds the app-core chunk at 3220.5 KB against main's 3360 KB ceiling, 139.5 KB of headroom, gate green. No ceiling change is needed for this work.

The backend shard failures visible in this PR's earlier check history were main-owned, not this change's: the ten failures were all in one Python test file, this diff contains zero Python files, and main's own fix for that file landed after the run started. Rebasing cleared them. The Coverage Gate red beside them was downstream of the same thing -- its log ends by failing closed on the backend result rather than reporting a finding of its own.

Two further red checks in this PR's history were also main-owned and are recorded here so a reviewer reading the check log does not mistake them for instability in this change. Ten backend test failures came from one Python test file that main fixed after the run started, and a flake8 F811 in that same file came from two main commits each adding the same property to a test double -- fixed on main by #8583. This diff contains zero Python files throughout.

Related Issues

Delivers the approved App View State and Cache Retention design in full: the identity seam, the view-state store, and cache retention.

Out of scope by design and needing its own issue: an external app can currently reach the host's QueryClient, since the shared-modules registry exposes the live react-query instance. That is pre-existing and independent of this feature.

Pattern harvest

Rule candidate: review-prompt

Pattern: a first-render assertion under a React.lazy child is vacuous, because Suspense hides the difference between a render-body publication and an effect.

This nearly shipped. The first version of the test recorded what a consumer saw on each render and asserted the first entry was populated, which looks mutation-proof and is not: moving the publication into a useEffect still passed, because the lazy page suspends, so the parent commits and its effects run before the child renders at all. It only passes for a cold module. On a repeat visit the module is already resolved, React renders parent and child in one pass, and an effect is a render too late -- and a repeat visit is precisely the case this feature exists to serve. Both halves of this PR depend on that ordering, and both are now pinned twice: with a synchronous consumer, which Suspense cannot mask, and with a second mount of the real tree.

The generalizable rule: when asserting when something becomes available, a lazy boundary between the publisher and the observer invalidates the test. Assert against a synchronous observer, or against a second mount.

Rule candidate: review-prompt

Pattern: a PR that states a one-way-door principle must be checked against its own additions before it ships.

This PR argued that publication to the third-party vendor stub is a one-way door, and used that to justify keeping the identity layer off the barrel -- then published AppScopedApiProvider there in the same change, with zero external consumers, on the weaker ground that the export was harmless. Review caught it, and could only catch it because the principle was written down where it could be held against the diff. The check is: when a description states a rule about a one-way door, enumerate that PR's own additions of that kind and say for each whether a consumer needs it now. "Safe" is not the test; "needed" is.

Rule candidate: agents-md

Pattern: the i18n gate recognises a developer diagnostic by its CALL SITE, so relocating an unchanged string out of console.* / Error( / reportSeamCollision( reclassifies it as user-facing copy.

Refactoring two refusal paths into one predicate that returned a reason string moved five diagnostics out of the calls that exempted them, and the gate flagged five untranslated literals in a file whose base count was zero, with prose that had not changed by a character. A pure predicate that returns prose pays this cost; one that reports in place does not. The fix is structural rather than a suppression, because the exemption is a statement about the sink.

Checklist

• At most two commits, with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated -- the App SDK spec covers the view-state record and query-client scope
• No secrets, credentials, or internal references in the diff

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

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

The screenshots confirm the before/after pair, and the reconciliation is complete. The diff adds no new user-visible controls or copy — every confusion in the blind read (the "124.8K/10K" counter, Accounts vs Profiles, the mystery icons) belongs to pre-existing chrome this PR doesn't touch. The one gap: the headline view-state fix (drive folder restored on return, DRIVE_VIEW in DrivePage.tsx) is never shown — all six screenshots show the accounts pane or unrelated pages, never the drive, and the retention before/after was captured on an empty dataset ("0 accounts · 0 keys"), where cached-vs-refetched looks identical either way.

UX-Verdict: CONCERNS

Invisible-infrastructure PR that only improves UX, but its headline fix — the drive folder surviving navigation — appears in no screenshot.

Evidence gaps

• The restored drive-folder state (DRIVE_VIEW path persistence, DrivePage.tsx) is shown nowhere: all screenshots show the empty accounts pane or other pages. Close with a pair (or short recording) of the drive descended into a folder, then returned to after navigating away.
• The cache-retention before/after (shot-01 → shot-03, pixel-identical per the blind read) was captured with "0 accounts · 0 keys" — an empty page proves nothing about retained data. Re-capture with at least one account loaded.

[UX-REVIEWED] c7c8008

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Root cause (missing identity seam) is built first, every mechanism has a named cause, and the one one-way door — barrel publication — is deliberately avoided.

The design gate passes cleanly: the persisted record versions in-blob with all rejections resolving to defaults, so a schema change can never stop a page mounting; the query-key prefix is byte-identical to existing hand-written keys, so no cache migration exists to get wrong; alternatives (per-app fixes, ['app', appId] prefix, origin-from-data, version-in-key) are each named and killed on concrete grounds; and the description matches the diff in both directions, including the smuggled-looking i18n exemption and screenshot convention, which check out. The trust gate (useTrustedAppId refusing non-builtin origins, registry membership as the builtin proof) correctly protects the namespace an external app could otherwise claim by name.

[DESIGN-REVIEWED] c7c8008

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] c7c8008

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

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

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

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

All checks are done: temp-screenshots/ is an established convention (1369 files), usePersistedString cannot serve the scoped case (its useEffect([key, value]) would write the old value under a new key — the exact falsehood viewState.ts guards against), retention's [appId] prefix genuinely reaches most apps' hand-written keys, and the new modules are path-imported, off the published barrel. The one mechanical hit: the view-state store's generalized declaration has exactly one consumer.

First-Principles-Verdict: CONCERNS

The view-state store ships a generalized declaration contract — guard map, revision, canonical serialization — for exactly one consumer declaring one string field.

Not justified as shipped

• Item 5 — one consumer, generalized: grepped useAppViewState across website/src → 1 production consumer (aws-control/DrivePage.tsx, DRIVE_VIEW); one declared field, revision only ever 1.
• Item 8 — rides along: incident-chat error toasts are a behavior change delivered by the new notifyFn default, not by the stated persistence intent; evidenced (the old no-op's own comment says it existed "to satisfy the provider contract"), so it carries no premise risk.

What this change ships

Intent: returning to an app should find it where you left it — folder restored, data repainted without skeletons. ADDITION (platform capability), motivated by the reported AWS Control revert-to-root behavior.

1. Returning to AWS Control's drive reopens the folder you were in, per account — justified
2. Returning to any builtin app repaints cached data for 30 minutes instead of skeletons — justified
3. New persisted state: kc:app:<appId>:view:* localStorage records, cleared at defaults — justified
4. Every builtin page carries host-minted app identity with a builtin-only namespace gate — justified
5. Generic useAppViewState declaration store (fields/guards/revision/scope) — one consumer, generalized
6. useAppQuery/useAppQueryKey host-authored key prefix; 2 sites converted, 33 deferred to Convert AWS Control's remaining query sites to the host-namespaced key #8401 — justified
7. issue-radar's own retention registration deleted; one 30-minute constant remains — justified
8. Incident-chat embed errors now surface as toasts (was a deliberate no-op) — rides along
9. SDK provider split; three props default; origin prop (1 consumer: AppHost.tsx) — justified
10. getBuiltinComponent renamed getBuiltinApp, no shim — justified
    (More than 10 differences ship — safeRemoveItem, an i18n file exemption, RFC/doc updates, screenshots per repo convention — these are the 10 a person would notice.)

Watch

• ViewStateDecl's multi-field guard map and revision are exercised only by tests; the sole shipped declaration is {path: string} at revision 1. The scoped re-read is real (nothing existing does it — usePersistedString writes the old value under a new key on key change), but the generality is ahead of its consumers.
  Clears when: a second declaration lands (multi-field or revision > 1), or the decl shrinks to a single validated string.

Subtractions

• Drop applyCacheRetention + the exported QueryDefaultsSink — 1 consumer (app-sdk/appQuery.ts); inline client.setQueryDefaults(plan.keyPrefix, { gcTime: plan.gcTime }) at that call site.

[FIRST-PRINCIPLES-REVIEWED] c7c8008

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] c7c8008

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

[chenmingwei23]

Bundle Size Gate: pre-existing on main, not this PR

The failing Bundle Size Gate check is not caused by this branch, and I measured that rather than assuming it.

• This branch and its base build a byte-identical t chunk: t-BLZeayKy.js, 755,868 B on both sides, same content hash. This diff does not touch that chunk.
• main's tip 1cd64b8c9, carrying none of this branch's code, reproduces the failure by the same 4 bytes with the same content hash CI reported on the merge ref: t-Cr8LfdHT.js at 757,764 B = 740.00 KB against a 740 KB ceiling.

So the ceiling's measured 702 KB note is ~38 KB stale and main has drifted into it. The fix is a re-measure, and it is deliberately NOT folded in here: the defect is main's rather than this branch's, and fixing it separately unblocks every open PR instead of one. It is up as #8412.

This PR stays red on that one check until #8412 merges. Every other gate is green locally at this head: tsc -b, eslint src/ --max-warnings 0, jscpd, npm run i18n:check (19 checks PASS), and 1369 tests across the 54 files touching app-sdk, the registry, or AppHost.

[chenmingwei23]

First Principles subtractions: one adopted, one declined

Adopted -- dropped the module-scope refuseBadEntry loop over the core table. The finding is right that it was a second enforcement point over compile-time constants. builtinRegistry.identity.test.tsx already holds every core appId to isValidAppId, and I mutation-verified that removing the loop costs no coverage: setting a core appId to Agent_Worlds still fails three named tests (holds every appId to the storage-key charset, names an app that ships an app.json, which also declares that route, and takes /worlds from the manifest, not from the route). refuseBadEntry stays on registerBuiltinComponents, the one caller that takes input the compiler never saw; gutting its appId check still fails eight assertions.

Declined -- the trust half stays. The observation is accurate: useTrustedAppId, AppOrigin and the origin prop have no non-test consumers in this PR. The remedy is not mine to apply, for three reasons:

1. The approved design names the gate as PR 1 scope, not as a later addition: "Copy the builtin-only gate from apps/overlaySlots.ts:77: an explicit if (app.origin !== 'builtin') refusal plus a console.warn", and its verification plan asks PR 1's tests to cover that refusal specifically.
2. The repository owner approved this PR's gate placement with an explicit condition to keep the origin check inside useTrustedAppId, so the two stacked PRs inherit exactly one gate rather than each hand-rolling it. That is the duplication the design argues against in its cache-retention section.
3. Both stacked PRs are already branched off this PR's first commit and consume useTrustedAppId() by name. Deleting it now would break two in-flight branches in order to re-add it in a few days, which defeats the reason the identity API surface was committed first.

On the sub-point that origin rides on the published barrel where surface is one-way: I considered removing the prop and having AppHost mount the path-imported AppIdentityProvider directly, which would add zero published surface. I decided against it because it orphans AppApiProvider -- a published export with no in-repo consumer invites a "delete it" finding, and deleting it is a break. An optional prop with a safe default on an already-published component is a materially smaller one-way door than a new exported name: a caller can stop passing it with no break at all. That is a distinction worth stating rather than assuming, so I am happy to be told I have it wrong.

Escalated to the repository owner rather than actioned unilaterally. No AI-review override used.

[chenmingwei23]

Settled by the repository owner: the trust half stays in this PR

The owner has ruled on the First Principles subtraction that asked to defer useTrustedAppId, refusedNamespaceLogged and the origin prop threading to whichever stacked PR first namespaces state. The deferral is rejected. All three ship here as they stand. This is a scope decision by the owner rather than a reviewer judgement call, so it is closed; please do not re-raise it against a later head.

The substantive reason is worth recording, because it is the opposite of what the subtraction assumed. The gate exists precisely so that the two downstream PRs inherit exactly ONE of it. Moving it into whichever of them happens to namespace state first means the other inherits nothing and hand-rolls its own origin check -- which is the duplication the design's cache-retention section argues against, and it would have arrived silently, as an omission rather than as a visible decision. Deferring a shared gate to one of its two consumers does not remove the gate; it removes the sharing.

The rest of the reasoning stands as posted earlier: the approved design names the builtin-only gate as this PR's scope and asks this PR's tests to cover the refusal, and both stacked PRs are already branched off this PR's first commit consuming useTrustedAppId() by name.

For completeness, the other subtraction in that review was correct and was adopted -- the module-scope loop over the core table is gone, with mutation evidence in the comment above.

No review-bot override was used at any point in this PR. Every finding was either fixed or dispositioned in writing.

Remaining item

The full-window screenshot is outstanding and is deliberately not waived. This environment cannot mint a pod dashboard credential: port_owner returns an unproven verdict because the agent process cannot see PIDs outside its own namespace, so mint_token correctly refuses rather than putting the pod's own secret on the wire to a listener it cannot attribute. The pod is up and serving this exact branch, verified by fetching the served bundle and grepping it, with the table in the description above. Captures will be attached under a temporary screenshots directory with commit-pinned URLs once a credential is available.

[chenmingwei23]

Backend Lint & Type Check is main-owned too, and now has a fix up

Same class as the backend shard failures above, verified the same way rather than assumed.

flake8 reports F811 redefinition of unused 'can_read_body' at test/test_slot_close_recreation_race.py:131. That file in this branch is byte-identical to main's (git diff kirocrew/main HEAD -- <file> is empty), and the finding reproduces against main's tip in a clean worktree with none of this branch's code. This diff contains zero Python files.

Cause: #8536 and #8549 each added a can_read_body property to the same test double, at different points in the class body, so neither conflicted and the class ended up declaring it twice.

Fix is up separately as #8577, kept out of this PR because the defect is main's and one fix unblocks every open PR rather than this one. This PR will go green on that check once #8577 merges and this branch is rebased.

The backend shard failures earlier in this history cleared exactly this way: they were ten failures in that same Python test file, main fixed it, and rebasing turned Backend Tests (Windows) (4) green.

[buluoray]

Comment-only — 0 blocking findings on 224d23fb1cea4b5143c382f72c25ce90c71512e1. Four asks below, none of them a code defect; the biggest is that the evidence for the headline behavior does not demonstrate it.

Reviewed in a worktree at the PR head across five independent lanes — the identity seam, the view-state store, query-key scoping and retention, the repo's own gates, and blast radius over the seven migrated apps — verifying claims against source and mutation-testing each new guard. Roughly 25 mutations, 24 caught by a specific named test.

The seam itself holds up

• The trust gate keys on origin, not on app name, and fails closed. useTrustedAppId returns null unless identity.origin === 'builtin', so an external app self-registering as an aws-control lookalike gets origin:'external' from /api/apps and is refused. Flipping the comparison reddened 6 tests; neutering isValidAppId's ^[a-z0-9-]+$ reddened 7 refusal cases (., .., /, \, uppercase, whitespace, empty).
• No app can address a key outside its own namespace. I had this checked adversarially rather than taken on trust. Every resolveAppQueryKey return path begins with the caller's own appId: a crafted ['other-app','x'] under appId A resolves to ['A','other-app','x'], still inside A's namespace, because the double-prefix collapse only matches the caller's own id. Non-string first elements are prefixed normally. An external app gets null and is un-namespaced, so it cannot reach a builtin's ['aws-control', …]. 10 of 10 mutations in that lane were caught.
• Retention cannot cross a user boundary durably. It registers gcTime = 30min on the shared QueryClient and is memory-only; auth transitions run a full window.location.href navigation that builds a fresh client, and attemptSilentRefresh only invalidates ['auth-me'] for the same user. A → identity change → B never reads [A, …].
• All 126 lines deleted from app-sdk/index.ts were moved, not dropped — the public types re-exported as export type from scopedApi.ts, the rest never exported. No export function/const/default/* removed.
• issue-radar/lib/format.ts's 18-line deletion is genuinely dead code. CACHE_RETENTION_MS has no live referent left, and its 30 minutes is preserved at the new home, registered before the first child query mounts.
• The migration did not weaken its tests. The edits to IncidentChat.cov80 and issueRadarPolling adapt to real behavior changes and in one case assert more than before. 323 tests across the migration-critical suite and 569 across the broader app suites pass; tsc -b --force exits 0.

Ask 1 — the cache-retention screenshots prove nothing

temp-screenshots/app-cache-retention/01-aws-control-loaded.png and 03-returned-no-skeletons.png are the same file: sha256 87cdd464… and byte length 143681 for both. And the frame they share shows AWS Control at "0 accounts · 0 keys · 0 healthy / No accounts yet".

So the pair is vacuous twice over. The "after" image is the "before" image, and even if it were a distinct capture, an empty app renders no skeletons whether or not the cache was retained — there was nothing cached in that frame to retain. The Screenshot Evidence check passed on it regardless, because it greps for the presence of images, not for whether they differ or show anything.

This is the PR's central user-visible claim, so it is worth a real capture: register one account so the page has content, load it, navigate to chat, return, and take genuinely distinct frames. As it stands the behavior is verified by unit tests only, and the UX bot reached the same conclusion independently.

Ask 2 — one mutation survivor, in the store with 671 lines of tests

Removing the defaults-merge in canonicalState (website/src/app-sdk/viewState.ts:257, { ...decl.defaults, ...state } → { ...state }) leaves all 65 tests green. It is unreachable rather than untested: every caller — isDefaultState:261 and serializeViewRecord:269, both reached via resolveViewStateWrite:347 and the write effect at :473 — passes a complete T, since the hook's state is always fully populated. Its stated purpose, canonicalising {} as {path:''}, cannot be triggered by any shipped path.

Either drop it (nothing breaks, which is what the surviving mutation proves) or give it the one test that would make it load-bearing.

Ask 3 — the spec index still says this is unimplemented

docs/system-specs/features/README.md:13 labels the doc "Proposed, not implemented.", but this PR ships useAppViewState. The doc body was correctly updated to match the code; only the one-line index summary is stale, and scripts/docs-lint.sh checks that the entry exists rather than whether it is true, so CI stays green. Since the durable-jobs half really is still proposed, the fix is to split the label rather than delete it.

Ask 4 — a user-visible change arriving by inherited default

ops-mission-control/IncidentChat.tsx previously passed notifyFn = useCallback(() => {}, []), an explicit silent no-op. The diff drops that prop, and AppScopedApiProvider defaults notifyFn = hostNotify (scopedApi.ts:173), which dispatches mc:notify (:143). Embed notifications that were being swallowed now raise toasts on the incident board.

The strengthened cov80 assertion (expect(seen).toEqual(['zzq hello'])) shows this is intended, and it is arguably a fix. Worth a line in the PR body all the same: in a seven-app sweep, "prop removed, default inherited" is exactly the shape an unintended behavior change hides in, and a reader diffing IncidentChat.tsx alone cannot see that the default is not also a no-op. Same pattern in spec-builder, where I did verify the three inherited defaults are byte-identical to the literals removed.

On the two advisory CONCERNS

• First Principles, "479 lines for one consumer persisting one string field" — directionally fair, and I would not block on it. From the code: the namespace gating, the write-side declared-field filter, the synchronous first read and scope-in-key are all load-bearing today (DrivePage really does switch account without remounting, and the gating is a genuine security boundary). revision is speculative but is one int and the right migration seam for a host-owned store. Roughly 65% of the file is doc comment. The only concretely inert piece is Ask 2 above.
• UX, "phantom screenshot claim" — correct, independently confirmed, and Ask 1.

Yellow, no action needed

The view-state keys (kc:app:*:view:*) are never evicted and are absent from safeStorage's RECLAIM_TIERS:44, so quota pressure reclaims other caches but never these. Fine for the shipped consumer, whose scope is an account id and whose default positions self-delete; a future high-cardinality scope would accumulate. The header documents this as an accepted contract, which seems right for now.

Not verified

AppApiProvider's no-shadow early return means a builtin page mounting AppHost for a different external app would let the inner app inherit the outer builtin's trusted namespace. I confirmed this is unreachable today — AppHost mounts only at the /apps/:name route, never nested under a builtin identity — but it rests on a routing fact rather than an assertion, so a one-line invariant comment would keep it that way. Separately, I did not exercise every logout and account-switch path to prove each triggers a full reload; retention never touches disk, so the exposure is capped at the tab's lifetime either way.

Method note: the five lanes shared one worktree, so a few tsc/status readings were contaminated by a sibling lane's in-flight mutation. Each lane verified its own files clean, and the numbers quoted above are from runs I could attribute; that contention is mine, not the PR's.

[buluoray]

Approving 224d23fb1cea4b5143c382f72c25ce90c71512e1.

My detailed review is in the comment above. Nothing in it is blocking: the trust gate keys on origin and fails closed, no app can address a query key outside its own namespace (checked adversarially, not taken from the design summary), retention is memory-only and cannot outlive an identity change, all 126 lines removed from app-sdk/index.ts were verified as moved, and the seven-app migration is behavior-preserving apart from the one intended notify change. 24 of 25 mutations reddened a specific test.

The four asks stand as follow-ups rather than merge conditions:

1. The cache-retention screenshots — 01-aws-control-loaded.png and 03-returned-no-skeletons.png are the same file (sha256 87cdd464…), and the shared frame shows an empty app, so the pair cannot demonstrate the behavior. Worth a real capture with one account registered, since the unit tests are currently the only evidence for the headline claim.
2. viewState.ts:257 — the canonicalState defaults-merge is unreachable; removing it keeps all 65 tests green. Drop it, or add the test that makes it load-bearing.
3. docs/system-specs/features/README.md:13 still reads "Proposed, not implemented" now that useAppViewState ships. The durable-jobs half genuinely is still proposed, so split the label rather than delete it.
4. IncidentChat.tsx — one line in the PR body noting that dropping notifyFn inherits hostNotify, so previously-swallowed embed notifications now raise toasts. The change is right; it is just invisible in the diff of that file.

Also worth a one-line invariant comment on AppApiProvider's no-shadow return: a builtin page mounting AppHost for a different external app would let the inner app inherit the outer builtin's namespace. That is unreachable today only because AppHost mounts solely at /apps/:name — a routing fact, not an assertion.

[iamwhatever]

Big change, require design review first, please do not override and merge

[bolichen97]

Rebased onto main 9af9543b0 by a maintainer as part of the 2026-09-08 open-PR audit (was 376 commits behind, mergeable_state=dirty).

Conflicts and how they were resolved:

• website/src/app-sdk/index.ts / scopedApi.ts: carried merged feat(apps): app session controls — a composer seam for per-chat app state #7573's sessionKey + X-Session-Key injection and merged feat: add contributes.panelTabs app-manifest contribution #7975's active flag into the relocated createScopedApi / AppScopedApiProvider, and forwarded both from AppApiProvider. Taking the moved file as-is would have dropped the fail-open restricted-session guard.
• website/src/apps/builtinRegistry.ts: converted main's new /project-scaffolder entry to { component, appId: 'project-scaffolder' }, which satisfies both directions of your app.json parity test.
• website/src/apps/spec-builder/SpecBuilderPage.tsx: kept AppScopedApiProvider; dropped the now-unused X import (main replaced that button with ErrorNotice).
• website/src/apps/aws-control/DrivePage.tsx: kept both main's new PreviewDialog block and your DRIVE_VIEW declaration; imports follow main.
• website/src/apps/aws-control/ConsoleView.test.tsx: main's feat(aws-control): overview pane and a primitive-based visual pass #8986 renamed the console-cost-value testid to console-cost-stat › stat-card-value, so your two new key-agreement tests now wait on costValue().

Gates run locally: tsc --noEmit clean, eslint clean on all 40 changed frontend files, and vitest on the 18 test files this PR touches — 411/412 passing. The one failure and 4 unrunnable suites are a local node_modules gap (@excalidraw/excalidraw, @radix-ui/react-tabs) in files this PR does not touch, not a code problem; CI will run them properly.

Please review the resolutions, especially the sessionKey carry-over. A maintainer push makes the maintainer the last pusher, so under this repo's last-push rule a second approver is needed. Reply if anything looks wrong.