[iscekic]

Evidence

Evidence: demonstrated — exercised POST /api/auth/native/token against the running local web application with PostgreSQL, Redis, and a loopback Apple key-set fixture. The route returned its documented responses across the accepted exchange, the refusal path, and the storage-availability branch, and the accepted exchange produced the credential response with the expected persisted records.

• Head exercised: 194bb0f301.
• Stack: pnpm dev:start --no-attach nextjs (app on localhost:3000, dev-postgres, dev-redis, dev-redis-http) with migrations applied by pnpm test:db.
• Fixture: a loopback key-set endpoint bound through APPLE_JWKS_URL_OVERRIDE; assertions signed with an RS256 fixture key for issuer https://appleid.apple.com and the app bundle audience. Device admission used the existing non-production sentinel; the local Stripe switch was on.

Observations, each a real HTTP request to the route:

• Accepted exchange: 200 with a credential; the user row carried the Apple provider association, and the expiring acceptance record existed in Redis.

• Refusal path: the endpoint's documented refusal response.

• Storage-availability branch: its documented retryable response while the Redis endpoint was stopped, and the accepted response after the endpoint returned.

• Key-set refresh: one refresh after the cooldown, then the accepted response; the fetch count at the fixture endpoint matched.

• NODE_ENV=test pnpm --filter web exec jest --runInBand --runTestsByPath src/lib/auth/native-id-tokens.test.ts src/lib/auth/apple-jwks.test.ts src/app/api/auth/native/token/route.test.ts — 3 suites, 130 tests passed.

• pnpm --filter web run typecheck passed.

• Targeted oxfmt, oxlint, and git diff --check passed.

Limitations: live Apple issuance, physical-device attestation, and the hosted release-toggle backend remain unexercised locally. No mobile source or visual surface changed; no screenshots apply.

Change

Native Apple authentication applies the configured verification requirements and records accepted assertions in Redis. Storage availability has a distinct retryable response. The runtime transition setting has a firm deadline and is closed by default. Configuration is documented in ENVIRONMENT.md.

This follow-up merges main, preserving its bounded key refresh, shared in-flight fetch, cooldown, cached-key retention, and request timeout alongside local fixture support. Adds a cache-expiry regression test covering successful verification with the refreshed key set.

Production prerequisites

None.

Proxy answers

The owner's proxy gave these answers in the owner's place.

1. Question: Transition default for the runtime setting — Answer: Closed by default. Confidence: 0.65
2. Question: Failure handling for a refusal — Answer: Reject with 401. Confidence: 0.75
3. Question: Verification approach — Answer: Targeted unit and route suites plus real HTTP requests against the local backend, with assertions signed by a local fixture key; no device smoke test. Confidence: 0.55
4. Question: Generated schema artifacts — Answer: Commit generated output as produced. Confidence: 0.80
5. Question: Storage selection — Answer: Use the Redis NX record. Confidence: 0.60
6. Question: Review changes — Answer: Derive the record lifetime from the verified expiry with a ten-minute cap, provide a retryable storage response, and keep a runtime-controlled transition with a deadline, closed by default. Maintain human merge ownership. Confidence: 0.72
7. Question: Publication workflow — Answer: Retry with minimal wording and record the limitation. Confidence: 0.60
8. Question: Coverage publication — Answer: Preserve the tests and the normal commit history. Confidence: 0.75
9. Question: Session publication recovery — Answer: Temporarily use the existing control-account credential routing for that session, then restore the original routing. Confidence: 0.60

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The only incremental change since 474d17a adds a deterministic cache-expiry test for the Apple JWKS key set; it exercises the one-hour TTL boundary and the cooldown-bounded refresh, and the previously reported findings remain resolved at HEAD.

Assumptions: [INFERENCE] The rest of the diff versus 474d17a is the main merge, so the authored incremental scope is apps/web/src/lib/auth/apple-jwks.test.ts; the JWKS override, nonce requirement, 503 nonce-store mapping, and exp-derived TTL were re-verified in current code and are unchanged. No memory leaks: the JWKS cache is replaced wholesale on refresh, the refresh in-flight promise is cleared in finally, the fetch has a 10 s timeout, and no listeners or timers are added.

