feat(presentation): the companion's own sign-in for several servers; a failed browser start no longer logs the sign-in address #53

2026-11-11T07:13:00Z

james.maes commented

2026-11-11 07:13:00 +00:00

Step 3 of the connection wizard build (ADR-0005): the companion's own sign-in, rebuilt for several servers, plus a fix to the existing connection dialog that stops a failed browser start from writing the sign-in address to the log.

The fix (operator-visible)

When Windows could not start the default browser for a browser sign-in, the companion's log entry and the dialog's "Browser sign-in not completed" message repeated Windows' own error text, which names what it could not start: the whole sign-in address, with that attempt's state, nonce and code challenge. The same happened for the identity provider's sign-out address, with its ID token hint.
Both now say only that the default browser could not be opened, with the Windows error number, and what to check.
After a sign-out whose provider page could not be opened, the dialog no longer says the page "was opened in your browser"; it says it could not be opened and that the provider's session may still be there.
An unexpected failure of the browser callback inbox is reported as "Browser sign-in not completed" instead of an unhandled error.

Where the leak exists

Sign-in case: every release from 0.2.0-rc.1 to 1.2.1 (0.1.x has no browser sign-in).
Sign-out case: no release. Both cases are in the development snapshot snapshot-2.0.0-rc.1 (not installed anywhere; its release notes list it as a known issue) and on the unreleased release/1.2.2 branch.
2.0 builds from this change on have neither (audit U22). A 1.2.x fix is a separate decision for the owner.

New, not used by the window yet (presentation assembly)

CompanionSignIn: the part of signing in that needs a session and runs as the signed-in Windows user: reads the server's sign-in description, signs in by password or through the browser, returns the session credential.
BrowserCallbackDispatcher: one inbox for browser callbacks, shared by every server's sign-in and sign-out; an attempt is matched by its state, and a callback for no attempt is dropped.
ProviderSignOut: the provider's sign-out, with four outcomes (confirmed, not opened, not confirmed, cancelled).
BrowserLaunch, PresentationLogTemplate: starting the browser without repeating the address; the presentation log can be written to with a message template and its values.
Nothing the operator sees changes through these types in this build: the connection dialog still runs on the existing flow.

Verification

Head 0b9102e on release/2.0 at 2e08344.
build/Verify.ps1 passed on that head: 1,174 Core, 2,321 Windows runtime and command line, 537 presentation tests, none failed; Core 100% of 2,311 lines, presentation 100% of 1,373, command line 100%, Windows 96.08% (4,540 of 4,725; minimum 95).

Review (independent reviewer)

afe7e08: 1 blocker, 4 should-fix, 11 nits.
209c369 (on 4e01519): Verdict on 209c369: 0 blockers, 0 should-fix, 3 nits.
209c369 was replayed onto 2e08344 as 0b9102e after pull request 52 merged. One conflict, in CHANGELOG.md, resolved by keeping both entries. git range-diff reports 25 of 27 patches identical; the other two differ in context only, and the branch's own diff over src, tests, build and installer is line for line the same. The reviewer checked the moved head: Verdict on 0b9102e: 0 blockers, 0 should-fix, 3 nits. (the same three nits; the two patches differ in context only; nothing of the step 8 entry is lost; U22 is still one row and still true).

Left open (to the audit in the next documentation change)

Three nits, all in the existing dialog's sign-out: an inbox failure outside the listed kinds still escapes; one wording case where the inbox itself could not be opened; stricter wording of ReadState.

Not proven here

Nothing ran in the installed companion. The fix is covered by tests on the presentation types and on the dialog's flow with a browser start that fails; a real failed browser start on an installed build is in the upgrade qualification list.

🤖 Generated with Claude Code

