fix: make refreshOn reachable for custom AuthStrategy projects (#135)

[garretpremo]

Closes #135

Summary

sessionAuth.refreshOn (#77) was unreachable for any project that supplies its own AuthStrategy via .apijack/auth.ts. Both client-construction sites in src/cli-builder.ts gated the refresh-and-retry wiring on mergedSessionAuth, which is only populated from a sessionAuth block in the env config — something a custom-strategy project doesn't have. onRefreshNeeded came out undefined, so a stale-session 401 propagated to the caller with no retry, breaking routines mid-run rather than at a step boundary.

The obvious workaround made it worse: adding a sessionAuth block to unlock refreshOn also flipped the strategy ternary, wrapping the custom strategy as the base of a SessionAuthStrategy and running the /session handshake twice. Custom strategy or 401 auto-retry — not both.

This decouples the two. refreshOn is now sourced independently of whether SessionAuthStrategy is in use, and onRefreshNeeded is passed whenever a refresh is possible at all (ctx.refreshSession() was already strategy-agnostic — it invalidates the cached session and re-resolves through whatever strategy is wired). refreshOn alone decides whether the retry fires. The one-retry cap and the "propagate the original error if the refresh fails" behavior from #77/#98 are untouched.

What changes

New: src/auth/refresh-wiring.ts

Both cli-builder.ts sites shared the same merge-and-gate logic; it's now one pure function so they can't drift:

const rawSessionAuth = options.sessionAuth
    ? deepMergeSessionAuth(options.sessionAuth, envConfig?.sessionAuth)
    : undefined;
// Only a block that actually defines a handshake endpoint drives SessionAuthStrategy
// construction and request-header resolution.
const mergedSessionAuth = rawSessionAuth?.session?.endpoint ? rawSessionAuth : undefined;
const refreshOn = options.refreshOn ?? rawSessionAuth?.refreshOn;

Narrowing mergedSessionAuth (rather than widening what flows downstream) is deliberate: resolveRequestHeaders dereferences config.cookies.applyTo unguarded (src/auth/resolve-headers.ts:13), so a cookies-less block must never reach it.

Modified: src/cli-builder.ts — both wiring sites

The createCli path (~L196-258) and the run() path (~L544-658) both call the helper and pass refreshOn through independently of mergedSessionAuth:

-                mergedSessionAuth ? async () => { await ctx.refreshSession(); } : undefined,
-                mergedSessionAuth?.refreshOn,
+                async () => { await ctx.refreshSession(); },
+                refreshOn,

On the run() path the ctx is nullable, so the callback stays gated on ctx (both ctx and sessionMgr are gated on resolved, so the sessionMgr! assertions inside refreshSession can't fire with a null manager). Passing onRefreshNeeded unconditionally is inert without refreshOn — the generated client gates the whole branch on this.refreshOn?.includes(res.status) (src/codegen/client.ts:232).

New opt-in surface

A custom-strategy project can now enable this with no sessionAuth block at all:

// .apijack/settings.json
{ "auth": { "refreshOn": [401] } }

• src/types.ts — CreateCliOptions.refreshOn?: number[]
• src/settings.ts — ProjectSettings.auth?: { refreshOn?: number[] }
• bin/apijack.ts — threads refreshOn: projectSettings.auth?.refreshOn into createCli(...)

Precedence: explicit options.refreshOn wins; otherwise it falls back to the merged sessionAuth.refreshOn, so every existing #77 config behaves identically.

Docs

CLAUDE.md — the "Stale-session refresh and retry (opt-in)" section now states that refreshOn isn't limited to SessionAuthStrategy, shows the settings.json route, and corrects the old blanket claim that strategies not using /session are unaffected. .apijack/settings.json's new key is documented alongside the existing customCommands default.

Acceptance criteria

• A project with a custom AuthStrategy and refreshOn: [401] recovers from a stale-session 401 in-process, calling the strategy's authenticate() once and retrying the original request once
• Enabling refreshOn does not force a custom strategy to be wrapped in SessionAuthStrategy (no double /session handshake, no mandatory cookies.extract)
• Test coverage for the custom-strategy + refreshOn combination, mirroring the SessionAuthStrategy: auto-refresh and retry once on stale-session 401/403 #77 test for the SessionAuthStrategy case
• Both wiring sites in cli-builder.ts (CLI and MCP paths) are covered
• Docs note that refreshOn applies to custom strategies too, and how to enable it without a sessionAuth block
• No regression on the existing sessionAuth.refreshOn path

Test plan

• bun test — 1038 pass / 0 fail (12 new)
• bun run lint — 0 errors (118 pre-existing warnings, none in changed files)
• tsc --noEmit — 15 errors, all pre-existing and in untouched files (src/commands/config/register.ts, src/mcp/tools/*.spec.ts); identical breakdown before and after

New coverage:

• tests/cli-builder-refresh-wiring.integration.test.ts — a custom AuthStrategy (not SessionAuthStrategy) against a local HTTP server: the stale 401 triggers exactly one re-authenticate() and exactly one retry (asserted via distinct Bearer token-1 → token-2 on the retried request), with sessionEndpointHits === 0 proving no handshake was performed and no cookies config supplied. Also covers options.refreshOn precedence, the sessionAuth fallback, and SessionAuthStrategy: preserve original 401/403 (with refresh error as cause) when refresh callback throws #98 error propagation on the custom-strategy path ({status, body, cause} intact when the refresh callback itself throws).
• tests/auth/refresh-wiring.test.ts — unit coverage of the merge/narrow/precedence decision, including the endpoint-less block that must not reach resolveRequestHeaders.
• tests/settings.test.ts — auth.refreshOn parsing, mirroring the existing customCommands.defaults.requiresAuth test.

Both wiring sites were mutation-verified during review rather than assumed: reverting the createCli site alone fails 3 of the new tests; reverting the run() site alone fails the one that exercises it (401 propagates instead of retrying). No new test passes against the unfixed code.

[garretpremo]

🤖 Automated review by claude — generated by the review-issue skill. Treat as advisory; a human still owns the merge decision.

All six acceptance criteria from #135 are met, the change is tightly scoped to the wiring it claims to fix, and CI is green across all 11 checks. The mutation claim in the PR body reproduces exactly: reverting the createCli site alone fails 3 of the new tests, reverting the run() site alone fails 1 — so both call sites are genuinely covered rather than asserted.

Verification performed:

• bun test on 92d73f8 — 1037 pass / 1 fail, the single failure being tests/plugin/paths.test.ts asserting the checkout directory is named apijack (I ran from a /tmp worktree; unrelated to this diff, and green on CI).
• Mutation check on both cli-builder.ts sites, as above.
• Traced every mergedSessionAuth reference on the branch (src/cli-builder.ts:199,200,252,545,552,553,651) — the only consumers are SessionAuthStrategy construction and resolveRequestHeaders, both of which genuinely want the narrowed value. No third client-construction site exists, so "CLI and MCP paths" is exhaustive.
• Confirmed the unconditional onRefreshNeeded is inert without refreshOn: the generated client gates on this.onRefreshNeeded && this.refreshOn?.includes(res.status) (src/codegen/client.ts:232).
• Confirmed the run()-path ctx gate is sound — ctx and sessionMgr are both gated on resolved, so the sessionMgr! assertions inside refreshSession cannot fire with a null manager.

Non-blocking observations

• Narrowing mergedSessionAuth on session?.endpoint silently disables the handshake for a typo'd config. The narrowing is the right call — resolveRequestHeaders dereferences config.cookies.applyTo unguarded — and it's unreachable from a fully-typed caller since SessionAuthConfig.session.endpoint is required. But envConfig.sessionAuth arrives from JSON as an untyped Partial, so a user who misspells the key (sessions: instead of session:) now falls through to options.auth with no SessionAuthStrategy at all, where previously they'd have gotten a loud failure fetching <baseUrl>undefined. Failing softer is arguably an improvement, but a one-line startup warning when rawSessionAuth is present and mergedSessionAuth is not would keep the diagnosis short.
• settings.json auth.refreshOn is unvalidated. loadProjectSettings is a bare JSON.parse + cast, so { "auth": { "refreshOn": 401 } } reaches the generated client and surfaces as this.refreshOn.includes is not a function on the first non-ok response — an error that points nowhere near settings.json. This matches how customCommands.defaults.requiresAuth is handled today, so it's a consistency call rather than a regression.
• Docs cover the settings.json route but not the createCli option. The new CLAUDE.md paragraph gives custom-strategy projects the .apijack/settings.json route, which only applies to consumers on the shared apijack binary; a project with its own bin/<cli>.ts opts in via createCli({ refreshOn: [401] }). That's documented on CliOptions.refreshOn in src/types.ts, but a reader in CLAUDE.md won't see it.

Nitpicks

• The Project Extensions table's .apijack/settings.json row still reads "Framework defaults (see below)", and the only place that file's shape is spelled out is the customCommands example under "Opt-in auth for custom commands and dispatchers". A reader scanning for settings keys won't find auth.refreshOn, which lives several sections earlier under Session Auth. A cross-reference in either direction would help.

[garretpremo]

🤖 Final review by claude — generated by the final-review skill. CI is green, no blockers, soak window elapsed. Marking approved and merging.

Approved.

Outstanding observations from first-pass review

• Narrowing mergedSessionAuth on session?.endpoint silently disables the handshake for a typo'd config. The narrowing is the right call — resolveRequestHeaders dereferences config.cookies.applyTo unguarded — and it's unreachable from a fully-typed caller since SessionAuthConfig.session.endpoint is required. But envConfig.sessionAuth arrives from JSON as an untyped Partial, so a user who misspells the key (sessions: instead of session:) now falls through to options.auth with no SessionAuthStrategy at all, where previously they'd have gotten a loud failure fetching <baseUrl>undefined. Failing softer is arguably an improvement, but a one-line startup warning when rawSessionAuth is present and mergedSessionAuth is not would keep the diagnosis short.
• settings.json auth.refreshOn is unvalidated. loadProjectSettings is a bare JSON.parse + cast, so { "auth": { "refreshOn": 401 } } reaches the generated client and surfaces as this.refreshOn.includes is not a function on the first non-ok response — an error that points nowhere near settings.json. This matches how customCommands.defaults.requiresAuth is handled today, so it's a consistency call rather than a regression.
• Docs cover the settings.json route but not the createCli option. The new CLAUDE.md paragraph gives custom-strategy projects the .apijack/settings.json route, which only applies to consumers on the shared apijack binary; a project with its own bin/<cli>.ts opts in via createCli({ refreshOn: [401] }). That's documented on CliOptions.refreshOn in src/types.ts, but a reader in CLAUDE.md won't see it.