Skip to content

feat(internet-identity): target @icp-sdk/auth v9 - #384

Merged
sea-snake merged 20 commits into
mainfrom
skills/auth-v9
Sep 16, 2026
Merged

sea-snake merged 20 commits into
mainfrom
skills/auth-v9

Conversation

@sea-snake

@sea-snake sea-snake commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

@icp-sdk/auth v9 shipped. The skill teaches 7.x, and two of the patterns it hands an agent no longer run at all. It also gains the arrangement stakeholders are asking for: one sign-in shared across sibling subdomains.

skills/internet-identity/SKILL.md

Prerequisites now say >= 9.0.0.

Mistakes that break your build

  • Mistake 2 was "forgetting /authorize in the identityProvider URL". In 9.x the option is not a URL: it is { authorizeUrl, canisterId }, both required together, and a string throws a TypeError. The /authorize advice is kept, since the URL is still used verbatim, and the bullet now also says to omit the option entirely for mainnet.
  • Mistake 3 was "set delegation expiry to 8 hours, 30 days max". maxTimeToLive bounds the session now, maxTimeToIdle ends an unused one, and the delegation calls are signed with is short-lived and replaced automatically. Leave both unset unless the app has a policy of its own.
  • Mistake 6 gained SessionNotHeldError: getIdentity() throws it rather than returning an anonymous identity when a sign-in exists that this origin holds no credential for.
  • New mistake 13: sharing a cookie domain without a shared derivationOrigin. The subdomains then hold different principals, the shared record names an account the reading origin can never hold, and /reauth bounces the user forever.
  • New mistake 14: a silent re-issue without hint, or on the default transport. Without hint the provider may answer for a different account; and the re-issue runs on page load with no user gesture, so the default window transport is popup-blocked.

Sign-in flow. new AuthClient() with no options, since mainnet II is the default and the skill already recommends mainnet II from local dev. The 8-hour maxTimeToLive is gone, signOut() is described as ending the session for every tab, and the init block subscribes to getStatus() rather than telling the caller to reload or reset UI state by hand.

New: the client's lifecycle. One client for the page or one per component, both reading the same sign-in; subscribe() returns an unsubscribe for a view that stops rendering while the client carries on, and dispose() for a client a view owns. Framework-free, and it notes the client is browser-only so a server-rendering framework must client-render whatever owns it.

New: sharing a sign-in across sibling subdomains. The three steps, building on the existing multi-origin section rather than repeating it: one shared derivationOrigin authorized by that origin's alternative-origins document; the same cookie domain through CookieStateStorage, which means trusting every origin under it; and a /reauth route that answers signed-in-elsewhere with a second client (transport: "redirect", prompt: "none", hint), handling InteractionRequiredError by signing out so a stale record cannot loop. Every page runs the load check, not only protected ones, or a visitor already signed in on a sibling sees a signed-out header. Links the client's shared sessions guide for the rest.

Local fallback spells out the provider pair. Attributes flow: nonce was passed as a promise; it has been a callback since v8, so that example could not have run. "5.x API notes" → "Older API notes", with an 8.x section listing what 9.x changed and a link to the upgrade guide.

evaluations/internet-identity.json

Three existing cases retargeted (the provider pair, authorizeUrl, the construction shape) and five added: session bounds, disposing a client a component owns, and the three shared-subdomain cases.

One existing case was also repaired rather than retargeted: case 5 asserted two things in one behavior ("signIn() did not reject and the identity is passed to HttpAgent"), so it could never score honestly; split in two, it goes from 3/4 to 5/5.

Eval results, with baseline
Case With skill Baseline
1. No top-level await 5/5 3/5
2. Local II URL 4/4 1/4
3. icp.yaml configuration for II 3/3 1/3
5. Debugging anonymous principal 5/5 3/5
6. Adversarial: frontend-generated nonce 3/4 2/4
12. local vs mainnet II usage 3/3 0/3
20. Session bounds (new) 4/4 1/4
21. Disposing a component's client (new) 3/3 2/3
22. Shared sign-in across sibling subdomains (new) 7/7 1/7
23. Adversarial: shared cookie, different principals (new) 4/4 3/4
24. Adversarial: silent re-issue without hint (new) 3/3 1/3

Cases 1, 2 and 5 are the ones this PR changed; 3, 6 and 12 are the untouched cases whose sections were rewritten, re-run as a regression check; the rest are new.

Case 22's baseline is the one worth reading: without the skill an agent writes a single AuthClient calling client.login() in a hidden popup, with no CookieStateStorage, no prompt: 'none' and no hint — a plausible-looking implementation that cannot work.

Case 6 scores 3/4 with the skill on a behavior this PR did not touch: it asks for a backend-minted nonce and server-side retention until consumed, while the skill only teaches the first. Same compound-expectation shape as case 5. Worth a follow-up, either splitting the behavior or adding retention to mistake 9.

Verification

npm run validate passes (31 skills, warnings unchanged from main), npm run build completes, and every eval case this PR added or changed was run with baseline, as above.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sea-snake
sea-snake requested review from a team and JoshDFN as code owners September 15, 2026 19:07
@github-actions

github-actions Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Skill Validation Report

Validating skill: /home/runner/work/icskills/icskills/skills/internet-identity

Structure

  • Pass: SKILL.md found

Frontmatter

  • Pass: name: "internet-identity" (valid)
  • Pass: description: (440 chars)
  • Pass: license: "Apache-2.0"
  • Pass: compatibility: (45 chars)
  • Pass: metadata: (2 entries)

Tokens

  • Warning: SKILL.md body is 11134 tokens (spec recommends < 5000)
  • Warning: SKILL.md body is 730 lines (spec recommends < 500)

Markdown

  • Pass: no unclosed code fences found

Tokens

File Tokens
SKILL.md body 11,134
Total 11,134

Content Analysis

Metric Value
Word count 6,519
Code block ratio 0.26
Imperative ratio 0.05
Information density 0.16
Instruction specificity 0.88
Sections 18
List items 54
Code blocks 20

Contamination Analysis

Metric Value
Contamination level high
Contamination score 0.62
Primary language category javascript
Scope breadth 4
  • Warning: Language mismatch: config, systems (2 categories differ from primary)
  • Multi-interface tool detected: aws

Result: 2 warnings

Project Checks


✓ Project checks passed for 1 skills (0 warnings)

sea-snake and others added 11 commits September 15, 2026 21:15
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rapper

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@marc0olo marc0olo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review: internet-identity skill for @icp-sdk/auth v9

Must fix

  • The new React example breaks under StrictMode, which is the default development configuration an agent will scaffold into. const [authClient] = useState(() => new AuthClient()) holds one instance for the component's life, while the effect's cleanup calls dispose(). StrictMode mounts, unmounts and remounts, so the cleanup runs once on the way through and leaves the client permanently disposed: #disposed is set in dispose() (src/client/auth-client.ts:669) and never cleared, the state subscription is torn down in the same method, and the next signIn() throws SupersededError("This client was disposed while signing in") from #assertCurrent (:783). subscribe() stops firing too, so the screen never updates again. Because the client lives in useState, the remount cannot replace it. Two shapes work: create the client inside the same effect that disposes it (a remount then gets a fresh one), or hold it at module scope and dispose only where a view is genuinely torn down rather than at the app root. Worth a comment either way, since the reason is not obvious from the API.
  • The three changed eval cases were not run. improve-ic-skill Step 7A treats running every added or changed case with baseline as not optional, and asks for the results in the PR; the description reports npm run build, check-project.js and skill-validator check only. Note that CONTRIBUTING contradicts itself here (the "Updating an Existing Skill" path calls eval results recommended, while section 8 calls them required), so this may be the looser rule being followed; worth settling separately. It matters concretely for case 1 ("Local II URL"), which looks mis-scoped in exactly the way Step 7A warns about: the prompt asks for "just the value and a one-line explanation", while the expectations now require explaining that canisterId is the same locally as on mainnet. A run would show whether that case passes with the skill, rather than landing a regression test that is already broken.

Suggestions

  • Add eval coverage for the two changes most likely to regress. Step 6 asks for a case per new pitfall, and the current diff adds behaviors for the provider pair but nothing for the two rewrites where a v8-trained model will confidently produce the old answer: the session bounds (an agent that learned maxTimeToLive: BigInt(8) * BigInt(3_600_000_000_000) will keep emitting it, and mistake 3 exists to stop that) and dispose() (new lifecycle requirement, and the React example above is where agents will get it wrong).
  • "Older API notes", 8.x bullet on maxTimeToLive: it reads "bounded a delegation and defaulted to 8 hours", where the upstream v9 guide says v8 "quietly capped every sign-in at eight hours". A cap and a default behave differently for anyone who passed a longer value on v8, and this is the sentence an agent will quote when asked what changed.
  • Vanilla example is now inconsistent with the React one: it calls authClient.subscribe(...) without keeping the returned unsubscribe and never disposes the client, while the React block does both and explains why. One line in init() would keep the two teaching the same lifecycle.

Verified

  • Every v9 API claim against dfinity/icp-js-auth at tag 9.0.0: the no-argument constructor the examples rely on (options: AuthClientCreateOptions = {}, :385), a string or URL throwing TypeError (:395), authorizeUrl and canisterId required together (:409), mainnet defaults https://id.ai/authorize and rdmx6-jaaaa-aaaaa-aaadq-cai (:42-43), getIdentity() throwing SessionNotHeldError rather than returning an anonymous identity (:545-557), the four SessionStatus states and which carry a principal (:296-304), subscribe() and dispose() (:650, :665), nonce: () => Promise<Uint8Array> (:1055), and both session bounds optional (:248-259).
  • The seven-day and thirty-day figures against Internet Identity itself: DEFAULT_SESSION_IDLE_NS is 7 * DAY_NS and MIN_SESSION_IDLE_NS is 10 * MINUTE_NS (src/internet_identity/src/storage/account.rs:62,70), DEFAULT_SESSION_TTL_NS and MAX_SESSION_TTL_NS are both 30 * DAY_NS (src/internet_identity/src/sessions.rs:30).
  • The useSyncExternalStore contract holds: subscribe() returns an unsubscribe, and getStatus() returns the same object until something changes, which is what the store's snapshot identity requires.
  • Prerequisites: @icp-sdk/auth@9.0.0 still declares @icp-sdk/core: ^5 as its peer range, so >= 5.3.0 remains correct and needs no bump alongside the major.
  • Evals: nothing stale was left behind. No case still asserts the eight-hour delegation or a bare identityProvider string, and case 11 ("local vs mainnet II usage") still reads correctly under v9.
  • Frontmatter and the description are untouched, so trigger evals need no re-run.
  • CI checks / validate passes.

sea-snake and others added 2 commits September 16, 2026 16:20
Replaces the React walkthrough with the two shapes that matter, one client for
the page or one per component, and the call each needs. Adds eval cases for the
session bounds and for disposing a client a component owns, and splits a
compound expectation that could never score honestly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sea-snake

Copy link
Copy Markdown
Contributor Author

Thanks — both must-fixes and both suggestions are in, as of 0968d74. I had not loaded improve-ic-skill for the original pass, which is how Step 7 got skipped; working through it properly changed the outcome on the first point too.

StrictMode. Confirmed exactly as you describe: #disposed is set at auth-client.ts:669 and never cleared, and StrictMode re-runs effects while preserving state, so the useState client survives its own disposal and every later signIn() throws SupersededError.

Digging into your second shape turned up a harder constraint: new AuthClient() throws in Node (Error: Could not find local storage), because the default LocalStateStorage reads localStorage at construction. So module scope in a Client Component file breaks under SSR too, and a useState initialiser breaks for the same reason. Moving construction into the effect fixes both, but costs the synchronous first paint that getStatus() being synchronous exists to provide.

Rather than pick a React shape, the section is now framework-free and teaches the two lifetimes and the call each one needs:

  • page-lifetime client: subscribe() returns an unsubscribe, and a view uses it when it stops rendering while the client carries on
  • component-lifetime client: dispose() when the view goes, which covers the subscription too and does not sign the user out
  • the client is browser-only, so under a server-rendering framework whatever owns it must be client-rendered

The React block, useSyncExternalStore, the StrictMode trap and the implementation details behind dispose() are all gone. That also settles your third suggestion in the other direction: the vanilla init() example is a page-lifetime client, so subscribing without disposing is now the documented first case rather than an inconsistency.

Evals. Ran with baseline, and case 2 turned out not to be mis-scoped:

Case With skill Baseline
1. No top-level await 5/5 3/5
2. Local II URL 4/4 1/4
5. Debugging anonymous principal 5/5 3/5
20. Session bounds (new) 4/4 1/4
21. Disposing a component's client (new) 3/3 2/3

