Skip to content

inkstone 0.1.4: the sign-in completes itself off Windows, and the key-list wiring guarded where the query lives - #847

Open
nonamelane wants to merge 2 commits into
magpie-community:mainfrom
nonamelane:inkstone-0.1.4
Open

nonamelane wants to merge 2 commits into
magpie-community:mainfrom
nonamelane:inkstone-0.1.4

Conversation

@nonamelane

Copy link
Copy Markdown
Contributor

inkstone 0.1.4: the sign-in completes itself off Windows, and the key-list wiring guarded where the query lives

Closes #839. Closes #840.

Follow-up to #832 (0.1.3), covering the B05 gap found in review and the #825
follow-up, plus every nit.

B05 (#839): the wiring test now reaches keys.mjs

The wiring test's key-list step called protocol.listKeys directly — the
query lives at the lib/keys.mjs call sites, so putting it back left the
package green (B05, found reviewing #832). Step ④ is driven through
ensureKey now, and the fake refuses a paginated /keys with -15100: with
the query restored at all four call sites (the three originals plus the
new verifier below), the test is red.

#840: the sign-in completes itself off Windows

Off Windows the only completion signal was quitting the browser, and the
headless read showed no progress (HuHong-Tao's #825 log). Two changes:

  • The profile's cookie store is the completion signal. A new
    lib/cookieProbe.mjs polls the profile's Default/Cookies sqlite for the
    uaa-token row (the name column is plaintext; the copy-then-query dance
    sidesteps Chrome's lock). The wait closes the window itself once the
    session lands — the same behavior Windows already had. The probe is
    swappable, the old exit-only path stays as the fallback, and the log
    carries the progress: watching the cookie store → closing the window →
    reading the session → waiting for the cookie → creating the key.
  • The instructions and the README say the window can be left open on
    every platform.

The probe is tested against a real (test-built) sqlite database: the row
found, other cookies not matching, a profile without the store, a file that
is not a database, the probe ending the wait, and zero title reads off
Windows. macOS verification itself stays with the #825 reporter, as agreed.

The nits

  1. chatFetch carries no dead key parameter and no empty try/catch.
  2. The fake browser sweeps its sibling hashes (a source change leaves no
    ~60 MB directory behind) and builds through a pid-suffixed temp name
    (two concurrent runs cannot race on one temp file).
  3. The inkstone: "Browser sign-in" is offered where no Chromium browser exists, then fails only in the callback while its own instructions say "a browser window opens" #825 test also asserts the reuse-a-key method fails at authorize.
  4. The test's hostShapeRefused copy no longer accepts status 226.
  5. The body is parsed once — precheckImages(obj) and the new
    rewriteBodyObj(obj) share the object (near the 40 MB cap, twice was
    waste); rewriteBody(text) remains as a wrapper.
  6. The README's "silently ignores" now says how it was checked (the three
    reasoning fields answering 200 with identical output on the free models).
  7. The soft-refusal retry checks the key list before it retries: a -15100
    that actually created the key is answered with the -15108 wording (the
    plaintext is shown once), never a second ink-* key. Tested with a fake
    that creates on the first POST and refuses with -15100.

Version

0.1.3 → 0.1.4.

What was run

yunwang2442-creator added 2 commits November 8, 2026 01:09
…-community#840 - the wiring test's key-list step is driven through ensureKey with a fake that refuses a paginated /keys (B05: the query going back now turns it red), chatFetch parses the body once and carries no dead key or empty catch, the fake browser builds under a pid-suffixed temp name and sweeps its sibling hashes, the magpie-community#825 test covers the reuse method, the host-shape copy drops the status the host does not take, the soft-refusal retry checks the key list before it builds a second key, and off Windows the sign-in watches the profile's cookie store for the session so the window closes itself (magpie-community#840)
… key-list wiring is guarded where the query lives
@yetone

yetone commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

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 pushed a commit that referenced this pull request Nov 8, 2026
…le probe backoff (#841)

The fake GET /api/tokenplan/v1/keys in keys.test.mjs's serveKeys() now
behaves like the live platform: no `uaa-token` cookie gives -10002, and
any query parameter gives -15100. A new test covers the third listKeys
call site, the 401 cleanup in ensureKey: the probe refuses the new key,
and the test checks that the refused key is deleted by name (#839).

probeWithRetry takes its delays as an optional third argument, and
ensureKey passes `probeDelays` through to the probe of a key it just
created. The default delays are unchanged, so nothing outside tests
behaves differently. The new test passes [0] and doesn't wait through
the ~15s backoff. Because lib/ changed, inkstone goes to 0.1.4.

Run at the reviewed head c4ab471, merged onto main 7e01fa8, on mote
(Bun 1.4.2) with a temp HOME:
- packages/inkstone `bun test`: 102 pass / 0 fail
- `bun scripts/check.mjs inkstone`: ok
- repo-root `bun test` and the full `bun scripts/check.mjs`: CI run
  42476974057 at this head, green. The mote run waited 23 minutes
  behind 10 queued suites on the machine's suite lock and was dequeued
  without running.
Broken by hand, each red:
- query back at keyCount's listKeys: 3 fail (blank-prompt create,
  ten-key limit, the new cleanup test)
- query back at pruneKeys' listKeys: 2 fail (both pruning tests)
- query back at the 401 cleanup listKeys: 1 fail (the new test;
  main's tests stayed green there)
- listKeys on the bearer instead of the cookie: 7 fail
- probeDelays not passed through: 1 fail (the new test runs past
  bun's 5s limit at 15.0s)

#847 (also inkstone 0.1.4) merges cleanly on top, but it has to move to
0.1.5.
@yetone

yetone commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants