Skip to content

fix(platform): feedback reach, OAuth renewals, unique slots, one 429 - #3197

Merged
larryro merged 2 commits into
mainfrom
fix/followup-authz-consistency
Sep 4, 2026
Merged

fix(platform): feedback reach, OAuth renewals, unique slots, one 429#3197
larryro merged 2 commits into
mainfrom
fix/followup-authz-consistency

Conversation

@larryro

@larryro larryro commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Five verified follow-up defects from the backend review, each fixed at its root with a colocated unit test and a real-Postgres probe. Base: origin/main @ 1ea4cc9e9.

Per-finding outcome

# Finding Outcome Evidence
1 POST /feedback never verifies messageId/threadId belong to the caller's org fixed Service now requires the message to exist in the caller's org + thread and the thread to be the caller's own or project-shared (loadOwnedThread / loadProjectSharedThread, the chat surface's own read grants); anything else is one opaque 404 MESSAGE_NOT_FOUND. Red on base: itest probe answered 200 and stored the rival's row (items=2).
2 A second Slack workspace connect (and Reconnect) always fails with storage_failed fixed (still live after open #3189, which keeps name: displayName) storeOauth2Grant: a workspace the team route already maps to this org RENEWS its credential (tokens rotated, active, detail cleared); a no-workspace connector renews its one/default oauth2 credential; a new workspace gets a unique label — Slack (Second Workspace) from the vendor's team.name (now parsed), or Slack (2). Red on base: itest reconnecting … → error/storage_failed, second workspace → error/storage_failed.
3 app.messages lacks UNIQUE (thread_id, order, step_order) fixed Migration 0074 (0071–0073 belong to open #3191): dedup first, then CREATE UNIQUE INDEX messages_thread_slot, drop the redundant plain index. Appenders use INSERT … ON CONFLICT DO NOTHING RETURNING + bounded re-claim. Red on base: 12 concurrent appends → 5 distinct orders (ties). Branch: 12/12 distinct, contiguous.
4 Twelve doors hand-roll divergent RateLimitExceededError → 429 shapes fixed (#3186's rateLimitedResponse is NOT on base — its file is added here verbatim as a superset) lib/rate-limit-response.ts: rateLimitedResponse (Hono), rateLimitedPlainResponse (Slack webhook, SSE auth), rateLimitExceededCause (documents/TTS wrappers carry the refusal as cause). Every refusal is 429 {error:'RATE_LIMITED', data:{retryAfterMs}} + whole-second Retry-After ≥ 1. Red on base: 5/6 doors had no Retry-After, the 6th the wrong body.
5a .env.example describes the 0.5 backend as an opt-in parallel build with Convex present fixed Rewrote the backend block (always on, BACKEND_UPSTREAM = proxy upstream override only, TALE_CONTROL_TOKEN semantics), removed POSTGRES_URL/INSTANCE_NAME (read nowhere), corrected INSTANCE_SECRET's real role (WebDAV HMAC key derivation), /metrics/backend, the TALE_PROVIDER_KEY_ name rule (1–64 chars, no 40-char env-sync cap), "convex image" → platform image.
5b generateDevCompose emits depends_on: object-store with no such service fixed tale dev needs blobs (the backend seeds the default blob connection at boot and refuses uploads without it; DEV_VOLUME_NAMES already carried object-store-data) → the dev stack now defines object-store via the shared createObjectStorageService. Red on base: new generator test fails with backend-api depends on object-store, which the dev compose does not define.

Intentionally untouched RateLimitExceededError sites (protocol adapters, not app 429 shapes): auth/auth.ts (Better Auth APIError TOO_MANY_REQUESTS), webdav/handlers.ts (WebDAV AppError consumed by the DAV lib), documents/service.ts#retryRagIndexingForDocument (an RPC {success:false} soft result).

Messages unique key + dedup

Key: (thread_id, "order", step_order) — exactly the ordering readers sort by; every appender writes step_order = 0 at max(order)+1, branch/arena copies preserve both columns into a fresh thread, and nothing legitimately shares a slot. Dedup (same migration, before the index): within every (thread_id, "order") group holding a tie, rows are renumbered 0..n-1 by (step_order, created_at_ms, id) — the order readers already observe, deterministic, no row deleted, untied groups untouched (replayed on a scratch table in the itest). Rolling-safe: the old image's appenders carry no ON CONFLICT, so a lost race during the roll answers an error for one send instead of writing a tie. Under SERIALIZABLE, ON CONFLICT raises 40001 and transactSerializable reruns the transaction; under READ COMMITTED the bounded loop re-claims the next slot.

Tests

  • vitest (server project), all new, red on base (19 failing tests + 1 unresolvable module): feedback/access.test.ts (4), feedback/routes.access.test.ts (1), connectors/oauth.test.ts (8), threads/store.test.ts (3), chat/append-message.test.ts (3), lib/rate-limit-response.test.ts (7); token_exchange.test.ts expectation extended for teamName.
  • bun test (CLI): generate-dev-compose.test.ts +2 (red on base: 2 fail).
  • integration-check: +16 probes (feedback ×2 incl. the rewritten upsert probe, slots ×2, 429 ×6, OAuth ×3 + 1 expectation), all green on branch; red on base as listed above.

Verification (observed)

  • services/platform: tsc --noEmit clean; oxlint --type-aware clean; vitest server project 5580 pass.
  • tools/cli: tsc clean, oxlint clean, bun test 313 pass / 18 skip.
  • backend:integration, throwaway tale-db + MinIO per run: branch 187 PASS / 7 FAIL, base 171 PASS / 7 FAIL — the 7 are identical base-environment failures (see discoveries), and both runs abort at the same point. The batch's own 16 probes: 16/16 on branch; feedback/slots/429/OAuth probes red on base code.
  • Compose: read-only validated through the generator test (parsed YAML: every depends_on target defined; object-store mounts the declared object-store-data volume).

Cross-class discoveries (not fixed here)

  1. Tip-of-main itest defectcheckTwoFactor's third probe (fix(platform): gate conversation writes and audit 2FA successes #3187) runs two-factor/enable→disable on the suite's MAIN user; Better Auth drops that session on disable (auth.api.getSession → null → requireSession 401), so every later probe fails (chat memories/composer/deferred ERR/401, automations trigger mint → "404 Not Found" parsed as JSON → fatal abort). Reproduced standalone: GET /api/app/chat/memories 200 → enable → 200 → disable → 401. Fix belongs to that probe (re-sign-in, refresh ctx.cookie) — it currently hides ~200 later checks, including checkConnectorOauth (verified here by temporarily reordering, not committed).
  2. fix(platform): gate conversation writes and audit 2FA successes #3187 × fix(platform): blob refs grant nothing; upload and import lanes gated #3160 conflict on main — the editor-or-above write gate on POST /api/app/conversations/compose now fires before fix(platform): blob refs grant nothing; upload and import lanes gated #3160's probe expects 403 attachment_not_owned (compose → 403 FORBIDDEN).
  3. Vite fs allow-list in worktrees — three app/routes/*.test.tsx fail to load (Denied ID …/tale/node_modules/@fontsource/...woff2) whenever node_modules is a symlink to another clone; identical on base.
  4. Docs: docs/en/self-hosted/configuration/environment-reference.md:45 still says INSTANCE_SECRET derives "the deploy admin key" (Convex-era); needs the same correction as .env.example in all three locales.

Merge notes (open siblings)

Squash of the wt-3 batch for rebase onto main; the PR body carries the per-finding record.
…harness

After the rebase onto #3189 and the renumbering to 0076, two probes drifted: the metadata-forgery probe voted on ids no message row carries, which the new reach gate refuses before any write, and the slot-migration probe still read the 0074 file. Both now use the lane's own message fixtures and the renumbered file.
@larryro
larryro force-pushed the fix/followup-authz-consistency branch from f55c516 to 312186d Compare September 4, 2026 04:10
@larryro
larryro marked this pull request as ready for review September 4, 2026 04:10
@larryro
larryro merged commit b978e90 into main Sep 4, 2026
37 of 38 checks passed
@larryro
larryro deleted the fix/followup-authz-consistency branch September 4, 2026 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant