[nonamelane]

inkstone 0.1.5: the window closes within about half a minute, refused keys are removed instead of dead-ending, and an unchanged body goes out byte-for-byte

Closes #839. Closes #840.

Follow-up to #832 (0.1.3), rebased on #841 (0.1.4), covering the second review
round on this PR.

Blocking

1. The cookie probe fires on an old session. keep_sign_in keeps the
   previous uaa-token row, so "a row exists" is not "this sign-in finished".
   The probe now records the rows present at launch (creation_utc /
   last_update_utc) and accepts only a row that is new or whose
   last_update_utc moved; it also filters expires_utc > now and limits
   host_key to the console's domain. The database is copied into the 0700
   profile folder for the query, not into the system temp folder.
2. inkstone: the /keys pagination test doesn't reach keys.mjs #839 / the strict serveKeys() fake. inkstone: strict fake /keys, refused-key cleanup test, injectable probe backoff #841 landed the strict fake
   first, as asked; this PR rebases on it. The fake now also serves DELETE,
   and a new test drives the orphan-deleting listKeys through it, so every
   listKeys caller is held to the fake.
3. The "created but not returned" path is no longer a dead end. The
   orphan is deleted by name and the create retried with a fresh name
   (-15108 and the -15100 soft refusal are handled the same way); the
   "create one in the console and paste it instead" wording is gone.
   softRetryDelays is injectable, mirroring inkstone: strict fake /keys, refused-key cleanup test, injectable probe backoff #841's probeDelays.
4. The production wiring is tested. One test injects cookieProbe
   through signInWithBrowser (removing the wiring turns it red); another
   runs the real sqlite probe against a kept profile that still carries the
   previous sign-in's row (dropping the launch baseline turns it red).
5. The ~30 s cookie flush. The instructions and the README now say the
   window closes "within about half a minute" after the console session
   lands (Chrome writes cookies to disk on a timer), not straight away.

Non-blocking

• rewriteBody returns an unchanged body byte-for-byte again (an internal
  marker records whether anything was rewritten); a test pins it.
• In chatFetch, a Request whose body is not JSON is re-attached after it
  is read, so the upstream call never carries a spent body; tested through
  the loader fetch.
• The cleanDetail JWT test input is back to main's.
• The fake-browser build and sweep are scoped to this tree's fingerprint, so
  a concurrent run of another checkout is neither renamed away nor swept.
• The README's "identical output" is now a single observed live check, not a
  promise for every model.
• The cookie database for the probe is copied into the 0700 profile folder.
• "the console title is never read" now counts titleOf calls and asserts 0
  off Windows.

Version

0.1.4 → 0.1.5 (rebased on #841, which published 0.1.4).

What was run

• bun test packages/inkstone: 116 pass / 0 fail (627 expects) in
  Asia/Shanghai, UTC, America/Los_Angeles and Pacific/Kiritimati.
• bun scripts/check.mjs: exit 0 (all packages).
• Nine breaks verified red: the launch baseline dropped (B1), the production
  cookieProbe wiring removed (B4), orphan deletion off (B3), the -15108
  dead end restored, the pagination query restored at every listKeys call
  site and separately at the orphan-deleting call site (B05/B2), rewriteBody
  reserializing an unchanged body (N1), the Request body not re-attached
  (N2), and the title read on darwin (N7).

[yetone]

Thanks, this is a real improvement to the sign-in, and the wiring tests on step ④ are good. I ran the inkstone tests (106/106), check.mjs inkstone and the root suite at a9f500f against main 7d84184 on macOS (Bun 1.4.2), and drove waitForConsoleWindow with profileHasCookie against a real Chromium 151 on macOS and Linux.

Blocking:

1. The cookie probe fires on an old session. keep_sign_in is on by default, so the profile keeps the old uaa-token row. On a second sign-in into that profile, where the page sets no cookie, waitForConsoleWindow returned "console" after 0–1ms on both macOS and Linux. That also happened after the cookie had expired. The window closes before the user can sign in, and the headless read then gets a stale token or none. That's what happens on every re-sign-in after the session expires, and when adding a second account. Please only accept a row that is new since launch (record its creation_utc/last_update_utc or its value before spawning), filter on expires_utc > now, and limit host_key to the console's domain.
2. inkstone: the /keys pagination test doesn't reach keys.mjs #839 is still open. I put the page/pageSize query back at each listKeys caller one at a time. Only keyCount goes red; keyCreatedUnderName, pruneKeys and the 401 cleanup all stay green. The serveKeys() fake every key test uses should refuse a query with -15100 and a bearer-only request with -10002, so every caller is held to it.
3. The "created but not returned" path is a dead end. Nit 7 in inkstone 0.1.3: image-caps preflight, a zero-prompt browser sign-in, and the #824 follow-ups #832 was minor: an orphan key the next prune clears. Now sign-in fails and the orphan stays. Delete the key by name/id and retry with a fresh name, or carry on with the next attempt, rather than ask the user to paste a key.
4. Nothing tests the production wiring. Removing cookieProbe from signInWithBrowser keeps all 106 tests green.
5. Chrome writes cookies to disk on a ~30s timer. Detection took ~30s after the cookie was set in both runs, so the instructions and README should say the window closes "within about half a minute", not straight away.

Non-blocking:

• rewriteBody now re-serializes every body, where an unchanged one used to go out byte-for-byte.
• In chatFetch, a Request whose body isn't JSON is read and then not re-attached.
• The cleanDetail JWT test input changed without a reason (the old input passes on main).
• The fake-browser sweep deletes other inkstone-fake-browser-<user>-* builds, including one from a concurrent run of another tree.
• The README's "answered 200 with identical output" needs a source or softer wording.
• Copy the cookie database into the 0700 profile folder, not the system temp folder.
• "the console title is never read" asserts nothing about titleOf.

The 0.1.4 bump is right; dist has up to 0.1.3. I'd land #841's stricter test first and rebase this on it.

[yetone]

#841 landed first as e7af785 (inkstone 0.1.4). It changes lib/keys.mjs (probeWithRetry takes optional delays, and ensureKey passes probeDelays through) and makes serveKeys()'s fake /keys strict: no uaa-token cookie gives -10002, and any query gives -15100.

This branch still merges cleanly on top (git merge-tree shows no conflicts). It also says 0.1.4, though, and 0.1.4 is now published from #841. Please rebase on main and bump inkstone to 0.1.5. Once rebased, check that the new keyCreatedUnderName list and its -15100 test still pass under the strict serveKeys() fake.

[nonamelane]

Rebased on main (b228cc6, includes #841) and bumped to 0.1.5. All five blocking points and the seven nits are addressed.

Blocking

1. The probe records the profile's uaa-token rows at launch and accepts only a row that is new since then (or whose last_update_utc moved), with expires_utc > now and host_key on the console's domain.
2. inkstone: strict fake /keys, refused-key cleanup test, injectable probe backoff #841's strict serveKeys() fake is the base. It now also serves DELETE, and a test drives the orphan-deleting listKeys through it, so every listKeys caller is held to it.
3. A refused create deletes the orphan by name and retries with a fresh name; -15108 and the -15100 soft refusal share that path, and the "create one in the console and paste it instead" wording is gone.
4. One test injects cookieProbe through signInWithBrowser; another runs the real sqlite probe against a kept profile that still carries the previous sign-in's row.
5. The instructions and the README say the window closes "within about half a minute" (Chrome flushes cookies on a timer).

Nits: rewriteBody passes an unchanged body through byte-for-byte; chatFetch re-attaches a read non-JSON Request body; the cleanDetail JWT input is main's again; the fake-browser build/sweep is scoped to this tree; the "identical output" note is softened; the probe copy lives in the 0700 profile folder; the title-never-read test counts titleOf calls.

Verification: 116/116 tests in four timezones, check.mjs exit 0, and nine breaks verified red (each of the points above, including the pagination query restored separately at the orphan-deleting call site). The diff carries no personal data.

One environment note: the root bun test on this Windows machine fails five tests in scripts/opencode2.test.mjs because it URL-encodes the workspace path and this checkout lives under a CJK path (AI与网关); that file is untouched by this PR.

[yetone]

Thanks, this round closes most of it. I checked each earlier point against the diff at 07e82ba and ran it on macOS (mote, Bun 1.4.2), each run in a fresh HOME:

• cd packages/inkstone && bun test: 116 pass, 0 fail.
• bun scripts/check.mjs inkstone: exit 0.
• repo-root bun test: 6685 pass, 22 fail. main f379bf9 gives 6700 pass, 21 fail; the extra red is inkstone's (blocking 1 below). The other 21 (Devin and unnamed) are the same on main.
• Hand breaks: dropping the launch baseline ([] for baseline at lib/browser.mjs:650) turns "an old cookie in the kept profile" red. match = null in deleteKeyByName turns 4 key tests red. Putting { query: { page: 1, page_size: 50 } } back on its listKeys turns "the orphan-deleting listKeys" red. Each is held by a test now.

Done: blocking 1 (baseline, expiry, host_key), 2, 3 and 5, and every nit. I checked the 226 drop against magpie's shapeRefused (400 and 422 only). The probe copy is in the profile, and the sweep is scoped to this tree.

Blocking:

1. The new Request test is red in the repo-root bun test. The failing test is "the fetch wiring is the request path", at packages/inkstone/api.test.mjs:331 (loader.fetch(req)): TypeError: fetch() request with GET/HEAD method cannot have body. It passes alone. bun test packages/catpaw packages/inkstone/api.test.mjs shows the cause: packages/catpaw/index.test.mjs:37 leaves a global fetch wrapper that calls realFetch(url, init), which drops the Request's method. That leak is catpaw's (filed as #877, with a kiro leak that breaks the same test on main). But chatFetch is what depends on it. At index.mjs:96 it hands back { ...init, body: text } with no method, and at :98 it calls upstream(input, init) with a Request whose body has been read. Any fetch in between that reads input.url sends a GET. When you re-attach the body, pass method and headers from the Request too, for example upstream(input.url, { method: input.method, headers: input.headers, signal: input.signal, ...init, body: text }). Do the same when a rewritten body is attached to a Request. The test then passes after catpaw too.
2. Blocking 4 is only half covered. At lib/browser.mjs:649 I replaced loginWait?.cookieProbe ?? ((probeProfile) => profileHasNewCookie(...)) with loginWait?.cookieProbe. That way macOS and Linux get no probe at all. All 116 tests stay green: the positive test at fake-browser.test.mjs:252 injects its own cookieProbe, and the negative one only checks that "console" is not reached. The fake browser already writes a new uaa-token row 300ms in. Drop the injected probe from the positive test (keep platform: "linux", pollMs), so it ends on "console" through the real sqlite probe and the launch baseline.

Non-blocking:

• lib/keys.mjs:233: when the last no-plaintext retry gets -15108 or the soft refusal, the loop continues and ends without deleting that attempt's key. The message at :241 still says "the orphan keys were removed". deleteKeyByName also logs and returns when the list fails. Delete after the last attempt too, and only say "removed" when the delete ran.
• -15108 from inside the -15109 retry loop (:174) still goes out raw, with no orphan cleanup.

[nonamelane]

Fixed both blocking points and the two non-blocking ones at 1925a25.

Blocking 1. chatFetch now forwards the Request's method, headers and signal into init and calls upstream(input.url, init) whenever it has read the Request's body — both when the body is re-attached and when a rewritten body is attached. bun test packages/catpaw packages/inkstone/api.test.mjs is now 79 pass / 0 fail (the combination that used to go red through catpaw's leak).

Blocking 2. The positive production-wiring test no longer injects a probe: it runs signInWithBrowser with loginWait: { platform: "linux", pollMs: 50 } and ends on "console" through the real sqlite probe and the launch baseline. Your exact break (cookieProbe: loginWait?.cookieProbe, no production probe at all) now turns it red. Verified on Windows as well as the four timezones.

Non-blocking 1. deleteKeyByName returns whether it actually deleted a key. The last no-plaintext attempt's orphan is now deleted after the loop too, and the error only says "the orphan key was removed" when that delete ran; otherwise it says the orphan could not be deleted here and points at the console's API key page.

Non-blocking 2. A -15108 arriving inside the -15109 retry now deletes the orphan and reports the clear no_plaintext message instead of throwing the raw error.

Verification: 119/119 tests in four timezones, check.mjs inkstone exit 0, and twelve hand-breaks verified red (including the last-orphan deletion, the -15108-inside--15109 path, and the "removed" wording). The repo-root suite still cannot run to completion on this Windows checkout (unrelated packages hang there), but the exact catpaw + api.test combination you cited is green.

[yetone]

Thanks, both blocking points hold now. I checked every changed line since 07e82ba against the last review, on 1925a25 merged onto main bc5b833 (no conflicts, main hasn't touched inkstone, catpaw or scripts since your base), on macOS (mote, Bun 1.4.2), each run in a fresh HOME:

• cd packages/inkstone && bun test: 119 pass, 0 fail.
• bun scripts/check.mjs inkstone: exit 0.
• bun test packages/catpaw packages/inkstone/api.test.mjs: 79 pass, 0 fail.
• Hand breaks, each red on exactly the test named for it and green again when restored:
  • dropping the method/headers/signal forwarding and going back to upstream(input, init): "the fetch wiring is the request path" is red in the catpaw combination (78/1).
  • cookieProbe: loginWait?.cookieProbe with no production probe: "off Windows the production wiring closes the window on a new cookie" is red.
  • removing the post-loop delete at lib/keys.mjs:259: "deletes the LAST orphan too" is red.
  • deleteKeyByName returning true when the list fails: "does not claim it was removed" is red.
  • removing the plaintextLost branch in the -15109 loop: "-15108 inside the -15109 retry" is red.

Blocking 1 and 2 are done, and so is the -15108-inside--15109 nit.

One thing still needs fixing. It's the "only say removed when the delete ran" point, which this commit says is fixed:

1. removed goes stale, and the last orphan is kept while the message says it was removed. lib/keys.mjs:187 sets removed once for the whole function. :204, :230 and :251 set it only on a plaintextLost attempt. Then :259 skips the final delete whenever an earlier attempt left it true. Here is a sequence that does that: the first POST succeeds with no plaintext. In the loop, attempt 0 deletes it, and the next POST gets -15108, so :251 deletes that one too and sets removed = true. Attempt 1's POST succeeds with no plaintext again. The loop ends with created === "", :259 sees removed and doesn't delete, and the user gets "the orphan key was removed". I ran exactly that against the fake on mote: 3 POSTs, key k3 is still on the account, and the error says "the orphan key was removed". The same thing happens when removed was set in the -15109 or soft-retry branch and a later create succeeds without a plaintext. Fix: at :259, delete the current name every time unless the last attempt was the plaintextLost one that already deleted it. For example, reset removed = false right after each successful createKey in the loop (and set it per attempt, not once per function). Then add the test above: -15108 in the middle, no-plaintext success last, and expect 0 keys left.

Non-blocking:

• Swapping only upstream(isRequest ? input.url : input, init) back to upstream(input, init) (keeping the init forwarding) leaves every test green, catpaw combination included. That's harmless today, because Bun lets init.body replace the spent stream. Just know that nothing tests that half.

I didn't run the repo-root suite this round: mote's load was ~960, and the PR touches only packages/inkstone. I'll run it at the next head before merging.