Repository navigation
refactor(web): replace global aria state with app resources and Foldkit overlays - #338
Confidence 5/10 · 1 issue to address
🟡 Confidence 5/10 · needs attention
879 files: backend auth/outbox and most web hunks were read only through child reviewers, two of which stopped before answering, and 227 files went unread.
quality 90/100 · 1 warning · tests partial · risk high · 0/1 new units observable · 227 files not read
Refactors web state, overlays and backend auth/failure handling across 879 files; one real observability gap in the new session-failure path. 227 files, notably chat-sync-core-worker.ts and the backend route and test files, were not read. The open HttpClientError issues all point at services/common/atom-client.ts, which this diff deletes, so those stacks go away on deploy rather than being fixed.
verifyandloadUserinhazel-session.tsmap failures toSessionLoadErrorand log themServerFailureReporterLivelogs RPC/HttpApi 5xx tags with their `Cause- Outbox handlers propagate retryable failures instead of logging them
- Web overlays move to
@foldkit/uiDialog/Popover/Tooltip/Toast; Atom dropped
Before merge
- Manual · Land this as a reviewable series; 879 files is past every reviewer and CI limit
Findings
🟠 Warning · F1 · verify drops the cause of a session-store failure and the reporter excludes its tag
observability · STAT-01 · apps/backend/src/services/hazel-session.ts:80-86
When auth.verifySession fails for any reason other than SessionInvalid, the new mapping keeps only error._tag in detail and produces a SessionLoadError with no cause, and apps/backend/src/lib/error-reporter.ts:6 deliberately skips every SessionLoadError on the premise that the raise sites log it. The DatabaseError site at line 35 does log; this one does not, so a broken session store answers every browser and native request with a 503 and leaves no log or trace of why. Log the failure here too (Effect.tapError((error) => Effect.logError("Session verification failed", Cause.fail(error))) before the mapError) or carry it as the error's cause.
Production impact
Open errors in the changed files
| Issue | Service | Occurrences | File |
|---|---|---|---|
| HttpClientError | maple-web | 27 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
| HttpClientError | maple-web | 12 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
| HttpClientError | maple-web | 9 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
| HttpClientError | maple-web | 9 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
| HttpClientError | maple-web | 8 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
| HttpClientError | maple-web | 8 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
| HttpClientError | maple-web | 4 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
| HttpClientError | maple-web | 4 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
| HttpClientError | maple-web | 4 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
| HttpClientError | maple-web | 3 | apps/web-foldkit/src/core/lib/services/common/atom-client.ts |
After this merges, Maple checks whether they stop.
Production traffic of the changed files (last 7 days)
| File | Calls/day | Busiest operations |
|---|---|---|
apps/backend/src/routes/webhooks.http.ts |
974 | POST 830/day, 4.0% errPOST 144/day, 0.5% err |
apps/web-foldkit/src/core/lib/access-token.ts |
974 | POST 830/day, 4.0% errPOST 144/day, 0.5% err |
apps/web-foldkit/src/composer/upload.ts |
7 | PUT 7/day, 0.0% err |
Telemetry this change adds and removes (1)
- ➕ span name
MessageSideEffectService.nameThreadIfDue·apps/backend/src/services/message-side-effect-service.ts:110
What was checked
access-token.tsexchange keeps its attempt bound (Effect.retrytimes 2) and generation guard- Token retry only re-sends the idempotent
POST /auth/tokenexchange - Backend RPC handlers group (bots, channels, connect-shares, organizations): no dropped auth or org check found
Observability coverage: 0 of 1 changes observable
| Change | Kind | Observable | Evidence |
|---|---|---|---|
| POST /auth/token token exchange wrapper in access-token.ts | outbound-http | no | Runs via Effect.runPromise from fetch wrappers outside any runtime; no Client span, MAP-01 |
Files not reviewed (227)
The review ended before it read these diffs, so nothing above vouches for them.
.oxlintrc.jsonapps/backend/.artifacts/chat-sync/chat-sync-diagnostics.jsonlapps/backend/src/lib/error-reporter.test.tsapps/backend/src/lib/policy-utils.test.tsapps/backend/src/routes/api-v1/integrations.http.tsapps/backend/src/routes/api-v1/messages.http.tsapps/backend/src/routes/bot-commands.http.tsapps/backend/src/routes/integration-commands.http.tsapps/backend/src/routes/integration-resources.http.tsapps/backend/src/rpc/middleware/auth-class.tsapps/backend/src/rpc/middleware/auth.test.tsapps/backend/src/services/chat-sync/chat-sync-core-worker.integration.test.tsapps/backend/src/services/chat-sync/chat-sync-core-worker.tsapps/backend/src/services/chat-sync/discord-gateway-service.tsapps/backend/src/services/chat-sync/discord-sync-worker.test.tsapps/backend/src/services/hazel-auth.test.tsapps/backend/src/services/invitations.test.tsapps/backend/src/services/message-side-effect-service.test.tsapps/backend/src/services/org-resolver.test.tsapps/backend/src/services/session-manager.tsapps/electric-proxy/resources.tsapps/web-foldkit/package.jsonapps/web-foldkit/public/images/appearance/dark.svgapps/web-foldkit/public/images/appearance/light.svgapps/web-foldkit/public/images/appearance/system.svgapps/web-foldkit/src/app/desktop.test.tsapps/web-foldkit/src/auth/flows.test.tsapps/web-foldkit/src/chat/embeds/url-embeds.tsapps/web-foldkit/src/chat/image-viewer.tsapps/web-foldkit/src/chat/markdown/leaves.test.ts- and 197 more
d620f82 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one. Check ids refer to Maple's instrumentation audit.
Annotations
Check warning on line 86 in apps/backend/src/services/hazel-session.ts
maple-review-bot / Maple / review
observability · STAT-01: `verify` drops the cause of a session-store failure and the reporter excludes its tag
When `auth.verifySession` fails for any reason other than `SessionInvalid`, the new mapping keeps only `error._tag` in `detail` and produces a `SessionLoadError` with no `cause`, and `apps/backend/src/lib/error-reporter.ts:6` deliberately skips every `SessionLoadError` on the premise that the raise sites log it. The `DatabaseError` site at line 35 does log; this one does not, so a broken session store answers every browser and native request with a 503 and leaves no log or trace of why. Log the failure here too (`Effect.tapError((error) => Effect.logError("Session verification failed", Cause.fail(error)))` before the `mapError`) or carry it as the error's `cause`.