Step 3 of the connection wizard build ([ADR-0005](docs/adr/0005-connection-wizard-and-server-tree.md)): the companion's own sign-in, rebuilt for several servers, plus a fix to the existing connection dialog that stops a failed browser start from writing the sign-in address to the log. **The fix (operator-visible)** - When Windows could not start the default browser for a browser sign-in, the companion's log entry and the dialog's "Browser sign-in not completed" message repeated Windows' own error text, which names what it could not start: the whole sign-in address, with that attempt's state, nonce and code challenge. The same happened for the identity provider's sign-out address, with its ID token hint. - Both now say only that the default browser could not be opened, with the Windows error number, and what to check. - After a sign-out whose provider page could not be opened, the dialog no longer says the page "was opened in your browser"; it says it could not be opened and that the provider's session may still be there. - An unexpected failure of the browser callback inbox is reported as "Browser sign-in not completed" instead of an unhandled error. **Where the leak exists** - Sign-in case: every release from 0.2.0-rc.1 to 1.2.1 (0.1.x has no browser sign-in). - Sign-out case: no release. Both cases are in the development snapshot `snapshot-2.0.0-rc.1` (not installed anywhere; its release notes list it as a known issue) and on the unreleased `release/1.2.2` branch. - 2.0 builds from this change on have neither (audit U22). A 1.2.x fix is a separate decision for the owner. **New, not used by the window yet (presentation assembly)** - `CompanionSignIn`: the part of signing in that needs a session and runs as the signed-in Windows user: reads the server's sign-in description, signs in by password or through the browser, returns the session credential. - `BrowserCallbackDispatcher`: one inbox for browser callbacks, shared by every server's sign-in and sign-out; an attempt is matched by its state, and a callback for no attempt is dropped. - `ProviderSignOut`: the provider's sign-out, with four outcomes (confirmed, not opened, not confirmed, cancelled). - `BrowserLaunch`, `PresentationLogTemplate`: starting the browser without repeating the address; the presentation log can be written to with a message template and its values. - Nothing the operator sees changes through these types in this build: the connection dialog still runs on the existing flow. **Verification** - Head `0b9102e` on `release/2.0` at `2e08344`. - `build/Verify.ps1` passed on that head: 1,174 Core, 2,321 Windows runtime and command line, 537 presentation tests, none failed; Core 100% of 2,311 lines, presentation 100% of 1,373, command line 100%, Windows 96.08% (4,540 of 4,725; minimum 95). **Review (independent reviewer)** - `afe7e08`: `1 blocker, 4 should-fix, 11 nits`. - `209c369` (on `4e01519`): `Verdict on 209c369: 0 blockers, 0 should-fix, 3 nits.` - `209c369` was replayed onto `2e08344` as `0b9102e` after pull request 52 merged. One conflict, in `CHANGELOG.md`, resolved by keeping both entries. `git range-diff` reports 25 of 27 patches identical; the other two differ in context only, and the branch's own diff over `src`, `tests`, `build` and `installer` is line for line the same. The reviewer checked the moved head: `Verdict on 0b9102e: 0 blockers, 0 should-fix, 3 nits.` (the same three nits; the two patches differ in context only; nothing of the step 8 entry is lost; U22 is still one row and still true). **Left open (to the audit in the next documentation change)** Three nits, all in the existing dialog's sign-out: an inbox failure outside the listed kinds still escapes; one wording case where the inbox itself could not be opened; stricter wording of `ReadState`. **Not proven here** Nothing ran in the installed companion. The fix is covered by tests on the presentation types and on the dialog's flow with a browser start that fails; a real failed browser start on an installed build is in the upgrade qualification list. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 27 commits 2026-11-11 07:13:00 +00:00

feat(presentation): log entries can be written as a message template and its values ba30561d49

The presentation log took finished strings only, so every caller put its
entry together by concatenation (K22-CODE-CS-10 asks for a template and
values). IPresentationLog gains one template method per level. Each has a
default that renders the template to text and writes it through the
level's text method, so a host that keeps text (the companion's
CompanionLog, the tests' RecordingLog) needs no change and a host that
keeps structured entries can implement them.

PresentationLogTemplate fills the holes in the order the values are given,
in the invariant culture; a hole without a value stays as written, a value
without a hole is left out, and a brace inside a value is never taken for
a hole.

The wizard-and-tree plan gives these overloads to its step 2
(ManagementClient). They are added here because step 3's new types are the
first to need them; the existing callers are not changed.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

feat(presentation): the companion's own sign-in, one dispatcher for browser callbacks, and the provider's sign-out 081f900642

Step 3 of the wizard-and-tree stage (ADR-0005). Three types for the
connection wizard and the per-server Connection page. The window does not
use them yet; the connection dialog still runs on SignInFlow, which is
not touched.

