Skip to content

feat(bootstrap): add createSessionLifter for renewable session lifts - #191

Merged
chrischall merged 1 commit into
mainfrom
feat/create-session-lifter
Aug 2, 2026
Merged

feat(bootstrap): add createSessionLifter for renewable session lifts#191
chrischall merged 1 commit into
mainfrom
feat/create-session-lifter

Conversation

@chrischall

Copy link
Copy Markdown
Owner

Closes #183.

The problem

bootstrap() returns a value, so consumers naturally capture a session once at process start and hand it to their client. That's the API's grain, not a consumer mistake — and it produces a bug that stays invisible until the site's credential lifetime is short enough to notice:

  • The expiry dead end. A browser-backed account has no password, so when the captured credential lapses there's nothing to re-login with. The MCP is unauthenticated for the life of the process — while the browser held a live session the whole time.
  • The sticky startup failure. A lift that failed at boot (user not signed in yet) gets cached just as permanently, so signing in afterwards changes nothing.

The audit in #183 found four MCPs carrying the one-shot capture (infinitecampus, groupon, setlist, canvas-parent) and four that had independently hand-rolled the renewable shape (ofw, resy, evite, zola). Four repos solving it and four not is the argument for putting it in the library.

The API

const lift = createSessionLifter({ serverName, version, domains, declare });
const manager = new CookieSessionManager({ login: lift, isExpired });

Construction is pure — no server, no listen(), no pair prompt — so it drops straight into whatever mints a session, and every expiry re-reads the browser.

Answering the design questions from #183

Keep bootstrap() or deprecate it? Kept, and reimplemented as one invocation of a lifter so the two cannot drift. It stays first-class because the tool-invoked capture pattern (vibo, honeybook: a capture_session tool the user runs on demand, which then persists the token) is legitimately one-shot.

Should the lifter own TTL/caching? No. Expiry semantics are app-specific — some sites need the lifted token exchanged before it's usable, others lapse on an idle timer the library can't observe. The caller's session manager owns when to re-lift; the lifter owns how.

A renew hook? Not needed — post-processing composes in userland:

const raw = createSessionLifter(opts);
const lift = async () => exchangeIfStale(await raw());

Adding a hook would bake app-specific auth into the bridge library for no expressiveness gain.

One thing not in the issue: concurrent calls are single-flighted, so two simultaneous expiries share one bridge round-trip rather than racing two open/close cycles for the same MCP. That's de-duplication, not caching — once a lift settles, the next call starts a fresh one.

Verification

1092 tests pass; typecheck clean. Beyond the stubs, verified against a live bridge:

constructed — bridge not touched yet
lift 1: cookies: accessToken,cfid,cftoken | missing: none
lift 2: cookies: accessToken,cfid,cftoken | same lifter, second bridge round-trip
concurrent lifts single-flighted: true

Follow-on

The four affected MCPs can migrate to this once it publishes. signupgenius-mcp and infinitecampus-mcp already have the renewable shape hand-rolled and will simplify onto it.

bootstrap() returns a VALUE, so consumers naturally capture a session
once at process start and hand it to their client. That is the API's
grain, not a consumer mistake, and it produces a bug that stays
invisible until the site's credential lifetime is short enough to
notice:

  * The expiry dead end. A browser-backed account has no password, so
    when the captured credential lapses there is nothing to re-login
    with — the MCP is unauthenticated for the life of the process even
    though the browser held a live session the whole time.
  * The sticky startup failure. A lift that failed at boot (user not
    signed in yet) is cached just as permanently, so signing in
    afterwards changes nothing.

A fleet audit found four MCPs carrying the one-shot capture and four
that had independently hand-rolled the renewable shape. That split is
the argument for putting it in the library.

createSessionLifter(opts) returns a repeatable lift. Construction is
pure — no server, no listen(), no pair prompt — so it drops straight
into a session manager's `login`, and every expiry re-reads the browser.

Deliberately excluded: TTL tracking and result caching. Expiry semantics
are app-specific (some sites need the lifted token exchanged before it
is usable; others lapse on an idle timer the library cannot observe), so
the caller owns WHEN to re-lift and the lifter owns HOW. Post-processing
composes in userland instead of needing a hook. Concurrent calls are
single-flighted so two simultaneous expiries share one bridge round-trip
— de-duplication, not caching.

bootstrap() is kept and reimplemented as one invocation of a lifter, so
the two cannot drift. It stays first-class because the tool-invoked
capture pattern (a capture_session tool the user runs on demand) is
legitimately one-shot.

Verified against a live bridge: deferred construction, two independent
lifts from one lifter, and concurrent single-flighting.

Closes #183

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@chrischall chrischall added the enhancement New feature or request label Aug 2, 2026
@github-actions github-actions Bot added the auto-review Trigger Claude + Copilot review on this PR label Aug 2, 2026
@claude

claude Bot commented Aug 2, 2026

Copy link
Copy Markdown

Claude finished @chrischall's task in 2m 45s —— View job


Review: PR #191createSessionLifter

  • Read CLAUDE.md conventions
  • Read PR diff (origin/main...HEAD)
  • Review packages/bootstrap/src/index.ts changes
  • Review test coverage in packages/bootstrap/tests/bootstrap.test.ts
  • Post findings / verdict

Note: npm test/npm run typecheck were not runnable in this sandbox (Bash execution required approval that wasn't available), so verification here is by careful reading rather than execution. The PR body reports 1092 tests passing and clean typecheck, plus a live-bridge smoke test.

What I checked closely

The single-flight mechanism in createSessionLifter (packages/bootstrap/src/index.ts:263-275) is the trickiest part of this change — it wraps runOneLift(opts).finally(...) and clears inFlight inside the finally callback. I traced the microtask ordering: because p = q.finally(fn), fn (which clears inFlight) runs and completes before p itself settles, so any code awaiting lift() sees inFlight === null by the time its continuation runs — matching the "a lift started from within a continuation is not deduped against the one that just settled" comment, and the "single-flights concurrent lifts" test. .finally fires on both success and rejection, so a failed lift correctly un-sticks the lifter for the next attempt (covered by the "surfaces a failed lift and stays usable" test). bootstrap() being reimplemented as runOneLift(opts) is a faithful behavior-preserving refactor, confirmed by the "bootstrap() is exactly one invocation of a lifter" test.

I didn't find any 🔴 Important issues — no bugs, no security/data-integrity concerns, and the change follows the documented @fetchproxy/bootstrap conventions (kept bootstrap() for one-shot capture-tool callers, no TTL/caching baked into the lifter, per the PR description's design rationale). Test coverage for the new function is thorough: construction purity, repeatability, fresh-state-per-call, failure recovery, single-flight, the disable-env-var path, and equivalence with bootstrap().

🟡 Nit (non-blocking)

  • The file-level doc comment at the top of packages/bootstrap/src/index.ts (lines 1-17) still describes the module as a one-shot-only helper and doesn't mention createSessionLifter, even though it's now the recommended entry point whenever a session can expire. Left as an inline comment.

Verdict: warn

* bridge round-trip rather than racing two. Once a lift settles the next call
* starts a fresh one — this is de-duplication, not caching.
*/
export function createSessionLifter(opts: BootstrapOpts): SessionLifter {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit: now that createSessionLifter is the recommended entry point whenever a session can expire, the file-level doc comment at the top of this file (lines 1-17) is stale — it still frames the whole module as a "one-shot session-capture helper" and only describes bootstrap()'s lifecycle, with no mention of this function. Worth a short update so a reader skimming just the header doesn't get the superseded picture.

Fix this →

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🟡 Auto-review verdict: warn — The createSessionLifter implementation is correct — the single-flight/finally microtask ordering, error-recovery, and bootstrap() delegation all check out — and test coverage is thorough; only a stale file-level doc comment nit was found.
📋 Tracking follow-ups: #192

@chrischall chrischall added the ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green label Aug 2, 2026
@chrischall
chrischall enabled auto-merge (squash) August 2, 2026 17:59
@chrischall
chrischall merged commit b2c7049 into main Aug 2, 2026
18 checks passed
@chrischall
chrischall deleted the feat/create-session-lifter branch August 2, 2026 17:59
chrischall added a commit that referenced this pull request Aug 2, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.9.0](v1.8.0...v1.9.0)
(2026-08-02)


### Features

* **bootstrap:** add createSessionLifter for renewable session lifts
([#191](#191))
([b2c7049](b2c7049))


### Bug Fixes

* **cli:** catch every
gate-[#2](#2) scope
rejection, not just three
([#187](#187))
([b2ecced](b2ecced)),
closes [#185](#185)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
chrischall added a commit to chrischall/infinitecampus-mcp that referenced this pull request Aug 3, 2026
…#113)

The fetchproxy path captured `JSESSIONID`/`XSRF-TOKEN` once, at process
start, and handed the values to the client.

`JSESSIONID` is a Java servlet session with a short idle timeout, so the
first lapse killed the path for the life of the process: the synthesized
account has empty credentials, `verify.jsp` was therefore not an option,
and `login()` raised a `permanent` `AuthFailedError` telling the user to
**"restart the MCP"**.

Restarting really was the only cure — which is precisely the bug. The
browser held a live session the whole time and nothing ever re-read it.
A lift that failed at boot was equally sticky: signing in afterwards
changed nothing until restart.

## The fix

`resolveAuth()` now returns a `refresh` function instead of a captured
value, and the primary manager's `login` calls it on the first request
**and on every expiry**. A lapsed session recovers by re-reading the
browser.

Same defect and same shape as `signupgenius-mcp`; tracked fleet-wide in
chrischall/fetchproxy#183. When `createSessionLifter` publishes
(chrischall/fetchproxy#191) this hand-rolled lifter can collapse onto
it.

## On the tests

Three existing tests asserted the *old* behavior — `refuses to attempt
verify.jsp`, `caches the permanent no-creds error`, `refuses to retry on
401 with empty creds`. They weren't merely failing; they encoded the
dead end as correct. Rather than delete them I rewrote them to hold the
guarantee that still matters:

- **Empty credentials are never POSTed to `verify.jsp`** — preserved and
still asserted, now across a renewal.
- **The permanent-error cache** still applies, but only in the genuinely
unrecoverable case: no credentials *and* no lift.
- Added coverage for a lift failing *during* a renewal (user signs out
mid-session), which the old dead-end path had covered incidentally.

295 tests, 100% coverage, build clean.

---

**Auto-review follow-up (#114) addressed:**

The `fail` finding was real and mine. In fetchproxy mode the primary's
login short-circuited to the lift and never ran
`discoverLinkedDistricts`, while `fetchproxyDiscoveryRan` latched true
after the first request. A linked district that lost its session
re-authed *through the primary* into a primary that never re-seeded it —
permanently broken until a restart, which is the exact failure this PR
set out to remove.

Env mode never had this: `mintSessionCookie()` ends with discovery, so
every primary re-mint re-seeds. The lift branch now does the same, which
makes the latch unnecessary — discovery runs on one condition in both
modes (a primary session was minted), so no flag is left to go stale.

Both nits fixed too (stale CLAUDE.md/README copy, and a test whose title
still described the old dead-end).

296 tests, 100% coverage.

Closes #114

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
chrischall added a commit that referenced this pull request Aug 3, 2026
Auto-review follow-up for #191.

The file-level comment still described the module as one-shot-only and
never mentioned `createSessionLifter` — so the first thing a reader sees
pointed at exactly the shape that PR set out to stop being the default.

Rewritten to lead with the choice: `createSessionLifter` for anything
whose session can expire, `bootstrap` for genuinely one-shot callers
(the user-invoked `capture_session` tool pattern). Includes the *why*,
since the failure mode is what makes the default matter — capturing a
value once is how an MCP ends up working for one credential lifetime and
then dying with no way back.

Docs only; 1092 tests pass, typecheck clean.

Closes #192

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
chrischall added a commit to chrischall/canvas-parent-mcp that referenced this pull request Aug 3, 2026
…minal (#108)

The fetchproxy path captured `canvas_session`/`pseudonym_credentials`
once, at process start, and handed the cookie to the client.

`canReauth()` returned **false** in that mode — the synthesized account
has empty `username`/`password`, so there was nothing to re-mint with —
which made a 401 terminal. The user was told to re-sign-in in the
browser, but doing so changed nothing until the MCP restarted, because
nothing ever re-read the tab.

## The fix

`resolveAuth()` returns a `refresh` function instead of a captured
cookie, and the client calls it on the first mint **and on every 401**.
`canReauth()` now counts a lift as re-auth capability, so the existing
exactly-once replay in `CookieSessionManager` finally has something to
replay with.

## Severity

Canvas session cookies are long-lived, so this was **latent** rather
than actively biting — unlike `signupgenius-mcp` (30-minute JWT) and
`infinitecampus-mcp` (servlet session idle timeout), where the same
one-shot capture killed the integration outright. Fixing it here closes
the pattern rather than waiting for a user to hit it.

## Scope

The lift is additive — nothing else changes:

| Mode | Behavior |
|---|---|
| `session` + lift (fetchproxy) | Re-lifts on 401, replays once |
| `session` + `CANVAS_USERNAME`/`PASSWORD` | Still mints via
`sessionLogin` |
| `token` | Untouched — no refresh path, 401 still terminal |
| `oauth` | Untouched — already re-mints |

229 tests, 100% coverage, typecheck and build clean.

Part of the fleet-wide audit in chrischall/fetchproxy#183. Collapses
onto `createSessionLifter` (chrischall/fetchproxy#191) once that
publishes.

---

**Auto-review follow-up (#109) addressed.**

The `fail` findings were both real:

- `tests/auth.test.ts:62` and `:118` still asserted `result.preloaded` —
a field this PR removes. `expect(undefined).toBeUndefined()` passes for
the wrong reason, so both tests silently stopped verifying what they
were written for (that env-var paths carry no fetchproxy lift). They now
assert on `refresh`.
- `CLAUDE.md` documented the removed design in four places, including a
"fetchproxy 401s are terminal" gotcha that is now the *opposite* of the
shipped behavior.

Plus the nit: the `ResolvedAuth` doc comment survived the rename
half-edited, describing `refresh` as a captured cookie string rather
than the lift function it is.

229 tests, 100% coverage.

Closes #109

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review Trigger Claude + Copilot review on this PR enhancement New feature or request ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(bootstrap): createSessionLifter — make renewable session lifts the default shape

1 participant