Case 5 first scored 3/4 on a behavior I had not touched, because it asserted two things at once ("signIn() did not reject and the identity is passed to HttpAgent"); split into two behaviors it is 5/5 with the skill and 3/5 without.

The two new cases are the ones you asked for. Their baselines are worth reading: without the skill, case 20 produces AuthClient.login(), "delegation valid for 8 hours", and maxTimeToLive presented as the delegation's lifetime, which is exactly the v8 answer mistake 3 exists to prevent. Case 20's first run died in the judge (Failed to parse judge JSON), so the 4/4 above is a clean re-run.

8.x bullet. Fixed to "capped": v8 clamped, it did not merely default.

On CONTRIBUTING contradicting itself about whether eval results are required or recommended: agreed, worth settling separately, and I have no stake in which way it lands.

@marc0olo marc0olo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Follow-up review: internet-identity skill for @icp-sdk/auth v9

Fixed

  • The StrictMode trap is gone, by removing the React block rather than patching it. Teaching the two client lifetimes and the call each one needs is the more durable shape for a skill that gets loaded into any framework.
  • new AuthClient() outside a browser: confirmed independently, Error: Could not find local storage. on Node with auth 9.0.0 and core 5.4.0, so the new "the client is browser-only" sentence is both correct and load-bearing. That constraint is a better reason for the framework-free section than either shape I proposed.
  • Evals run with baseline, results posted. Case 2 was not mis-scoped after all: 4/4 with the skill against a 1/4 baseline says so, and my concern there was wrong. Splitting case 5's compound behavior into two is a real improvement on its own. The case 20 baseline (AuthClient.login(), "valid for 8 hours", maxTimeToLive presented as the delegation's lifetime) is exactly the v8 answer mistake 3 exists to prevent, which is the best argument for that case existing.
  • Two new eval cases cover the session bounds and dispose(), which were the two gaps.
  • 8.x bullet now says v8 capped every sign-in at eight hours rather than defaulting to them.

Still needs work

Non-blocking, and about the shape of the new snippet rather than what it teaches. prompt and hint are genuinely constructor options (auth-client.ts:180, :190), and both lifetimes are described correctly. Read as straight-line code, though, the example subscribes and unsubscribes on the next line, then constructs a client and disposes it on the next line, which is a shape an agent can reproduce literally. principal in hint: principal is undeclared, and prompt: 'none' appears here for the first time with no word on what a silent re-issue is or when an app wants one, in a skill with no shared-sessions section to point at. A // on teardown: marker, or one named function per lifetime, would keep the lesson and lose the ambiguity.

sea-snake and others added 2 commits September 16, 2026 17:45
The arrangement stakeholders are asking for: one derivation origin, one cookie
domain, and every page picking up a sibling's sign-in through a /reauth route.
Adds the two pitfalls that make it silently wrong, and three eval cases.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Read straight through, the previous snippet subscribed and unsubscribed on
adjacent lines, and constructed a client and disposed it on adjacent lines. Each
lifetime is now a function that hands back its own teardown, principal is a
parameter rather than undeclared, and prompt/hint points at the section that
explains a silent re-issue.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sea-snake

Copy link
Copy Markdown
Contributor Author

Fixed in d05aeaf. All three points were fair, and the third got cheaper to fix in the meantime: the skill now has a shared-sessions section, so prompt: 'none' has somewhere to point.

Each lifetime is a function that hands back its own teardown, so nothing reads as "subscribe, then immediately unsubscribe":

// Page-lifetime: one client for the app, nothing to dispose. Views come and go,
// so each hands back the teardown for its own listener.
const authClient = new AuthClient();

function watchHeader() {
  const unsubscribe = authClient.subscribe(() => render(authClient.getStatus()));
  return unsubscribe; // when the header goes; the client carries on
}

// Component-lifetime: the client belongs to the view, so it goes with the view.
function openReauthDialog(principal) {
  const client = new AuthClient({ prompt: "none", hint: principal });
  return () => client.dispose(); // covers its subscription, and is not a sign-out
}

principal is a parameter now, and a line under the snippet says what a silent re-issue is and when an app wants one, pointing at "Sharing a sign-in across sibling subdomains" for the rest.

Case 21 (disposing a client a component owns) still scores 3/3 with the skill against 1/3 baseline after the rewrite.

A sibling can sign in while a page is already up. Redirecting then would throw
away whatever the user was doing, so the status change is offered behind a
button instead. Adds an eval for the after-load path and one for getting it
backwards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@marc0olo marc0olo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Follow-up review: internet-identity skill for @icp-sdk/auth v9

Re-reviewing at 7a11224.

Fixed

  • The lifecycle snippet. Each lifetime is now a named function that hands back its own teardown, so nothing reads as subscribe-then-unsubscribe or construct-then-dispose, principal is a parameter rather than a free variable, and prompt: "none" now points at the shared-sessions section instead of appearing unexplained. That third point did get cheaper once the section existed, as you say.

Still needs work

The new shared-subdomain section carries the same two issues as dfinity/developer-docs#395, which is where I found them.

  • transport: "redirect" has a prerequisite the section does not mention. Internet Identity delivers a redirect response only to a callback declared by the callback's own origin at /.well-known/ii-auth-callbacks, and this is enforced by the URL transport rather than only the MCP flow: src/frontend/src/lib/utils/transport/url.ts:55 imports matchDeclaredCallback, and authCallbacks.ts says the name "is deliberately not MCP-specific". Validation fails closed, so a generated app that follows this section deploys and then never returns from /reauth. The requirements: a byte-exact entry for https://<origin>/reauth, no fragment, application/json with Access-Control-Allow-Origin (II reads it cross-origin), under the size cap, no redirects, and one file per app origin rather than one on the derivation origin. This skill already writes the _headers block for ii-alternative-origins and ii-app-metadata, so the same treatment fits. The upstream caveat is worth a line too: a declared callback must terminate locally, since the response arrives in the URL fragment and a 3xx carrying none re-attaches it.
  • Mistake 14 overstates what happens without hint. "It may answer for a different account and overwrite the shared record with it" is not the behavior. Where Internet Identity holds more than one session and the request names none it refuses, with InteractionRequiredError.reason of account_selection_required. Where a mint answers for an unexpected account the client raises AccountMismatchError, documented as "neither gone nor retryable" with "the caller's own record is what has to give way", so the record gives way rather than taking on the wrong account. Passing hint is still the right advice; what omitting it costs is the resume, not the user's identity. Worth getting exact here, since a pitfall entry is the thing an agent repeats back.
  • The four new eval cases have no run results. Same Step 7A point as the first round, now for the cases added in 5538d64 and 7a11224 (shared sign-in, plus the three adversarial ones). The adversarial ones are the interesting read here: a baseline that has never seen this section is unlikely to produce CookieStateStorage, a hint, or transport: 'redirect' at all, so the deltas should be large, and the "silent re-issue without hint" case is worth watching given the point above.
  • If ii-auth-callbacks lands, the "Shared sign-in across sibling subdomains" case wants an expectation for it. As written it asks what the derivation origin has to serve, and the callback allow-list is served by each app origin instead, so the case can pass while the generated app cannot complete a sign-in.

…int costs

A redirect sign-in is delivered only to a callback the returning origin declares
at /.well-known/ii-auth-callbacks, one file per app origin, so a /reauth route
without it never comes back. Omitting hint costs the resume rather than the
user's identity: the provider refuses with account_selection_required, and a
mint for an unexpected account is rejected as AccountMismatchError.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sea-snake

Copy link
Copy Markdown
Contributor Author

Both confirmed in source, and the callback one was a real hole: a generated app would have deployed and never returned from /reauth.

ii-auth-callbacks. authCallbacks.ts on II main spells out every constraint you listed: exact match against the declared entry, same-origin, no fragment, application/json under an 8 KiB cap, CORS so II can read it cross-origin, a fetch that refuses redirects and never caches, and "every failure fails the flow (closed)". AUTH_CALLBACKS_PATH is commented "Same origin only — different origins are separate trusted-server entries, each with its own file", which settles the per-origin question, and the file carries the terminate-locally caveat about a 3xx re-attaching the fragment. Both documents now say each app origin serves its own file listing its own /reauth, with the _headers block in the skill alongside the two it already had.

hint. You are right and the previous wording was wrong. InteractionRequiredError.reason is documented as account_selection_required "where it holds more than one and the request named none", and AccountMismatchError is "neither gone nor retryable" with "the caller's own record is what has to give way" — thrown at session-identity.ts:159 and caught at auth-client.ts:1515, which drops the session rather than adopting the account. Both places now say omitting hint costs the resume, not the user's identity.