CompanionSignIn is the part of signing in that needs a session: it reads
the server's sign-in description as the signed-in Windows user, signs in
by password or through the browser and returns the session credential. It
hands nothing to the service, enrolls nothing and holds no trust setting;
the certificate exception to read under is an argument. It offers the
methods the service's check and its own read both found, stops when the
description names another sign-in provider than the check reported, and
says whose certificate this account refused, the server's or the
provider's, keeping the refusal for the wizard to show. A cancelled or
timed-out browser attempt keeps its session.

BrowserCallbackDispatcher owns the one callback inbox. An exchange
registers the OAuth state it sent and awaits a ticket; the dispatcher has
an inbox open exactly while a ticket waits, completes the ticket whose
state a callback carries and drops every other callback, logging only
that one was dropped. A failing inbox fails every waiter once, and the
next registration opens a new one.

ProviderSignOut is the provider's half of a sign-out: open its page, wait
up to two minutes for this sign-out's own state.

When Windows cannot start the browser its failure names the whole address
it was asked to open (state, or the ID token hint). The new types report
only the kind of failure.

ISignInSession gains LastCertificateRejection; the companion's adapter
passes the session client's through and the test fake scripts it.

Every entry the new types log is a template with values, and the tests
assert the log's content wherever a secret could have reached it.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(companion): a browser that cannot be started no longer puts the sign-in address in the log or the dialog e37f957cfa

When Windows cannot start the default browser, .NET's failure text names
what it could not start: "An error occurred trying to start process
'<the whole address>' ...". The connection dialog's flow logged that text
and showed it as the reason of "Browser sign-in not completed", so a
failed start at sign-in wrote the provider's authorization address, with
the attempt's state, nonce and code challenge, to the companion log and
the dialog. A failed start at the provider's sign-out logged the
end-session address, with the ID token hint.

SignInFlow now opens the browser through BrowserLaunch at both places, as
CompanionSignIn and ProviderSignOut already do. BrowserLaunch replaces any
launcher failure with fixed text: that the default browser could not be
opened, the kind of failure (for a Windows error, its number), and what to
check. The original failure is not attached as the cause, so no way of
writing the new one out can contain the address. The text now ends with
the remedy, because it is what the operator reads as the reason.

This is a change to SignInFlow, which step 3 of the wizard-and-tree stage
was not to touch. It is kept to exactly this: the two calls that start the
browser, and the comments beside them. The flow is what ships until the
wizard replaces it, so the fix could not wait for that. What the dialog
shows at a sign-out whose browser did not start is unchanged (it never
showed the failure's text there); only the log line changed.

Tests at all four sites use a launcher that fails the way Windows does,
with an address carrying marked state, nonce, code challenge and ID token
hint, and assert that none of them and no query reaches a log line, a
message, or a property a view binds to.

The audit records it as U22. Every tag from v0.2.0-rc.1 to v1.2.1 has the
sign-in case; the sign-out case is in no tag and is on release/1.2.2.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): a browser sign-in whose wait has ended no longer asks the session to complete a6b78ca4d7

Between receiving the callback and completing the sign-in, CompanionSignIn
did not look at the cancellation token, which the dialog's flow does.
Waiting for a callback that has already arrived returns it whatever the
token says, so when the browser came back at once and Cancel was pressed
(or the three minutes ran out) in the same moment, the session was asked
to complete a sign-in for an attempt that was then reported as cancelled.
The real session client refuses before it consumes anything, but
ISignInSession does not promise that, and the comment that said a session
refuses was wrong.

The token is now looked at after the wait on the ticket, and the comment
says why the check is made here.

Tests: the callback reaches its ticket and the wait is ended, by Cancel
and by the limit, before the sign-in starts waiting; the session must
never be asked to complete, and keeps what it holds. The inbox answers
inline for this, so the order of events is certain and no thread is
waited for. A second test pins that the completion is given the
exchange's own token: the token the session was handed is live while it
completes and ends when Cancel is pressed. The test support for both is a
launch hook and an inline inbox in the fakes, and NoContext moves out of
the dispatcher tests to be shared.

Review of afe7e08, B1.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): a discard from a subscriber while an operation is released is carried out at once f9a31ca41f