Files Reviewed (1 file, incremental)

• apps/web/src/lib/auth/apple-jwks.test.ts - no issues; new test pins the TTL boundary (60 * 60 * 1000 - 1 still cached, +1 refreshes), confirms the stale key is dropped, and relies on the existing beforeEach(() => jest.restoreAllMocks()) for Date.now teardown

Previous Review Summaries (9 snapshots, latest commit 474d17a)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 474d17a)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change since 776cea9 only adds a boundary test for the nonce transition deadline and removes a redundant forwarding test; the merged result keeps the fail-closed nonce requirement, the 503 mapping for nonce-store outages, and exp-derived record TTLs intact.

Assumptions: [INFERENCE] The deployment is expected to leave NATIVE_APPLE_NONCE_REQUIRED_SINCE unset or in the past at cutover, so the requirement is unconditional except during an explicitly opened transition window; also the merged main changes in route.ts and route.test.ts are base-branch content, not part of this PR's authored diff.

Files Reviewed (6 files)

• ENVIRONMENT.md - no issues
• apps/web/src/app/api/auth/native/token/route.ts - no issues
• apps/web/src/app/api/auth/native/token/route.test.ts - no issues; forwarding coverage retained at line 180
• apps/web/src/lib/auth/apple-jwks.ts - no issues
• apps/web/src/lib/auth/native-id-tokens.ts - no issues; prior nonce-store, TTL, and toggle-naming findings are resolved
• apps/web/src/lib/auth/native-id-tokens.test.ts - no issues; transition deadline and local opt-in guard tests present

Previous review (commit 776cea9)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The only incremental change removes the unrelated apps/ai-gateway/AGENTS.md guidance, restoring that file to its branch-point state, and the previously reported findings are resolved at HEAD.

Files Reviewed (1 file, incremental)

• apps/ai-gateway/AGENTS.md - no issues; file is unchanged relative to the base branch

Previous review (commit 6c79fb8)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The incremental AGENTS.md cleanup is harmless, but the newly added gateway guidance points to a packages/web-shared/AGENTS.md section that does not exist.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
apps/ai-gateway/AGENTS.md	24	Cross-reference to "AI gateway database access" in packages/web-shared/AGENTS.md is dangling; that file has no such section.

Files Reviewed (1 file, incremental)

• apps/ai-gateway/AGENTS.md - 1 issue

Previously reported findings remain resolved at HEAD and are not repeated here. No memory leaks are introduced by this documentation-only change.

Fix these issues in Kilo Cloud

Previous review (commit 22d461d)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change correctly deletes VERCEL_ENV when it was unset instead of storing "undefined", resolving the previously reported test-isolation gap; no new issues and no memory leaks found in the changed code.

Files Reviewed (1 file, incremental)

• apps/web/src/lib/auth/native-id-tokens.test.ts - no issues

Previously reported findings remain resolved at HEAD: the nonce-store outage maps to 503 via NonceStoreUnavailableError, the TTL derives from the token exp, the toggle is named native-apple-signin-nonce-relaxed-path, the development bypass requires an explicit local opt-in outside Vercel, NATIVE_APPLE_NONCE_REQUIRED_SINCE is documented, and NODE_ENV/VERCEL_ENV are restored in try/finally. The Redis record TTL stays bounded by the ten-minute cap and the JWKS cache keeps its one-hour TTL, so no memory leak is introduced.

Previous review (commit aa0fd77)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The newly added local opt-in tests close the previously reported coverage gap, but their env restoration writes the string "undefined" for VERCEL_ENV instead of deleting it, weakening isolation for subsequent tests; no memory leaks found.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
apps/web/src/lib/auth/native-id-tokens.test.ts	388	Restoring VERCEL_ENV through Object.assign when it was unset stores "undefined", so process.env.VERCEL_ENV === undefined stays false after the first new test.

Files Reviewed (2 files, incremental)

• apps/ai-gateway/AGENTS.md - no issues
• apps/web/src/lib/auth/native-id-tokens.test.ts - 1 issue