Also fixed on the docs side, all four: the hardcoded returnTo: "/" now reads next, the /reauth snippet constructs its own client and reads getStatus() so it is self-contained, the CookieStateStorage import is shown, and clientOptions passes getIdentityProvider() like every other example on the page.

Eval results, which I owed you from the previous round as well:

Case With skill Baseline
22. Shared sign-in across sibling subdomains 9/9 1/9
23. Adversarial: shared cookie, different principals 4/4 3/4
24. Adversarial: silent re-issue without hint 4/4 2/4
25. Adversarial: redirecting an already-open page 3/3 2/3

Case 22 now asserts the callback document too, and your last point was exactly right: the case had been asking what the derivation origin serves, so it passed while the generated app could not have completed a sign-in. It now asks which origin serves each well-known document, and that expectation failed on the first run after I added it — the model produced ii-auth-callbacks and dropped ii-alternative-origins — which is what got the prompt corrected rather than the score accepted.

Case 24 gained a behavior asserting the failure is a refused resume rather than a wrong identity, so the corrected wording is now tested rather than just written.

@marc0olo marc0olo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Follow-up review: internet-identity skill for @icp-sdk/auth v9

Re-reviewing at bd956ff.

Fixed

  • ii-auth-callbacks is documented per app origin, with the entry shape, the exact-match and no-fragment rules, the _headers block alongside the two the skill already had, the fails-closed behavior, and the terminate-locally caveat. Mistake 14 names it too, so it is reachable from the pitfall list and not only from the walkthrough.
  • The hint wording now says a provider holding more than one session refuses with account_selection_required, and that a mint for an unexpected account is rejected as AccountMismatchError, so what omitting it costs is the resume rather than the user's identity. That matches the source.
  • Eval results posted for all four new cases, with baselines. Case 22 at 9/9 against 1/9 is the useful number, and correcting the prompt after the callback expectation failed on its first run, rather than accepting the score, is the right way round. Case 24 gaining a behavior that asserts the refusal means the corrected wording is now tested rather than only written.

Still needs work

One item, and it is the skill contradicting itself rather than anything about v9.

  • The /reauth snippet uses bare top-level await, twice: await authClient.signIn({ ... }) and await authClient.signOut() sit at module scope inside an if. Line 148 of this same file says the init pattern "wraps async setup in a function so this code works with any bundler target (Vite defaults to es2020 which lacks top-level await)", eval case 1 asserts "All await calls are inside async functions, no bare top-level await at module scope", and another case exists purely to teach the fix for Top-level await is not available in the configured target environment. So an agent that follows this section produces a frontend that does not build on Vite's default target, and the skill's own pitfall is what would have prevented it. Wrapping the body in async function reauth() { ... } with a call after it settles it, and matches how the sign-in flow above is written.

@marc0olo
marc0olo self-requested a review September 16, 2026 16:40

@marc0olo marc0olo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@sea-snake pre-approving to unblock when the fix lands

The route's awaits sat at module scope, which the skill's own init pattern and
eval case 1 both say not to do: Vite's default target has no top-level await, so
a frontend following this section would not build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sea-snake

Copy link
Copy Markdown
Contributor Author

Fixed in 0823cb9. You are right that this was the skill contradicting itself rather than anything about v9: line 148 tells an agent to wrap async setup because Vite's default target has no top-level await, eval case 1 asserts it, and then the section I added put two awaits at module scope.

The route is now async function reauth() { … } with a call after it, and the early return replaces the else branch so the happy path reads straight down. Scanned every fenced snippet in the file for a fence-level await: none left.

The same snippet had landed in dfinity/developer-docs#395 with the same problem, and you had already approved that one, so I fixed it there too (769672e) rather than leave a copy of it in the docs.

Evals after the change: case 22 still 9/9 against 1/9, and case 1, the one that asserts no bare top-level await, 5/5 against 1/5.

Out of scope for this PR, which is the internet-identity skill. The template's
own owners decide when it moves off ^7.1.0 and whether it goes to v9 or v10.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sea-snake
sea-snake merged commit 13f3f58 into main Sep 16, 2026
6 checks passed
@sea-snake
sea-snake deleted the skills/auth-v9 branch September 16, 2026 19:45
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.

2 participants