Discard() decided "an operation is running" by the busy flag. An
operation that is being released clears its wait, announces how it
ended, and only then stops being busy. A subscriber that called Discard()
from that announcement set the discarding flag and cancelled nothing, and
no operation was left to act on it: the session stayed, and the flag
stayed set, so the next read returned an offer and then lost its session
as it ended.

Discard() now decides by the operation's own wait, which exists exactly
while the operation can still discard the session on its way out. When
it is gone, no call is using the session, and the session is discarded
on the spot.

A read could also return an offer whose session was already gone: a
subscriber that discards while the read announces "ready" is honoured
when the read ends. PrepareAsync now returns an offer only while it is
the one that is held, and null otherwise, which is what Offer and
HasSession say at that moment; a debug entry records why.

Tests: the reviewer's probe P5 as a test (a caller-cancelled password
sign-in whose "ready" announcement discards; the session goes at once and
the next read and sign-in work), and a read whose "ready" announcement
discards (null is returned, nothing is held, the next read works).

Review of afe7e08, S1.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): an inbox failure of an unexpected kind ends each waiting exchange as a reported failure c3cf0731b1

The dispatcher fails every waiting ticket with whatever the inbox failed
with, and each exchange catches a fixed list of failures. A failure
outside that list therefore left SignInWithBrowserAsync, from every
sign-in that was waiting, and ProviderSignOut.CompleteAsync as an
exception nobody handles. The real inbox can do that: it throws
ArgumentException when the process that sent a callback has already
exited (Process.GetProcessById), which any process in the user's session
can cause.

BrowserCallbackFailure holds the list every exchange reports (IO, invalid
data, unauthorized access, Windows error, invalid operation, cancelled).
The dispatcher hands tickets the inbox's failure when it is on the list,
and otherwise an IOException with fixed text and the failure's type name,
never its message. The same is done for a failure to open the inbox,
which is thrown to whoever registers.

The dialog's SignInFlow has the same hole for its one sign-in. The
dispatcher change does not close it, because that flow opens its own
inbox, so its wait for the callback now turns an unlisted failure into
the same reported one. Its sign-out has the hole too and is left as it
is: this round changes that file only for the message of a browser that
did not start.

Tests, each with ArgumentException, OverflowException and
TimeoutException carrying a marked text: two waiting tickets; two waiting
sign-ins (the reviewer's probe P4); a waiting sign-out; the dialog's
sign-in (P4b); and an inbox that cannot be opened. Each ends as a
reported failure, nothing escapes, and the marked text is in no log
line, reason, message or bound property.

Review of afe7e08, S2.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(presentation): pin that a callback's state is matched exactly 4729364e84

Nothing in the suite failed when the dispatcher's waiting set was keyed
without regard to case, or when a ticket was completed for a callback
whose state is the beginning of its own. The head matches exactly; the
tests did not say so.

The dropped-callback theory gains four rows against a ticket waiting for
"synthetic-state-x": the same letters in upper case, the state without
its last character, the state with one more, and the state followed by a
space. Each must be dropped and leave the ticket waiting.

Review of afe7e08, S3.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(audit): U22 and the changelog say which tags have the sign-out case 7c4e851ee5

U22 said the sign-out case "is in no tag", and the changelog that it "was
never in a release". The tag snapshot-2.0.0-rc.1 (commit 6f0237d), which
docs/SESSION-STATE.md records as a Gitea prerelease published on
2026-11-10 as a development snapshot and not installed anywhere, has both
cases unguarded (SignInFlow.Browser.cs:152, SignInFlow.SignOut.cs:114).
"Every tag from v0.2.0-rc.1 to v1.2.1" also left that snapshot out, and
the changelog's "every release up to 1.2.1" included 0.1.x, which has no
browser sign-in.

Checked with git tag --contains 2c1c5f1 and 5d4b7f4 and by reading the
browser start at every tag:

- sign-in case: the eleven version tags v0.2.0-rc.1 to v1.2.1, and
  snapshot-2.0.0-rc.1; not in the four 0.1.x tags.
- sign-out case: snapshot-2.0.0-rc.1 only; in no version tag.
- release/1.2.2, unreleased and in no tag: both.
- closed for 2.0 builds after the snapshot; the snapshot stays as
  published.

Both documents now say exactly that.

Review of afe7e08, S4.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): a browser sign-in whose wait ends after the callback was handed over discards the session df45fde261

