Skip to content

Add guarded API token refresh handling#9

Open
Soengkit wants to merge 3 commits into
9904099:mainfrom
Soengkit:codex/guarded-token-refresh-2
Open

Add guarded API token refresh handling#9
Soengkit wants to merge 3 commits into
9904099:mainfrom
Soengkit:codex/guarded-token-refresh-2

Conversation

@Soengkit

@Soengkit Soengkit commented Jun 21, 2026

Copy link
Copy Markdown

Summary

Fixes #2.

Adds guarded token refresh handling to frontend/src/services/api.ts so concurrent 401/403 responses share one refresh operation, successful refresh retries the original request once, and failed refresh clears local auth state with a typed authentication error.

Changes

  • Added single-flight refresh handling around /auth/refresh.
  • Retries the original request exactly once after a successful refresh while rebuilding headers from the new token.
  • Avoids recursive refresh loops when /auth/refresh itself returns 401/403.
  • Clears access token, refresh token, stored token bundle, and cached user data when refresh fails.
  • Added executable validation fixture frontend/src/services/api_refresh.validation.ts and npm run test:api-refresh.
  • Fixed a one-line build.py f-string syntax error that prevented the diagnostic script from parsing/running.
  • Committed focused diagnostic artifacts for source commit 600e1d20:
    • diagnostic/build-600e1d20.logd
    • diagnostic/build-600e1d20.json

Testing

  • cd frontend && npm install passed with 0 vulnerabilities.
  • cd frontend && npm run test:api-refresh passed.
  • cd frontend && npm run build passed.
  • git diff --check origin/main...HEAD passed.
  • python3 -m py_compile build.py passed.
  • python3 build.py -m frontend generated the focused diagnostic bundle with 1/1 frontend module passing and 0 failures.
  • gitleaks git --log-opts="origin/main..HEAD" reported no leaks for the PR range.

Scope note

The Haskell/OpenAPI shim files, frailbox portability changes, stale failed diagnostic bundle, and repository-wide diagnostic support commits were removed from this PR history. The only remaining build.py change is the one-line syntax fix needed for the diagnostic script to run.

Checklist

  • Relevant modules affected by these changes build locally
  • Tests pass locally
  • Focused diagnostic build log is committed in this PR
  • No generated build artifacts are committed, except the required diagnostic build log
  • Changes are scoped to the PR purpose and avoid unrelated cleanup
  • Security, privacy, and error-handling implications have been considered

  • I would like to request that my diagnostic build log is removed before merging

@9904099

9904099 commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Thanks for the submission and the detailed validation notes.

I am not going to merge this yet because the required repository-wide python3 build.py run recorded 8/10 modules passing. Since this fork issue follows the same diagnostic requirement as the upstream bounty template, I need the submission to either:

  • include a full green python3 build.py run, or
  • isolate and fix any repository issue that prevents the required diagnostic build from passing on a normal Linux build environment.

A couple of specific things I will check before merging:

  • the guarded refresh path must not retry /auth/refresh recursively
  • concurrent 401/403 responses should share one refresh operation
  • refresh failure should clear all local auth state consistently
  • the diagnostic JSON should not report failed modules for the final submitted head

Please push a follow-up with a clean full-build diagnostic bundle when ready.

@Soengkit

Copy link
Copy Markdown
Author

Thanks for the review. I pushed a follow-up that makes the required repository-wide diagnostic build green.

Changes in the follow-up:

  • Fixed the Linux-only frailbox build assumptions so the C module also builds on non-Linux hosts while keeping the Linux hardening flags enabled on Linux.
  • Added local OpenAPI Haskell diagnostic shims so the existing ghc -fno-code Types.hs Server.hs Validate.hs Generate.hs check does not depend on globally installed aeson, yaml, unordered-containers, or random packages.
  • Preserved the guarded refresh behavior from the original submission.

Validation now recorded:

  • npm run test:api-refresh passed, including non-recursive /auth/refresh, shared concurrent refresh, and auth-state cleanup on refresh failure.
  • git diff --check passed.
  • python3 -m py_compile build.py passed.
  • python3 build.py passed with 10/10 modules green.

New diagnostic artifacts committed for source commit c4f13314:

  • diagnostic/build-c4f13314.json
  • diagnostic/build-c4f13314.logd

The latest PR head is 5cb9e4c.

@9904099

9904099 commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Thanks for pushing the follow-up. I can see the latest diagnostic bundle now records the required repository-wide python3 build.py run as 10/10 green for source commit c4f13314, and the token-refresh validation you described is much closer to the acceptance criteria.

I am still not going to merge this head as-is, mainly because the final PR has grown beyond the frontend/src/services/api.ts token refresh bounty:

  • it now includes the earlier diagnostic/build-ac1a4b1e.* bundle, which still records 8/10 modules and two failures. Please remove that stale failed diagnostic bundle and keep only the final green diagnostic for the submitted source commit.
  • the full-green fix adds local OpenAPI Haskell shim modules under docs/openapi/Data/* and docs/openapi/System/Random.hs. Those may be useful for making diagnostics portable, but they are unrelated to guarded API token refresh and would need separate review as a build/tooling change.
  • the frailbox Makefile / C portability fixes are also outside the frontend token refresh scope. Again, they may be valid, but they should be a separate PR or very clearly justified as a minimal diagnostic-only prerequisite.
  • the PR body still describes the older 8/10 build result and only lists the older diagnostic artifacts. Please update it so the submitted head, validation commands, and diagnostic files all match the final state.

For the token-refresh part itself, the shape is reasonable: single-flight refresh, one retry after a successful refresh, no recursion for /auth/refresh, and clearing stale auth state on refresh failure are the right behaviors. Before I can merge, please push a cleanup pass that leaves the bounty PR focused on:

  1. the frontend/src/services/api.ts refresh implementation,
  2. the executable refresh validation fixture / npm script,
  3. the final green diagnostic bundle required by the issue,
  4. only truly minimal supporting changes needed to run that validation.

If the Haskell shim and frailbox portability changes are needed to make python3 build.py green on your machine, please split them into a separate PR first, or explain exactly why they must be merged together with the frontend auth change. I want to avoid accepting unrelated build-system changes as part of a token refresh bounty, even when the final diagnostic is green.

@9904099

9904099 commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Thanks for the cleanup. Removing the stale failed diagnostic bundle helps, but I still cannot merge this PR in its current shape.

The remaining diff is still larger than the guarded API token refresh bounty:

  • docs/openapi/Data/*, docs/openapi/System/Random.hs add local Haskell/OpenAPI shims.
  • frailbox/Makefile and C source changes add portability fixes.
  • build.py is still modified.

Those may be useful build/diagnostic fixes, but they are separate from the frontend token refresh behavior and need separate review. For this bounty PR, please narrow the branch to the token refresh implementation, its executable validation fixture/script, and the final green diagnostic bundle for that focused source commit.

I am leaving this open for now, but I will not merge while unrelated build/tooling changes remain in the same PR.

@Soengkit Soengkit force-pushed the codex/guarded-token-refresh-2 branch from 5a2f80c to 62362c3 Compare June 21, 2026 02:21
@Soengkit

Copy link
Copy Markdown
Author

Thanks again. I force-pushed the branch to the clean 3-commit history ending at 62362c3 and updated the PR description to match the current diff.

This supersedes my earlier repository-wide diagnostic-support attempt. The current PR no longer contains the Haskell/OpenAPI shim files, frailbox portability changes, stale failed diagnostic artifacts, or the repository-wide diagnostic support commits.

Current scope is now limited to:

  • guarded token refresh implementation in frontend/src/services/api.ts;
  • executable validation fixture/script: frontend/src/services/api_refresh.validation.ts and npm run test:api-refresh;
  • focused green diagnostic bundle for source commit 600e1d20: diagnostic/build-600e1d20.json and diagnostic/build-600e1d20.logd.

Validation rerun on the clean branch:

  • npm run test:api-refresh passed;
  • npm run build passed;
  • python3 -m py_compile build.py passed;
  • python3 build.py -m frontend passed with 1/1 frontend module green;
  • git diff --check origin/main...HEAD passed;
  • PR-range leak scan reported no leaks.

@Soengkit Soengkit changed the title [codex] Add guarded API token refresh Add guarded API token refresh handling Jun 23, 2026
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.

[$45 BOUNTY] [TypeScript] Add guarded API token refresh handling

2 participants