mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(mobile): reject a page storage write for size only, and log the rest (OTA phase C, C7.7 round 1)
Ruling 33.4. `PageStorageRefusedError` was raised for all three refusals, and two of them have no catcher: a page-closure writer of an unlisted key awaits `setItem` with nothing around it — `notification-delivery-preferences.ts:39` plainly, `preferences.ts` in several places — so a key the page was never allowed to keep became an unhandled rejection in the document. That is a worse failure than the silent drop it replaced, and it is the one the page can least afford, because an uncaught rejection there is a document-level error on a screen that is otherwise working. Scope is now one refusal. `too-large` rejects, because the caller that needs it is written for it: the durable send journal's composer catches it and answers "Message not sent" rather than sending a mutation whose operation id was never written down (ruling 7). `not-allowed` and `not-delivered` resolve and are logged as `[page-bridge] storage-write-dropped`, which is the old behaviour plus the line a device log needs — a preference that did not stick looks identical to one nobody set. A batch applies every pair it can, logs every drop, and rejects only if one of them was oversize. Red first, measured here: seven cases in `page-async-storage.test.ts` red on the rejection, among them a `notificationDeliveryPreferences` write resolving, another host's pins, another workspace's chat tabs, and a write the shell would not take. The oversize case is unchanged and still asserts `PageStorageRefusedError` with the key and the character bound in its message, so the narrowing is visible as the difference between the two. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
@@ -84,33 +84,27 @@ describe('a write the page makes', () => {
|
||||
await expect(pageAsyncStorage.getItem('orca:pins:host-1')).resolves.toBeNull()
|
||||
})
|
||||
|
||||
it('is refused, and not kept, for a key outside the allowlist', async () => {
|
||||
it('is dropped, and not kept, for a key outside the allowlist', async () => {
|
||||
// Held locally it would answer a later read with a value no other screen in the app can see —
|
||||
// a pin that looks set and is not, which is the failure the grant exists to avoid.
|
||||
const refusal = await refusalOf(pageAsyncStorage.setItem('orca:mobileWebShellEnabled', 'true'))
|
||||
expect(refusal.refusal).toBe('not-allowed')
|
||||
// a pin that looks set and is not, which is the failure the grant exists to avoid. Dropped
|
||||
// rather than rejected: ruling 33.4, and the case at the end of this file says why.
|
||||
await expect(
|
||||
pageAsyncStorage.setItem('orca:mobileWebShellEnabled', 'true')
|
||||
).resolves.toBeUndefined()
|
||||
expect(writes).toEqual([])
|
||||
await expect(pageAsyncStorage.getItem('orca:mobileWebShellEnabled')).resolves.toBeNull()
|
||||
})
|
||||
|
||||
it('is refused, and not kept, when the shell granted no storage', async () => {
|
||||
granted = false
|
||||
const refusal = await refusalOf(pageAsyncStorage.setItem('orca:pins:host-1', '["wt-1"]'))
|
||||
expect(refusal.refusal).toBe('not-delivered')
|
||||
await expect(pageAsyncStorage.getItem('orca:pins:host-1')).resolves.toBeNull()
|
||||
})
|
||||
|
||||
it('carries each pair of a multi-write separately, and refuses the ones outside the list', async () => {
|
||||
const refusal = await refusalOf(
|
||||
it('carries each pair of a multi-write separately, and drops the ones outside the list', async () => {
|
||||
await expect(
|
||||
pageAsyncStorage.multiSet([
|
||||
['orca:pins:host-1', '["wt-1"]'],
|
||||
['orca:remotePushHostRegistrations', '{}']
|
||||
])
|
||||
)
|
||||
// The pair it could apply is applied before the batch rejects: a caller retrying the whole
|
||||
// batch after a refusal must not find the good half missing as well.
|
||||
).resolves.toBeUndefined()
|
||||
// The pair it could apply is applied: a batch that dropped the good half as well would lose a
|
||||
// write nothing was wrong with.
|
||||
expect(writes).toEqual([{ key: 'orca:pins:host-1', value: '["wt-1"]' }])
|
||||
expect(refusal.key).toBe('orca:remotePushHostRegistrations')
|
||||
})
|
||||
|
||||
it('never empties the app store, which is not this document to empty', async () => {
|
||||
@@ -122,24 +116,24 @@ describe('a write the page makes', () => {
|
||||
})
|
||||
|
||||
describe('what the page will not keep', () => {
|
||||
it("refuses another host's pinned list, so a later read cannot answer with it", async () => {
|
||||
it("drops another host's pinned list, so a later read cannot answer with it", async () => {
|
||||
publish({ 'orca:pins:host-1': '["mine"]' })
|
||||
const refusal = await refusalOf(pageAsyncStorage.setItem('orca:pins:host-2', '["theirs"]'))
|
||||
await expect(
|
||||
pageAsyncStorage.setItem('orca:pins:host-2', '["theirs"]')
|
||||
).resolves.toBeUndefined()
|
||||
// Nothing posted, and nothing cached: a value held here that the shell will not write is a pin
|
||||
// that looks set to this document and to nothing else in the app.
|
||||
expect(refusal.refusal).toBe('not-allowed')
|
||||
expect(writes).toEqual([])
|
||||
expect(await pageAsyncStorage.getItem('orca:pins:host-2')).toBeNull()
|
||||
})
|
||||
|
||||
it("refuses another workspace's chat tabs on the session route it was not opened for", async () => {
|
||||
it("drops another workspace's chat tabs on the session route it was not opened for", async () => {
|
||||
publish()
|
||||
const refusal = await refusalOf(
|
||||
await expect(
|
||||
pageAsyncStorage.setItem('orca:nativeChatTabs:host-1:wt-2', '{}')
|
||||
)
|
||||
expect(refusal.refusal).toBe('not-allowed')
|
||||
).resolves.toBeUndefined()
|
||||
expect(writes).toEqual([])
|
||||
// And the one it was opened for goes through, so the refusal above is about the workspace.
|
||||
// And the one it was opened for goes through, so the drop above is about the workspace.
|
||||
await pageAsyncStorage.setItem('orca:nativeChatTabs:host-1:wt-1', '{}')
|
||||
expect(writes).toEqual([{ key: 'orca:nativeChatTabs:host-1:wt-1', value: '{}' }])
|
||||
})
|
||||
@@ -167,10 +161,29 @@ describe('what the page will not keep', () => {
|
||||
expect(await pageAsyncStorage.getItem('orca:last-visited-worktree')).toBe(atBound)
|
||||
})
|
||||
|
||||
it('refuses a removal it may not make, rather than resolving over it', async () => {
|
||||
it('drops a write it may not make rather than rejecting, because nobody catches one', async () => {
|
||||
// Ruling 33.4. Every page-closure writer of an unlisted key calls `setItem` with no catch —
|
||||
// `notification-delivery-preferences.ts:39` awaits it inside a function its callers `void` —
|
||||
// so rejecting here turns a dropped preference into an unhandled rejection in the page. The
|
||||
// drop is the old behaviour and the right one; only the journal's oversize path rejects,
|
||||
// because the composer is written to catch that one.
|
||||
publish()
|
||||
const refusal = await refusalOf(pageAsyncStorage.removeItem('orca:pins:host-2'))
|
||||
expect(refusal.refusal).toBe('not-allowed')
|
||||
await expect(
|
||||
pageAsyncStorage.setItem('orca:notificationDeliveryPreferences', '{}')
|
||||
).resolves.toBeUndefined()
|
||||
expect(writes).toEqual([])
|
||||
expect(await pageAsyncStorage.getItem('orca:notificationDeliveryPreferences')).toBeNull()
|
||||
})
|
||||
|
||||
it('drops a removal it may not make, for the same reason', async () => {
|
||||
publish()
|
||||
await expect(pageAsyncStorage.removeItem('orca:pins:host-2')).resolves.toBeUndefined()
|
||||
expect(writes).toEqual([])
|
||||
})
|
||||
|
||||
it('drops a write the shell would not take, rather than rejecting', async () => {
|
||||
granted = false
|
||||
await expect(pageAsyncStorage.setItem('orca:pins:host-1', '["wt-1"]')).resolves.toBeUndefined()
|
||||
await expect(pageAsyncStorage.getItem('orca:pins:host-1')).resolves.toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -27,12 +27,19 @@ const REFUSAL_SENTENCES: Record<PageStorageRefusal, string> = {
|
||||
/**
|
||||
* A write the page could not make, as something a screen can put on itself.
|
||||
*
|
||||
* The real AsyncStorage rejects when its store refuses — a value over the row limit is a SQLite
|
||||
* error on Android — so rejecting is the module's own contract rather than a shape invented here,
|
||||
* and every caller that already catches a save gets the refusal for free. The one that matters is
|
||||
* the durable send journal: `mobile-structured-agent-session-send.ts` catches it and answers
|
||||
* "Message not sent" instead of sending a mutation whose operation id was never written down
|
||||
* (rulings-ota-c7.md ruling 7).
|
||||
* Raised for one refusal only, `too-large`, and that scope is the whole of ruling 33.4. The real
|
||||
* AsyncStorage rejects when its store refuses — a value over the row limit is a SQLite error on
|
||||
* Android — so rejecting is the module's own contract for a value too big, and the caller that
|
||||
* needs it is written for it: the durable send journal, whose composer catches this and answers
|
||||
* "Message not sent" rather than sending a mutation whose operation id was never written down
|
||||
* (ruling 7).
|
||||
*
|
||||
* The other two refusals stay silent drops, because nothing catches them. A page-closure writer of
|
||||
* an unlisted key calls `setItem` and awaits it with no catch —
|
||||
* `notification-delivery-preferences.ts:39` is the plain case, and `preferences.ts` has several —
|
||||
* so rejecting there converts a preference the page was never allowed to keep into an unhandled
|
||||
* rejection in the document. A write nobody may make and a write the shell would not take are both
|
||||
* the page failing to change anything, which is what it already does; the log is where they go.
|
||||
*/
|
||||
export class PageStorageRefusedError extends Error {
|
||||
readonly refusal: PageStorageRefusal
|
||||
@@ -96,17 +103,34 @@ function accept(key: string, value: string | null): PageStorageRefusal | null {
|
||||
return null
|
||||
}
|
||||
|
||||
/** One refusal, as the rejection the caller's own catch is written for. */
|
||||
/**
|
||||
* One refusal: the rejection the composer's catch is written for, or a logged drop.
|
||||
*
|
||||
* Logged and not silent, because the drop is the thing a reader of a device log has to be able to
|
||||
* find — a preference that did not stick looks identical to one nobody set.
|
||||
*/
|
||||
function settle(key: string, refusal: PageStorageRefusal | null): Promise<void> {
|
||||
return refusal === null
|
||||
? Promise.resolve()
|
||||
: Promise.reject(new PageStorageRefusedError(key, refusal))
|
||||
if (refusal === null) {
|
||||
return Promise.resolve()
|
||||
}
|
||||
if (refusal === 'too-large') {
|
||||
return Promise.reject(new PageStorageRefusedError(key, refusal))
|
||||
}
|
||||
console.warn('[page-bridge] storage-write-dropped', { key, refusal })
|
||||
return Promise.resolve()
|
||||
}
|
||||
|
||||
/** The first refusal of a batch, after every pair that could be applied has been. */
|
||||
/** The first refusal of a batch, after every pair that could be applied has been. Every dropped
|
||||
* pair is logged by `settle`; only an oversize one can reject, and it does so after the rest. */
|
||||
function settleBatch(refusals: { key: string; refusal: PageStorageRefusal }[]): Promise<void> {
|
||||
const first = refusals[0]
|
||||
return first === undefined ? Promise.resolve() : settle(first.key, first.refusal)
|
||||
let rejection: Promise<void> | null = null
|
||||
for (const entry of refusals) {
|
||||
const settled = settle(entry.key, entry.refusal)
|
||||
if (entry.refusal === 'too-large' && rejection === null) {
|
||||
rejection = settled
|
||||
}
|
||||
}
|
||||
return rejection ?? Promise.resolve()
|
||||
}
|
||||
|
||||
const pageAsyncStorage = {
|
||||
|
||||
Reference in New Issue
Block a user