When Cancel or the three minute limit landed as the session finished
completing, the step ended as cancelled or timed out, no credential was
taken, and the session was kept, signed in on the server. Nothing could
export from it, but a sign-in reported as cancelled should not leave a
signed-in session behind.

CompanionSignIn now notes the moment it hands the callback to the
session. An attempt whose wait ends after that (the operator cancelled,
the limit ran out, or the caller stopped waiting, which had the same
residue under the state "ready") discards the session, withdraws its
offer before the ending is announced, and logs why. An attempt whose
wait ends before the hand-over gave the session nothing and keeps it, as
before, and so does an attempt that failed, whenever it failed. The note
is forgotten when the attempt ends, so it cannot reach a later operation.

The type's summary, SignInWithBrowserAsync, the two states and the
changelog now say exactly when a cancelled or timed-out attempt keeps
its session and when it does not.

Tests: the wait ends at the moment the session completes, by Cancel, by
the limit and by the caller (the reviewer's probe P9 as a theory): no
credential, the session discarded, the offer withdrawn first, a new read
works. Cancel while the session completes discards too. A wait that ends
before the hand-over keeps the session (asserted in the test added for
B1). A failed completion keeps the session, and a later read that is
stopped is not taken for one whose session has to go.

Review of afe7e08, nit 1.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(presentation): the two sign-ins that lose their inbox continue on one thread 8669430dc2

The test added for S2 (faa8865) ends two browser sign-ins with one inbox
failure. Both then continued at the same moment on different threads and
wrote to the recording log, a plain list, at the same time: one entry
could be lost, and the count of "not completed" entries came out as 1
instead of 2. It failed 5 times in 25 runs of that test alone, and once
during an unrelated single-change run, which is how it was noticed.

The test now runs on the single-threaded context the announcement test
uses, as a window would: each sign-in continues on that one thread, one
after the other. 40 runs of the test alone passed.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): a disposed sign-in no longer announces that a property is about to change 2366f6765b

CompanionSignIn promises that nothing is announced after disposal, and
kept that for PropertyChanged only. PropertyChanging was still raised,
for Offer and State, as an operation that was running at disposal ended.

OnPropertyChanging is now held back after disposal as OnPropertyChanged
is.

Tests: the two disposal tests also record PropertyChanging, during a
running operation and when nothing runs, and require that nothing arrives
after Dispose while the offer is withdrawn and the state settles.

Review of afe7e08, nit 2.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): a log that throws cannot break the callback dispatcher e445f42620

The dispatcher wrote "inbox open" after it had made an inbox the open one
and before it started waiting on it. A log that threw there left an
inbox nobody read: the registration failed, and the next ticket shared
that inbox and waited on it until it was disposed. The other two entries
had the same exposure: a log that threw on "dropped" ended the wait for
callbacks while tickets still waited, and one that threw on "inbox
failed" left the tickets of a broken inbox waiting for their own time
limits.

Every entry the dispatcher writes now goes through one method that
leaves a failure of the log with the log: the entry is lost, and the
dispatcher goes on. That covers all three places, where starting the wait
before the first entry would have covered one.

Test: a log that fails on every entry. A registration returns its ticket
with the inbox open and being read, a callback for nobody is dropped and
the next inbox is opened, the ticket's own callback reaches it, and the
tickets of a broken inbox are failed. The test checks that each kind of
entry was attempted.

Review of afe7e08, nit 3.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

refactor(presentation): the callback dispatcher works through inboxes in a loop 59a8057bd6

Opening an inbox started a wait on it, and the end of that wait opened
the next inbox, which started the next wait. When a wait was already
complete as it started, all of that happened inside the call that opened
the first inbox, one level deeper for every callback: 200 inboxes that
answered at once gave a stack 1,560 frames deep. The real inbox never
completes synchronously, so this did not happen in use, but nothing in
the type said it could not.

The wait is now one loop for a whole run of inboxes. Opening an inbox no
longer starts a wait: a registration starts one, and a wait that has
routed a callback goes on with the inbox that was opened for the tickets
still waiting. What happens to a callback, to a closed inbox and to an
inbox that cannot be opened is unchanged.