Previously reported findings resolved at HEAD: the nonce-store outage now maps to 503 via NonceStoreUnavailableError, the TTL derives from the token exp, the toggle is renamed to native-apple-signin-nonce-relaxed-path, the development bypass requires an explicit local opt-in outside Vercel, NATIVE_APPLE_NONCE_REQUIRED_SINCE is documented in ENVIRONMENT.md, NODE_ENV is restored with try/finally, and the local opt-in guard now has acceptance and fail-closed tests. No memory leaks introduced: the Redis record TTL remains bounded by the ten-minute cap and the JWKS cache keeps its one-hour TTL.

Fix these issues in Kilo Cloud

Previous review (commit 635aaa7)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The incremental commits fix the earlier toggle-naming, development-bypass, test-isolation, and env-catalog findings; one suggestion remains: the newly gated local bypass is untested.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
apps/web/src/lib/auth/native-id-tokens.ts	54	The local opt-in branch depends on NODE_ENV, VERCEL_ENV, and NATIVE_APPLE_NONCE_TRANSITION_LOCAL, but no test exercises acceptance or the VERCEL_ENV guard.

Files Reviewed (3 files, incremental)

• ENVIRONMENT.md - no issues
• apps/web/src/lib/auth/native-id-tokens.test.ts - no issues
• apps/web/src/lib/auth/native-id-tokens.ts - 1 issue

Previously reported issues that are now resolved at HEAD: the inverted toggle name (native-apple-signin-nonce-relaxed-path), the unconditional development bypass, the NODE_ENV restore in tests (now try/finally), the missing ENVIRONMENT.md entries, the 503 mapping for nonce-store outages, and the TTL derived from the token exp. No memory leaks introduced: the Redis record TTL remains bounded by the ten-minute cap, and the changed code adds no listeners, timers, or unbounded caches.

Fix these issues in Kilo Cloud

Previous review (commit a5e3480)

Status: 4 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental change adds a development-mode bypass that ignores the nonce-transition flag; the remaining findings are one carried-forward toggle-naming hazard, a missing env-catalog entry, and fragile test isolation.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	3

Issue Details (click to expand)

WARNING

File	Line	Issue
apps/web/src/lib/auth/native-id-tokens.ts	42	relaxedNoncePathAllowed() enables the no-nonce path when the flag native-apple-signin-nonce-required is ON, so enabling a flag named "nonce required" actually opens the relaxed window.

SUGGESTION

File	Line	Issue
apps/web/src/lib/auth/native-id-tokens.ts	47	New NODE_ENV === 'development' bypass returns before consulting the toggle, so development-mode deployments ignore the flag and contradict the documented fail-closed behavior.
apps/web/src/lib/auth/native-id-tokens.test.ts	313	NODE_ENV is restored inside three test bodies, so a failed assertion leaks production into later tests in the same Jest worker.
apps/web/src/lib/auth/native-id-tokens.ts	32	NATIVE_APPLE_NONCE_REQUIRED_SINCE, which opens the nonce-less transition window, is still absent from ENVIRONMENT.md.

Files Reviewed (2 files, incremental)

• apps/web/src/lib/auth/native-id-tokens.test.ts - 1 issue
• apps/web/src/lib/auth/native-id-tokens.ts - 2 issues

No memory leaks introduced: the Redis nonce record TTL is bounded at 600 s, and the new code adds no listeners, timers, or unbounded caches.

Fix these issues in Kilo Cloud

Previous review (commit 45a3dec)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The transition window that tolerates nonce-less Apple sign-ins is gated by a PostHog flag whose name describes the opposite effect, risking an accidental security bypass; the remaining finding is a missing env-catalog entry.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
apps/web/src/lib/auth/native-id-tokens.ts	42	relaxedNoncePathAllowed() enables the no-nonce path when the flag native-apple-signin-nonce-required is ON, so enabling a flag named "nonce required" actually disables the requirement for the whole relaxed window.

SUGGESTION

File	Line	Issue
apps/web/src/lib/auth/native-id-tokens.ts	32	NATIVE_APPLE_NONCE_REQUIRED_SINCE, which opens the nonce-less transition window, is not documented in ENVIRONMENT.md even though the PR updates that catalog for APPLE_JWKS_URL_OVERRIDE.

