[NO-JIRA] Add App State SDK - #75
Conversation
size-limit report 📦
|
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aeddf0d659
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 498b9c1074
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80e7d4e97b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf095d097e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0aaf0d489
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
🟡 Changes recommended
Several moderate reconciliation and error-classification issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a typed App State SDK for public and server clients, including HTTP errors, transactional reconciliation, account isolation, and shared exports.
Changes:
- Adds App State transport methods, types, and client facades.
- Adds immutable state management, reconciliation, and account switching.
- Adds tests, conformance fixtures, documentation, and release metadata.
File summaries
| File | Description |
|---|---|
packages/api/test/smoke/app-state-package.cjs |
Package entrypoint smoke checks |
packages/api/test/http-error.test.ts |
HTTP error behavior tests |
packages/api/test/app-state.test.ts |
App State operation tests |
packages/api/test/app-state-types.test.ts |
Public and server type-surface tests |
packages/api/test/app-state-state.test.ts |
Immutable state transition tests |
packages/api/test/app-state-reconciler.test.ts |
Reconciliation and account-switching tests |
packages/api/test/app-state-conformance.test.ts |
Shared conformance tests |
packages/api/src/types/quran-client.ts |
Request metadata options |
packages/api/src/types/index.ts |
App State type exports |
packages/api/src/types/app-state.ts |
App State contracts |
packages/api/src/server.ts |
Server API exports |
packages/api/src/sdk/public-fetcher.ts |
Public response and error handling |
packages/api/src/sdk/http-error.ts |
Shared HTTP error implementation |
packages/api/src/sdk/fetcher.ts |
Server response and error handling |
packages/api/src/sdk/app-state-errors.ts |
App State error classification |
packages/api/src/runtime/create-public-client.ts |
Public client facade |
packages/api/src/runtime/create-client.ts |
Server client facade |
packages/api/src/runtime/app-state.ts |
App State transport operations |
packages/api/src/runtime/app-state-state.ts |
Account state and mutation helpers |
packages/api/src/runtime/app-state-reconciler.ts |
Transactional reconciliation |
packages/api/src/public.ts |
Public exports |
packages/api/src/index.ts |
Root exports |
packages/api/README.md |
App State documentation |
packages/api/conformance/app-state-reconciliation-v1.sha256 |
Conformance fixture checksum |
packages/api/conformance/app-state-reconciliation-v1.json |
Conformance scenarios |
.changeset/add-app-state-sdk.md |
Minor release metadata |
Review details
Suppressed comments (4)
packages/api/src/runtime/app-state-reconciler.ts:338
- This catch classifies the result of an in-flight
getDocument()before checking whether the reconciliation generation is still current. If the account switches while the refresh is pending, a stale non-document_not_founderror is rethrown and rejects the old reconciliation instead of being discarded; even recognized stale errors are inspected after the account boundary has changed. CheckisCurrent(reconcileContext)first in this catch and return without classifying stale failures.
} catch (error) {
if (getAppStateErrorCode(error) !== "document_not_found") throw error;
current = null;
}
packages/api/src/runtime/app-state-reconciler.ts:499
reconcileQueueis shared across account generations and is not reset byswitchAccount. If an old-account transport request hangs, every reconcile started after switching to the new account remains queued behind that request and never uses the new account's transport, despite the generation checks making the old result safe to discard. Serialize within a generation (or otherwise release the queue on switch) so a stalled stale request cannot block synchronization for the active account.
reconcile: () => {
const capturedContext = context();
const operation = reconcileQueue.then(() =>
reconcileContext(capturedContext),
);
reconcileQueue = operation.then(
() => undefined,
() => undefined,
);
packages/api/src/runtime/app-state-reconciler.ts:480
- This return reads
activeAccountIdwithout binding the final state read tomutationContext. If account A's queued delete completes and itsgetStateFortransaction is still pending whenswitchAccountchanges the active account to B, the delete promise can resolve with A's view after B is active. Guard the returned snapshot by the captured generation (or re-read the current account before resolving) so a late result cannot expose the previous account's state.
return getStateFor(activeAccountId);
packages/api/test/app-state-conformance.test.ts:83
- The conformance test only executes
duplicate_change_is_idempotent,out_of_order_pages_keep_newest_document, and the same-version conflict case. The fixture also defines strict-conflict rebase, crash-atomicity, account-switch, and tombstone-lineage scenarios, but their expected results are never consumed by the JS conformance suite, so the claimed shared conformance can silently diverge on those cases. Add assertions/runners for the remaining executable scenarios (or narrow the fixture claim).
it.each([
"duplicate_change_is_idempotent",
"out_of_order_pages_keep_newest_document",
])("matches the %s state transition", async (scenarioName) => {
- Files reviewed: 26/26 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
Bootstrap recovery, stale conflict handling, and transient mutation retry paths do not fully satisfy the documented reconciliation contract.
Review details
Suppressed comments (4)
Previously missed (4) — in code that hasn't changed since the last review.
packages/api/src/runtime/app-state-reconciler.ts:176
- A 410 returned by
bootstrap()is rethrown here before the staging state is reset or the bounded restart loop is entered. The App State recovery contract allows bootstrap itself to returnbootstrap_required/sync_token_expiredand requires discarding partial staging and restarting, so an initial or recovery bootstrap can fail permanently instead of recovering. Handle recovery-coded bootstrap failures through the same reset/restart path as the change-drain failures.
packages/api/src/runtime/app-state-reconciler.ts:337 - This catch classifies the refresh error before checking whether the reconciliation context is still current. If the account switches while
getDocument()is pending and the old transport then rejects with anything other thandocument_not_found, the stale reconciliation rejects with that old error instead of discarding it. CheckisCurrent(reconcileContext)first and returnnullfor stale refresh failures, matching the stale-response boundary used by the other network calls.
packages/api/src/runtime/app-state-reconciler.ts:432 - All non-412 replay failures take this immediate throw path, so a transient 500/503, transport failure, or 429 with
Retry-Afterleaves reconciliation rejected without the bounded retry/backoff required for ambiguous mutations. Add a retry path that preserves the captured body, preconditions, and idempotency key and honorsRetry-After, otherwise a lost mutation response can force callers to retry manually and may cause unbounded hammering.
packages/api/src/sdk/app-state-errors.ts:46 - The App State contract permits JSON error envelopes without a
detailsmember, but this exported payload type requires bothdetailsanddetails.error. That makes valid 4xx/5xx payloads unrepresentable and forces consumers to assert a shape the wire contract does not guarantee; modeldetails/its nested error as optional and narrow before reading them.
- Files reviewed: 27/27 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
Adds a typed App State SDK surface for both public and server JavaScript clients, including transport-safe errors and transactional reconciliation.
Key changes
switchAccount(accountId, transport)to the target account's credential-bound transport; in-flight work retains its original transport and stale results cannot commit after a switch.@quranjs/api/publicand@quranjs/api/server.QuranHttpErrorso callers can distinguish410recovery from409and412conflicts while retaining response payloads and headers.Validation
At exact head
034f5ac1058d1c9528b3cda24e4f8f65a97d561d:Compatibility
Environment variables
Related rollout
NO-LAUNCH: keep this PR in draft and do not publish the App State SDK surface until the coordinated backend rollout, deployed-prelive acceptance, and required lifecycle/security/privacy/trust-and-safety approvals are complete.