Test: 200 inboxes in a row answer at once with a callback for nobody.
All are dropped, the stack is no deeper when the last inbox is opened
than when the second was, and the ticket's own callback then reaches it.

Review of afe7e08, nit 4.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

refactor(presentation): the callback ticket and its registration are internal 8890357ccd

BrowserCallbackDispatcher.Register, BrowserCallbackTicket and the
ticket's Callback were public. Callback is a Task<Uri> that carries a
browser callback with its one-time code and state. Only the presentation
assembly's own sign-in and sign-out register and wait, so all three are
now internal; the dispatcher's public surface is its constructor, which
the companion needs to create the one instance.

Test: the ticket is not a visible type and has no public property, and
the dispatcher declares no public method or property and one public
constructor.

Review of afe7e08, nit 5.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): a ticket that was failed and never awaited raises no unobserved task exception 9a95ed6b08

When the inbox failed after an exchange's own wait had ended and before
the exchange disposed its ticket, the ticket's task was failed and nobody
ever looked at it. The runtime reports such a task through
TaskScheduler.UnobservedTaskException when it is collected.

The ticket now marks the failure as seen when it is failed. Whoever does
await the ticket still gets the failure.

Test: a ticket is failed, disposed without being looked at and let go,
and everything is collected. An ordinary failed task, let go the same
way, is the control: it is reported, which shows the collection did what
it had to, and the ticket is not. The two are told apart, and from any
other test's failures, by their own texts. Nothing is waited for: the
collections and finalizers run to completion before the counts are read.

Review of afe7e08, nit 10.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): a callback the session would refuse to read is dropped, not handed to a sign-in f04d882ee9

The dispatcher read the state of a callback more leniently than the
session reads a callback: "??state=x" read as "x", and entries without
"=" were ignored. Such a callback was handed to the exchange whose state
it seemed to carry, the session then refused it, and that ended the
sign-in. Sending one needs the state.

ReadState now applies the rules of Core's callback parser: an absolute
address without fragment or user information, a query of 2 to 16,384
characters, every entry a name and a value, no name repeated, no control
character in a name or a value. An address that breaks one of them has no
state, so the callback is dropped as nobody's and the exchange goes on
waiting for the real one. The same reading is used for the authorization
address a session prepares, which Core builds from distinct names.

One consequence for a provider sign-out: its callback is matched by
QqqProviderLogout, which is laxer about names other than "state". A
sign-out callback that repeats another name or holds a control character
is now dropped and the sign-out waits out its limit, where it would have
been confirmed.

Tests: the reviewer's cases and the remaining rules as rows of the
ReadState theory (a second question mark, an entry without a value, a
repeated name, a control character in a value and in a name, a fragment,
user information, an empty query) and the length limit at its edge; five
of them again through the dispatcher with the right state in them, where
each must be dropped and leave the ticket waiting.

Review of afe7e08, nit 11.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(presentation): pin that a table-based server names no provider and that a ticket's waiter continues elsewhere 295906ac41

Two claims in the new types survived a single change each with every
test passing.

"A table-based server names no provider" (CompanionSignIn.Judge): a
table-based description that also carries a native grant with an issuer
is now read against a check that reported no provider. The issuer must
not be taken for a provider that changed, and the password is offered.

"Whoever awaits a ticket continues on its own thread, never inside the
dispatcher" (the ticket's RunContinuationsAsynchronously): a waiter that
asks to be continued inside the call that ends the ticket must not be
running on that thread while that call runs, for a callback and for a
failure. The test compares threads and a flag the ending call holds, and
waits for nothing but the waiter itself.

Review of afe7e08, nit 6.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): a provider sign-out its caller stopped is logged as information, in fixed words b37dad505c

ProviderSignOut logged every way of not being confirmed as a warning with
the failure's type and text, the caller's own cancellation included:
"WARN Identity provider sign-out was not confirmed:
TaskCanceledException: A task was canceled." A caller that ends the wait
has stopped it on purpose.

That case is now logged at information level as "Identity provider
sign-out was stopped by its caller before the provider confirmed.", with
nothing of the cancellation in it. Failures are logged as before.