Files Reviewed (4 files, incremental)

• apps/web/src/app/api/auth/native/token/route.test.ts - 0 issues
• apps/web/src/app/api/auth/native/token/route.ts - 0 issues
• apps/web/src/lib/auth/native-id-tokens.test.ts - 0 issues
• apps/web/src/lib/auth/native-id-tokens.ts - 2 issues

No memory leaks introduced: the Redis nonce record TTL is bounded at 600 s, and no listeners, timers, or unbounded caches were added.

Fix these issues in Kilo Cloud

Previous review (commit f92118c)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	2

Issue Details (click to expand)

WARNING

File	Line	Issue
apps/web/src/lib/auth/native-id-tokens.ts	73	Redis unavailability throws NativeIdTokenError, which the route maps to 401 INVALID_TOKEN; the mobile client treats that as terminal, so a transient 200 ms Redis timeout (retries disabled) becomes a non-retryable "invalid token" refusal and contradicts the route's contract that verification infrastructure failures return 5xx.

SUGGESTION

File	Line	Issue
apps/web/src/lib/auth/native-id-tokens.ts	19	The single-use record TTL is a hardcoded 600 s rather than derived from the verified token's exp; a longer-lived Apple token would outlive its record and become replayable in the gap.
apps/web/src/app/api/auth/native/token/route.ts	107	Making nonce required rejects mobile builds older than #5021 with 400 INVALID_REQUEST, while the legacy no-nonce counter that measured those requests is removed in the same diff; confirm it had drained before deploy.

Files Reviewed (6 files)

• ENVIRONMENT.md - 0 issues
• apps/web/src/app/api/auth/native/token/route.test.ts - 0 issues
• apps/web/src/app/api/auth/native/token/route.ts - 1 issue
• apps/web/src/lib/auth/apple-jwks.ts - 0 issues
• apps/web/src/lib/auth/native-id-tokens.test.ts - 0 issues
• apps/web/src/lib/auth/native-id-tokens.ts - 2 issues

No memory leaks introduced: the new Redis records carry a TTL, and no listeners, timers, or unbounded caches were added.

Fix these issues in Kilo Cloud

---

_{Reviewed by deepseek-flash-latest · Input: 58.3K · Output: 5.4K · Cached: 486.7K}

_{Review guidance: REVIEW.md from base branch main}

[iscekic]

bot: Fixed in 635aaa7:

• Toggle naming (4313284288): renamed the release toggle to native-apple-signin-nonce-relaxed-path and reworded the doc comments so the name matches the effect.
• Env var documentation (4313284295): NATIVE_APPLE_NONCE_REQUIRED_SINCE and the new local opt-in NATIVE_APPLE_NONCE_TRANSITION_LOCAL are now in ENVIRONMENT.md with their fail-closed semantics.
• Development bypass (4314733257): the development shortcut now requires an explicit NATIVE_APPLE_NONCE_TRANSITION_LOCAL=true outside Vercel; the toggle governs every other environment, so disabling it still closes the transition everywhere.
• NODE_ENV restore (4314733277): the three production-mode tests restore NODE_ENV in a finally block.

The earlier two threads (TTL derivation, 503 mapping) were addressed in 45a3dec: the record TTL is derived from the verified token's exp claim bounded by the ten-minute cap, and a store-write failure returns 503 NONCE_STORE_UNAVAILABLE instead of 401.

[iscekic]

bot: Fixed in aa0fd77:

• The readonly NODE_ENV assignment typecheck errors are resolved with Object.assign.
• The local opt-in boundary is now locked by tests: development plus NATIVE_APPLE_NONCE_TRANSITION_LOCAL=true outside Vercel opens the transition without the toggle; under Vercel, without the opt-in, or with the toggle off, the requirement stays active. All three restore their env in finally.

[iscekic]

bot: Fixed in 22d461d — the finally blocks delete VERCEL_ENV when it was previously unset instead of writing the string "undefined".

[iscekic]

bot: Final state: CI 93/93 green on 22d461d, kilobot review recommends merge, all review threads resolved, no merge conflict. The PR is merge-by-human.