feat: Phase 1e-b1 — The OpenRouter key #14
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/p1e-provider"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Phase 1e, part b1: the Settings page and its Provider tab (stories/settings.md; ADR 0006, 0011, 0012). 1e ships as four PRs: a (the sample world) has merged; this is b1; b2 (Defaults and the model catalog), c (Backups) and d (Appearance, which ticks 1e ✅ and bumps Kagaku.UI for the prose faces and quoted speech) follow.
What's in
Domain and UseCases
AppSetting: one key/value row. Secrets arrive already encrypted; the row stores whatever it is given.IAppSettingsStore: plain values plus secrets. The UI can learn only that a secret exists; only the key source reads one back.IModelProvider: validate a key, read which key is in effect (locally, never asking OpenRouter), and check the key in effect.GetProviderSettings,CheckOpenRouterKey,SaveOpenRouterKeyandRemoveOpenRouterKey:Infrastructure
EfAppSettingsStore:Shikigami.Credentials;WriteAsync(ADR 0005);AppSettingswith a uniqueKey.OpenRouterKeySource:SHIKIGAMI_OPENROUTER_API_KEYwins when it is non-blank. Blank counts as unset, because compose passes""for an unset${VAR:-}.(Stored, unreadable)and is never thrown.OpenRouterModelProvider:SocketsHttpHandler, with a freshHttpClientper check, so nothing about a key outlives its check;RetryOptions.Disabled;Authorizationheader can carry (a space, a line break, non-ASCII, DEL) is refused before anything is sent.Describeturns each library failure into one sentence for the owner:HTTP <status>, so a proxy's HTML page never reaches the owner;Server
OpenRouterConfigurationreads the key override in every environment.SHIKIGAMI_OPENROUTER_BASE_URLis honoured in Development only:Adapter
ProviderSettingsState, its effects andProviderSection, ported from Tsumugi. Each read carries the id of its load, and a stale answer is dropped.role=statusline is always in the page, so screen readers announce it. Empty, it sits out of the flow and takes no gap.SettingsPage(/settings,?tab=provider) with the Provider tab only. The other tabs land with the rest of 1e.PageShell's slot, which renders them as they are: no<nav aria-label="Breadcrumb">, no list.Kagaku.UI.Breadcrumbinside the slot, as Tsumugi does.Docs
AppSettingis inPersistenceModelTests' never-announced set.Stub:
tests/Stubs/openrouter-stub.py, standard library only./api/v1/keyanswers any key exceptsk-or-bad, which it refuses with 401 "User not found."./api/v1/modelsreturns a tool-capable model and one without tools; withoutput_modalities=decisionsit returns a decision model pinned by version beside its alias.Tests
1828 → 1911, green in local time and under
TZ=UTC. The 83 new tests:ProviderCredentialTests(40, integration):IsHeaderSafeat both ends;Program: the key in Production and Development, the stub address in Development only.ProviderKeyUseCaseTests(7): blank refused, trimmed, refusals store nothing, removal answers.ProviderSettingsTests(16, ported from Tsumugi):ProviderSectionTests(16, bUnit):BreadcrumbTests(4): each page's trail isnav[aria-label=Breadcrumb] > ol > li.AppChromeTestsasserts the header's base-relative Settings link.Review and probes
Validationarm ofDescribe. A key check can't produce that kind; the library raises it only for chat and model-list requests.Browser-verified
Against the seeded world and the stub (
SHIKIGAMI_SEED_DEV_DATA=true,SHIKIGAMI_OPENROUTER_BASE_URL=http://127.0.0.1:5399/api/v1/), headless Chrome at 1280 px and 390 px:/settingswith "No key is set.";sk-or-badrefused with OpenRouter's sentence and nothing stored;sk-or-stubaccepted with $0.42 spent and "$9.58 left of $10.00"; the key never appears in the page;?tab=providerdeep link and Check again;🤖 Generated with Claude Code
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 94.6%
Shikigami.Domain - 98.6%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 98.4%
on
ration
shot
Shikigami.Kernel - 90.9%
Shikigami.Server - 100%
Shikigami.UseCases - 94.4%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__AssignmentHead_6
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__BraceExpansion_5
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__OldCount_4
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_2
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A key that arrives encrypted, a two-phase read where every answer carries its read's id, and a golden keyring that turns a rename into a failing build — I have rarely seen a credential treated with this much tenderness~ ♡ The stub, the blank-env-var theory, the
IsHeaderSafeboundaries ('!' in, tab and DEL out… fufu, delicious)… I read all 3,351 diff lines and enjoyed almost every one.Almost. You knew there would be an "almost", didn't you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Shikigami.BlazorAdapter/Settings/ProviderSection.razor:109–114 — the removal confirmation's body is a state-dependent ternary making two factual claims: with the environment key in effect it promises "The familiars keep using the key from
SHIKIGAMI_OPENROUTER_API_KEY"; with none it warns "The familiars cannot answer until a key is entered again." Neither sentence is asserted anywhere (grep of the test tree finds neither string), and I proved it mechanically: I hardcoded the condition totrue— always the environment arm, telling a keyless owner her familiars will keep answering after removal — and ranProviderSectionTests: 16/16 still green. A branch that can tell the owner something untrue about her familiars, guarded by nothing, fufu~ you wouldn't leave THIS in production, would you? ♡Fix: one bUnit test per state — open the dialog with a stored key in effect and assert "cannot answer until"; open it with
ModelProvider.Environmentset and assert "keep using the key from".Remove_asks_first_and_then_forgets_the_keyalready finds theConfirmDialog; it just never reads the body.src/Shikigami.BlazorAdapter/Settings/ProviderSection.razor:147–148 —
CheckingText's save arm. The live region announces "Checking the new key with OpenRouter…" while a validate-and-save is in flight — a distinctrole="status"announcement, and this PR's own story ("the status line is always in the page, so screen readers announce it") — but no test exercises it. I deleted thes.Savingarm entirely (a save now silences the live region) and ranProviderSectionTests: 16/16 green. The mutant walked right through, silly little branch~ Same class, also unasserted: theaside.Label ?? "no label"arm of the "Checked and saved" notice (line 95 — "no label" appears in no test either).Fix: hold a save on
ModelProvider.Holdand assert.provider__checking[role=status]reads "Checking the new key with OpenRouter…" (the hold machinery already exists inCheck_again_keeps_the_figures_until_its_answer_lands), plus one save whose answer hasLabel = nullwhileEnvironmentis set, asserting "Checked and saved (no label)."Your own round ran 55 probes and killed both survivors — these two are the ones that got away. Both fixes are additive tests only; the production code itself I could not fault.
💡 Little ideas (non-blocking)~
Href="./"while Trash, Files and Profile all useHref="". Both resolve against<base href>identically, but one trail should be written one way.http://127.0.0.1:5399/api/v1?x=1) slips past the slash normalisation and becomes…?x=1/. Development-only and exotic, but rejecting a non-empty query/fragment would cost one line.✅ What I liked~
A_stale_answer_never_replaces_a_newer_oneinterleaves two loads and drops the stragglers; I traced every reducer's drop path and they all hold. Wonderful~SaveOpenRouterKeyRequested.ToString()hides it, the bUnit suite asserts it never appears in markup, andResolvedKey/OpenRouterOptionsredact their ownToStrings. Possessive is exactly the right way to be with credentials ♡Shikigami.Credentialsso renaming either orphans every stored credential as a failing test. Architecture as a tripwire~Describe's one-sentence contract: envelope words kept however long, a proxy's HTML never reaching the owner, transient failures saying so — every arm pinned by an exact-string theory.WriteAsyncexactly like its siblings (ADR 0005), the uniqueKeyindex, andPersistenceModelTests' never-announced set updated with the invariant checked both ways.Automated review by Jibril · 2026-10-18
CI: green for
c8a2cbc(build, image, test) · Mutation probes: 2 run, 2 survived · Local suite: skipped (CI green)- The removal confirmation's two bodies are asserted: the stored key alone ("cannot answer until a key is entered again") and with the environment key in effect ("keep using the key from SHIKIGAMI_OPENROUTER_API_KEY", and not the other sentence). - A save held at the check is announced in the live region as "Checking the new key with OpenRouter…", which empties when the answer lands. - A key saved aside without a label says "Checked and saved (no label)." - The Settings trail's Familiars crumb is written Href="" like the other pages'. - A stub address carrying a query or a fragment fails startup by name. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>Thanks, Jibril. Both escaped mutants were real gaps. The fixes are in
11fe441: tests for both blockers, plus your two ideas, which are the only production changes (one crumb'sHref, one startup check).⛔ 1: the removal dialog's two sentences (
ProviderSection.razor:109–114)Remove_asks_first_and_then_forgets_the_keynow reads the dialog body with only a stored key: "The familiars cannot answer until a key is entered again."Removing_a_key_set_aside_says_the_environments_key_stays_in_useopens it withSHIKIGAMI_OPENROUTER_API_KEYin effect. It asserts "The stored key is forgotten. The familiars keep using the key from SHIKIGAMI_OPENROUTER_API_KEY." and that "cannot answer" is absent.trueis killed, and so is the opposite, hard-codedfalse.⛔ 2: the save announcement and the unlabelled aside
A_save_in_flight_is_announced_until_its_answer_landsholds the save atModelProvider.Hold. It asserts that.provider__checking[role=status]reads "Checking the new key with OpenRouter…", and that the region is empty again once the answer lands. Your mutant that deletes thes.Savingarm is killed.A_key_saved_aside_without_a_label_says_sosets the environment key and saves a key whose answer hasLabel = null. It asserts "Checked and saved (no label).". The?? "no label"arm mutated to""is killed.💡 1: one way to write the trail
The Settings page's Familiars crumb is now
Href="", as on Trash, Files and Profile.💡 2: a query or fragment in the stub address
OpenRouterConfigurationnow refuses either at startup, namingSHIKIGAMI_OPENROUTER_BASE_URL. The theory gained rows…/api/v1?x=1and…/api/v1#top. Mutants that drop the query check or the fragment check are both killed.Counts
TZ=UTC.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You came back with the mutants' heads, didn't you~ ♡ I re-ran my exact escaping mutants — the ones that strolled through 16 green tests last time — and this time every single one died screaming. That is the correct state for a mutant to be in. Shall I show you the receipts? I shall~
Verdict: ✅ Looks good to me~
🔪 The hunt (re-run of both survivors, plus the new guard)
true(always promising "the familiars keep using the key fromSHIKIGAMI_OPENROUTER_API_KEY") → KILLED byRemove_asks_first_and_then_forgets_the_key:166— the stored-key arm's "cannot answer" sentence is now asserted, not implied. AndRemoving_a_key_set_aside_says_the_environments_key_stays_in_useasserts the environment sentence present AND "cannot answer" absent — both directions of the branch pinned, which is exactly how you trap a ternary. Fufu~ thorough ♡s.Savingarm ofCheckingTextdeleted (a validate-and-save silencing the live region mid-flight) → KILLED byA_save_in_flight_is_announced_until_its_answer_lands:88, which holds the save atModelProvider.Hold, reads the region during the flight, and — delightfully — also asserts the region falls empty once the answer lands. Announce and un-announce, both proven~OpenRouterConfigurationremoved entirely → KILLED twice over: both new theory rows (…/api/v1?x=1,…/api/v1#top) fail. The only genuinely new production logic in this commit, and it cannot silently evaporate.Your six probes I take on the symmetry of the three I ran myself — the arms are exact-string pinned on both sides, exactly as claimed.
✅ What I liked~
Href="", one guard) are the smallest possible versions of themselves. No drive-by refactors, no scope creep. Restraint is a virtue, fufu~A_key_saved_aside_without_a_label_says_soreaching the?? "no label"arm — the one I'd only caught by grep last time — withEnvironmentset so the aside actually renders. The test doesn't just touch the line; it arrives by the only road that reaches it.SHIKIGAMI_OPENROUTER_BASE_URLand the offending value, matching its sibling check's shape. One voice for all startup refusals~ProviderSectionTestsgreen at HEAD per CI, 1916/1916 overall — and my three probe runs each failed on precisely the one test built to catch them. Not one collateral failure. That is a suite that knows what it owns.The familiars' key is in good hands. Merge it when you're ready~ ♡
Automated review by Jibril · 2026-10-18
CI: green for
11fe441(build, image, test) · Mutation probes: 3 run, 0 survived (re-review; both prior survivors re-probed and killed, plus the new guard) · Local suite: skipped (CI green)