Test: the row for the caller's token expects that entry and no warning;
every other row still expects its warning.

Review of afe7e08, nit 9.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

feat(presentation): the provider sign-out says how it ended, not only whether it was confirmed e46d612788

ProviderSignOut.CompleteAsync returned a bool, so the page that will show
its result could not tell "the browser was never opened" from "the
provider did not confirm", and the operator is told something different
for each.

It now returns ProviderSignOutResult:

- Confirmed: the browser came back, on the registered address, with this
  sign-out's state.
- NotOpened: the default browser could not be started, or the callback
  inbox could not be opened and so no browser was started. The operator
  saw no page.
- NotConfirmed: the page was opened and the provider did not come back
  within two minutes, came back with the state on another address, or the
  inbox failed meanwhile.
- Cancelled: the caller's token ended it.

A sign-out whose caller had already stopped waiting when it was started
now opens nothing, neither an inbox nor a page, and ends as Cancelled.
Running out of time is logged in fixed words instead of the text of a
cancellation, and a page that was not opened is logged as that.

Tests: every way of not being confirmed with its result, whether a page
was opened, and its log entry, including an inbox whose own wait was
ended by something else and the two minute limit to the second; a caller
that had already stopped; the existing tests moved from bool to the
result.

From the first author's notes on afe7e08.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(companion): a sign-out whose provider page could not be opened no longer says the page was opened 144681b981

When the default browser could not be started for the identity
provider's sign-out, the connection dialog still told the operator that
the provider's sign-out "was opened in your browser but did not confirm;
close that page and sign out there if it is still open". No page had
been opened.

The dialog now says that the provider's sign-out page could not be
opened, that the operator may therefore still be signed in to the
provider and the next sign-in may not ask for credentials, and what to
check, in the fixed words every report of a browser that could not be
opened ends with (BrowserLaunch.Remedy). Nothing of the failure's own
text is shown, as before, and the log entry is unchanged.

The change to SignInFlow.SignOut.cs is exactly this: the flow's private
wait returns ProviderSignOutResult instead of a bool, with NotOpened for
a browser that did not start, and SignInMessages.ProviderSignedOut takes
that result. A callback inbox that cannot be opened is still reported
with the old sentence, although no page is opened then either; that is
left as it was.

Tests: the two tests of a browser that cannot be started expect the new
message, and that no message says a page "was opened in your browser";
the other unconfirmed endings keep their sentence.

From the first author's notes on afe7e08.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): a browser sign-in that ended before a page was opened no longer blames the browser 9d679a20f9

When a browser sign-in ended without a result, CompanionSignIn copied the
protocol handler's trace or, when there was none, logged that "the
browser never launched the sign-in protocol handler" and that "either
the identity provider did not redirect ... or the browser blocked opening
the QQQ Print Agent sign-in link". It said so for attempts that ended
before any browser was started: the server did not list this
application's callback, the callback inbox could not be opened, or the
default browser could not be started. Nothing had been opened that could
have redirected or been blocked.

The exchange now notes what it is at until the provider's page is open.
An attempt that ends before that logs, as information, that it ended
before a browser page was opened and what it had not got past: the server
does not list this application's callback; the session had not prepared a
usable sign-in address; the attempt was not yet waiting for its callback;
the default browser was not started. It does not read the handler's
trace, which could only be another attempt's. Attempts that end after the
page was opened are reported as before.

This is kept to CompanionSignIn. The dialog's flow has the same wording
and keeps it: its text is its own, not shared.

Tests: the failure theory expects the entry that is true for each row
and that the trace is read only after a page was opened; the unlisted
callback and the browser that cannot be started expect their own reason;
Cancel and the limit while the session is still beginning say that no
page was opened.

Review of afe7e08, nit 8.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(presentation): every private field of the new sign-in types says what it holds ddefb134c0

The repository's rule is a /// on every member. The private fields of
BrowserCallbackDispatcher, BrowserCallbackTicket, CompanionSignIn (both
parts) and ProviderSignOut had none. Each now says what it holds and,
where it matters, when: which fields are guarded by the gate, which wait
exists while an operation can still be cancelled, which flags outlive an
attempt and which do not. SignInFlow is left as it is.

No behaviour changes.

Review of afe7e08, nit 7.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(tests): the private fields and constants of the sign-in tests say what they hold 56969892bd

The same rule as for the source types, for the test types this branch
added or touched: the fakes' private fields (FakeInbox, NoContext,
SignInWorld) and the expected log entries the four test classes keep as
constants each get their ///. No test changes.

Review of afe7e08, nit 7.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(presentation): reflow the comment on the browser exchange 5bb2dc382a

The comment on ExchangeWithBrowserAsync was left with a short line when
the sentence about what the exchange notes before a page is open was
added for nit 8. Words unchanged.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(presentation): disposing from a subscriber while an operation is released discards the session 45d0c51982

Dispose() had the hole Discard() had (S1), seen while reading the fixes
through: it decided "an operation is running" by the busy flag and left
the session to that operation. An operation that has ended and is being
released announces how it ended while still busy. A view that disposed
the sign-in from that announcement left the session to an operation that
was past the point of discarding it, so the session was never disposed.

Dispose() now decides as Discard() does, by the operation's own wait.

Test: a caller-cancelled password sign-in whose "ready" announcement
disposes the sign-in; the session is discarded and nothing more is
announced.

Review of afe7e08, S1 (the same defect in Dispose).

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(changelog): reflow the sign-in entries and name the dispatcher's log rule df8ba707b9

Two paragraphs of the unreleased sign-in entries had lines left long by
this round's additions; they are wrapped again, words unchanged. The
dispatcher's entry gains the one sentence it lacked for nit 3: a log that
cannot be written to does not stop the dispatcher.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

docs(audit): U22 says what the log's redaction removes on the new base 0b9102e87b

The branch was replayed onto release/2.0 at 4e01519, which has the
redaction fixes that closed W40. U22 said that the log's redaction
removes only the value of state from the text of a failed browser start
and that id_token_hint passes unchanged. That was true where it was
found, at ebea861, and is not true any more.

Run against the replayed head, in a scratch copy, with the text Windows
gives for a failed start: the sign-out address loses its id_token_hint
and its state; the sign-in address loses its state and keeps its nonce
and code_challenge. The row now says what held when it was found and
what holds now. Nothing else in it changes: BrowserLaunch keeps the
address out of the line whatever the redaction does.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

james.maes merged commit 02f34c3214 into release/2.0

2026-11-11 07:13:10 +00:00

james.maes commented

2026-11-11 08:22:10 +00:00

Correction to this pull request's description, changelog entry and audit row U22 (found by the review of the next documentation change, confirmed against the tags):

The sign-in address leak on a failed browser start is in four releases: 1.1.0, 1.1.1, 1.2.0 and 1.2.1. It is not in "every release from 0.2.0-rc.1 to 1.2.1".
The seven releases from 0.2.0-rc.1 to 1.0.1 start the browser the same way but catch the failure and show a fixed text; they neither log nor show the address. The logging and the "Reason:" text came with commit aaa5051, which is in v1.1.0, v1.1.1, v1.2.0, v1.2.1 and snapshot-2.0.0-rc.1 only.
Everything else stands: the sign-out case is in no release; both cases are in the snapshot and on the unreleased release/1.2.2 branch; 2.0 builds from this change on have neither.

The changelog and U22 are corrected in the documentation change that follows; the snapshot's release notes are corrected.

Correction to this pull request's description, changelog entry and audit row U22 (found by the review of the next documentation change, confirmed against the tags): - The sign-in address leak on a failed browser start is in **four** releases: 1.1.0, 1.1.1, 1.2.0 and 1.2.1. It is not in "every release from 0.2.0-rc.1 to 1.2.1". - The seven releases from 0.2.0-rc.1 to 1.0.1 start the browser the same way but catch the failure and show a fixed text; they neither log nor show the address. The logging and the "Reason:" text came with commit `aaa5051`, which is in `v1.1.0`, `v1.1.1`, `v1.2.0`, `v1.2.1` and `snapshot-2.0.0-rc.1` only. - Everything else stands: the sign-out case is in no release; both cases are in the snapshot and on the unreleased `release/1.2.2` branch; 2.0 builds from this change on have neither. The changelog and U22 are corrected in the documentation change that follows; the snapshot's release notes are corrected.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QRun-IO/qqq-print-agent#53