Resolve #274: mistdemo web command with Hummingbird CRUD routes

[leogdion]

Summary

Phase 2 of #330 — stacked on top of #332. Adds mistdemo web, a long-running interactive demo combining the CloudKit JS auth round trip with a CRUD UI driven by MistKit on the server. The page also scaffolds the MistKit-vs-CloudKit-JS mode toggle; the CloudKit JS side stays disabled until #329 wires it.

What lands

• WebDemoCommand — long-running command (mistdemo web) that owns lifecycle: server task, browser open, signal handling.
• WebDemoServer — Hummingbird router covering /, /api/config, /api/authenticate, plus CRUD routes /api/records/{query,create,update,delete}.
• WebAuthTokenStore — actor holding the captured ckWebAuthToken; CRUD routes return 401 until it is set.
• WebDemoBackend — narrow protocol over the MistKit methods the routes need. CloudKitService conforms via extension; tests inject a MockBackend actor through WebDemoBackendFactory.
• Resources/web-demo-index.html — interactive UI with mode toggle, auth area, and panels for each CRUD op.

Scope notes

• Field values in CRUD request bodies are limited to strings for this MVP demo. The HTML form mirrors that constraint. Wider FieldValue coverage can land alongside richer UI controls.
• Database is hardcoded to private for now; public DB toggle is Add public database interface to MistDemoApp and web #275.
• CloudKit JS handlers in the page are stubbed ("not wired yet — see Integrate CloudKit JS into MistDemo web for MistKit-vs-CloudKit-JS comparison #329"). The toggle UI is in place so Integrate CloudKit JS into MistDemo web for MistKit-vs-CloudKit-JS comparison #329 can plug in without churn.

Test plan

• swift build clean
• swift test — 920 tests pass (13 new: 9 WebDemoServer + 4 WebAuthTokenStore)
• ./Scripts/lint.sh — clean
• Run the demo against a real container locally
• CI green on push

🤖 Generated with Claude Code

---

Perform an AI-assisted review on

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3e53eaed-a257-4f50-84e4-4c6efd6d00e7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 274-web-crud-routes

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 71.86992% with 173 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.10%. Comparing base (77715e0) to head (603ef83).
⚠️ Report is 1 commits behind head on 330-interactive-mistdemo.

Files with missing lines	Patch %	Lines
...ources/MistDemoKit/Commands/AuthTokenCommand.swift	0.00%	43 Missing ⚠️
.../Sources/MistDemoKit/Configuration/WebConfig.swift	0.00%	42 Missing ⚠️
...Demo/Sources/MistDemoKit/Commands/WebCommand.swift	0.00%	39 Missing ⚠️
...stDemo/Sources/MistDemoKit/Server/WebBackend.swift	0.00%	30 Missing ⚠️
...Sources/MistDemoKit/Server/WebBackendFactory.swift	15.38%	11 Missing ⚠️
...Demo/Sources/MistDemoKit/Server/WebIndexHTML.swift	72.72%	3 Missing ⚠️
.../MistDemo/Sources/MistDemoKit/MistDemoRunner.swift	0.00%	2 Missing ⚠️
...urces/MistDemoKit/Commands/TestPublicCommand.swift	0.00%	1 Missing ⚠️
...es/MistDemoKit/Server/LoopbackOnlyMiddleware.swift	75.00%	1 Missing ⚠️
...o/Sources/MistDemoKit/Utilities/AsyncHelpers.swift	66.66%	1 Missing ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##           330-interactive-mistdemo     #333      +/-   ##
============================================================
- Coverage                     70.24%   70.10%   -0.15%     
============================================================
  Files                           531      543      +12     
  Lines                         14536    14965     +429     
============================================================
+ Hits                          10211    10491     +280     
- Misses                         4325     4474     +149     

Flag	Coverage Δ
mistdemo-spm-macos	59.70% <71.86%> (+0.29%)	⬆️
mistdemo-swift-6.2-jammy	?
mistdemo-swift-6.2-noble	?
mistdemo-swift-6.3-jammy	?
mistdemo-swift-6.3-noble	59.73% <71.86%> (+0.31%)	⬆️
spm	51.42% <ø> (-0.23%)	⬇️
swift-6.1-jammy	?
swift-6.1-noble	?
swift-6.2-jammy	?
swift-6.2-noble	?
swift-6.3-jammy	?
swift-6.3-noble	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Review: PR 333 — mistdemo web command with Hummingbird CRUD routes

Overall the shape of this is good: clean protocol abstraction (WebDemoBackend), testable factory injection, actor-based token store, and solid router-level test coverage. One critical lifecycle bug needs fixing before this ships.

Critical bug — server stops after ~1 second

WebDemoCommand.execute() puts both app.runService() and openBrowserIfNeeded() in the same ThrowingTaskGroup and calls group.next() after both are enqueued:

group.addTask { try await app.runService() }
group.addTask { await openBrowserIfNeeded() }
try await group.next()   // returns as soon as EITHER task finishes
group.cancelAll()

openBrowserIfNeeded() sleeps 1 second, opens the browser, then returns Void. At that point group.next() unblocks, group.cancelAll() fires, and the server task is cancelled. The command exits roughly 1 second after the browser opens.

The fix is to fire the browser open outside the group that controls server lifetime:

try await withSignalHandling {
    Task { await openBrowserIfNeeded() }   // detached, no effect on server lifetime
    try await app.runService()
}

try? on Task.sleep swallows cancellation

In openBrowserIfNeeded(): try? await Task.sleep(nanoseconds: 1_000_000_000)

Ignoring the cancellation error means if the parent task is cancelled during the sleep the browser still opens. Use try await and let the error propagate so cancellation is respected promptly.

Dead code in WebDemoServerTests.queryForwards

The body variable is encoded but never used — jsonBody is the literal that is actually sent. Remove body and the _ = body suppression entirely.

WebDemoConfig — double nil-coalescing and redundant alias

1. let configReader = configuration is a no-op alias. Use configuration directly.

2. The container identifier line provides the same default twice. The fallback after string(forKey:default:) is unreachable if the default parameter works as documented. Either drop the fallback or document why the belt-and-suspenders pattern is intentional.

makeRouter() always throws for an unreachable error

makeRouter() throws only because JSONEncoder().encode(CloudKitClientConfig(...)) can theoretically throw. CloudKitClientConfig encodes three primitive-typed fields; in practice this call cannot fail. Consider try! with a comment, or restructure so configData is pre-encoded and throws can be dropped from the signature.

stringFields could use mapValues

static func stringFields(_ raw: [String: String]) -> [String: FieldValue]

Can be written as raw.mapValues(FieldValue.string).

Minor: @available on CloudKitService: WebDemoBackend

Verify the @available(macOS 11.0, iOS 14.0, ...) on the conformance extension matches what CloudKitService itself already requires. If redundant, remove it. If narrower, document why.

Summary

Severity	Issue
Critical	TaskGroup lifecycle: server exits 1s after browser opens
Medium	try? swallows Task.sleep cancellation
Minor	Dead body variable in queryForwards test
Minor	WebDemoConfig redundant alias + double nil-coalescing
Nit	makeRouter() throws for an infallible encode
Nit	stringFields handwritten loop vs. mapValues

The HTML/JS page is well-structured, the mode toggle scaffolding for #329 is clean, and the test suite covers the important paths (pre-auth 401, backend errors as 500, all four CRUD operations). Fix the lifecycle bug and this is ready for another look.

[claude]

Code Review: PR #332 + PR #333 (mistdemo web command)

Reviewed against the final stacked state on branch 274-web-crud-routes. This comment covers both PRs together since #333 is stacked on #332.

---

Code Quality and Conventions

PR #332 — Mostly excellent.

• Extracting isLoopbackAuthority from AuthTokenCommand into a standalone LoopbackAuthority enum is the right call. The new implementation (host(in:) + bracketedHost(in:)) is cleaner than the original.
• Moving HTML from multi-file Swift raw-string assembly into a SwiftPM resource bundle eliminates the swiftlint:disable line_length indentation_width suppressions and lets the HTML be edited directly.
• The switch from import to internal import across all Hummingbird-gated files is correct for library hygiene even in a demo target.

PR #333 — Good overall, a few rough spots.

• WebDemoRequests as a caseless enum namespace for request/response types is idiomatic Swift.
• WebDemoBackendFactory as a struct wrapping a closure follows the project's existing dependency-injection patterns.
• #if canImport(Hummingbird) guards are applied consistently.

Minor: WebDemoConfig and AuthTokenConfig are now structurally identical (same six fields, same parsing logic, both Sendable + ConfigurationParseable). A shared parsing helper or merge would reduce maintenance surface.

Minor: In queryForwards test, body is constructed as a [String: String] dict (wrong type for limit) and then discarded with _ = body before a raw jsonBody string is used instead. The dead body allocation should be removed.

---

Architecture

WebDemoServer replacing AuthTokenServer is a good consolidation. Using terminatesAfterAuth: Bool to switch between 205 and 204 on the authenticate endpoint is a clean boolean seam.

Concern: auth-token and web now share a single HTML page. PR #332 introduced auth-token-index.html for the one-shot token-capture flow. PR #333 removes it — AuthTokenCommand now uses WebDemoServer which serves web-demo-index.html. The JS distinguishes 205 from 204 and calls renderTokenDisplay, so it functions correctly, but users running mistdemo auth-token now see the full CRUD grid and mode-toggle even though those are irrelevant to their task. Minor UX regression worth noting.

WebDemoBackend hardcodes database: .private for all four operations. This is appropriate for the stated scope (web-auth token required for private DB), but a comment explaining this constraint would save the next reader from questioning it.

WebDemoBackendFactory creates a new CloudKitService on each authenticate call (backendFactory.make(token)). CloudKitService is cheap to construct since it doesn't hold persistent connections at that layer, but a comment would help — it's surprising to a reader who expects a service to be long-lived.

---

Concurrency Safety

WebAuthTokenStore is correctly implemented as an actor. The nonisolated let tokenUpdates pattern (AsyncStream with continuation) is safe. deinit calls continuation.finish(), which is correct and prevents consumer tasks from hanging after the store is released.

Bug: WebDemoCommand.execute() — server is killed immediately when --no-browser is passed. The task group structure is:

group.addTask { try await app.runService() }  // task A
group.addTask { await openBrowserIfNeeded() }  // task B: returns immediately when noBrowser=true
try await group.next()
group.cancelAll()

When config.noBrowser == true, task B completes essentially immediately. group.next() returns B's result and calls cancelAll(), cancelling the server before it has served a single request. Suggested fix:

group.addTask { try await app.runService() }
if !config.noBrowser {
    group.addTask { await openBrowserIfNeeded() }
}
try await group.next()
group.cancelAll()

AuthTokenCommand.execute() latent hang (low severity). If app.runService() terminates before the browser sends the authenticate request, the while loop exhausts the nil results from the two short-lived tasks and then waits indefinitely on the tokenUpdates iterator (the stream won't finish until the store deinits). The withTimeoutAndSignals(seconds: 300) wrapper bounds this to 5 minutes, which is an acceptable tradeoff, but the error surfaced will be a generic AsyncTimeoutError. A comment explaining this would clarify the intentional bound.

The old fire-and-forget Task { ... } in AuthTokenServer.addAuthEndpoint is gone. PR #333 replaces it with await tokenStore.update(...) directly in the handler. This is strictly better.

withSignalHandling double-nesting in WebDemoCommand. withSignalHandling { withThrowingTaskGroup } is safe, but withSignalHandling itself wraps a withThrowingTaskGroup, so UnixSignalsSequence may register signal handlers twice. Whether that is harmless depends on the swift-service-lifecycle implementation. Worth a comment, or the inner withThrowingTaskGroup could be eliminated by using withTimeoutAndSignals(seconds: nil).

---

Security

Loopback guard is applied consistently on all CRUD and auth routes. LoopbackAuthority handles localhost, 127.0.0.1, [::1], port variants, and rejects bypass attempts. Tests confirm the allow/deny list. This is good.

GET / and GET /index.html do not enforce loopback. This is intentional (the HTML page itself is not sensitive), but a comment would prevent a developer from copying this unguarded pattern for a route that is sensitive.

XSS risk in web-demo-index.html. renderTokenDisplay uses a template literal to inject ${token} into innerHTML:

card.innerHTML = `...<pre ...>${token}</pre>...`;

A CloudKit web-auth token is an opaque string that will never contain HTML in practice, but interpolating any untrusted data into innerHTML is a pattern violation. Use textContent on a constructed element instead. Low severity for a localhost-only demo, but sets a bad example.

GET /api/config returns the raw apiToken to any loopback caller. This is expected for the CloudKit JS auth flow (the API token is public-facing by design), but a comment acknowledging this is appropriate.

No CSRF protection. Since the server binds exclusively to loopback and is a single-user demo tool, traditional CSRF is not a realistic threat, but a note in the server's doc comment would clarify this is an intentional decision.

---

Test Coverage

Good coverage of the happy paths. The 19 tests across WebDemoServerTests, WebAuthTokenStoreTests, and LoopbackAuthorityTests cover index routes, config endpoint, authenticate capturing the token, terminatesAfterAuth 204/205 toggling, 401 pre-auth for CRUD routes, CRUD forwarding to backend, 500 on backend error, and LoopbackAuthority edge cases.

Missing:

• Test for loopback rejection on CRUD/auth routes. The security invariant is that non-loopback callers are rejected, but the test suite does not verify this. HummingbirdTesting in router mode bypasses real HTTP; you'd need to construct a request with a spoofed host header explicitly.
• Test for GET /index.html (only / is exercised; neither PR's tests cover the /index.html alias).
• Test for empty recordType in CRUD requests. The server passes it through to the backend; a test would clarify intended behavior.
• Test for WebDemoCommand registration — that "web" resolves to WebDemoCommand in MistDemoRunner. This is a one-liner that already exists for auth-token.

MockBackend.stubRecord uses try! with a hand-rolled JSON string. If RecordInfo's Codable representation changes, this crashes instead of failing cleanly. Consider building through a public initializer if one exists.

---

Summary

Severity	Issue
Medium bug	WebDemoCommand server is cancelled immediately when --no-browser is passed
Low bug	Dead body variable in queryForwards test
Low security	${token} interpolated into innerHTML in web-demo-index.html
Low	Loopback rejection not tested
Low	WebDemoConfig / AuthTokenConfig structural duplication
Low	Missing MockBackend.stubRecord error surfacing
Nit	Comments explaining private-DB hardcoding, WebDemoBackendFactory lifetime, and CSRF non-requirement

The --no-browser concurrency bug is the one to fix before this merges. Everything else is low severity or advisory. Overall this is well-structured work — the WebDemoBackend protocol abstraction is the right shape, actor usage is correct, and the test coverage is solid for the core paths.

🤖 Generated with Claude Code

[claude]

Code Review: PR #333 — mistdemo web command with Hummingbird CRUD routes

Overview: This PR adds mistdemo web, a long-running interactive demo that serves a combined CloudKit auth + CRUD UI. It refactors the existing AuthTokenServer/AuthTokenCommand flow onto a shared WebServer foundation, replaces AsyncAlgorithms.AsyncChannel with an actor-backed WebAuthTokenStore, and adds 13 new tests.

---

Code Quality and Best Practices

Strengths:

• WebBackend protocol + MockBackend actor is a clean dependency-injection pattern that makes the route handlers testable without a live CloudKit container.
• WebBackendFactory encapsulates the credential-wiring that would otherwise live inline in route closures.
• WebAuthTokenStore actor properly owns both state and the AsyncStream continuation — deinit correctly calls updatesContinuation.finish().
• LoopbackOnlyMiddleware at the api router group level is a good security chokepoint (better than per-route checks that can be forgotten).
• terminatesAfterAuth cleanly expresses the behavioral difference between auth-token (one-shot) and web (long-running) without a subclass hierarchy.
• withTimeoutAndSignals(seconds: Double?) making timeout optional is a pragmatic and backwards-compatible API extension.
• WebIndexHTML.loadContent() uses preconditionFailure instead of try! — fails fast with a meaningful message rather than a generic crash.

Issues / Suggestions:

1. WebCommand.execute() does not cancel the browser task if the server errors. The withThrowingTaskGroup loop calls group.waitForAll(), which is correct, but if app.runService() throws immediately (e.g. port already in use), the browser-open task is left running until it resolves on its own (~1 s sleep). This is benign here but inconsistent with AuthTokenCommand.captureToken which uses group.cancelAll() on success. Consider group.cancelAll() after catching the server error, or switch to the structured-cancel approach already used in captureToken.

2. WebBackend is hardcoded to .private database. Noted in the PR description as "database hardcoded to private for now" and tracked in Add public database interface to MistDemoApp and web #275 — just flag for the reviewer to be aware this is a known limitation, not a bug.

3. runOperation swallows all errors as 500. This is intentional for the demo, but CloudKit authorization errors (e.g. expired token) would be more useful as 401/403. Worth a follow-up issue or a comment noting the intentional coarseness.

4. WebRequests.stringFields could be a mapValues. Minor style nit:

   // current
   var result: [String: FieldValue] = [:]
   for (name, value) in raw {
     result[name] = .string(value)
   }
   return result
   // simpler
   raw.mapValues { .string($0) }

5. WebConfig and AuthTokenConfig share identical environment-parsing logic. Both parse "environment" from config with the same guard let environment = MistKit.Environment(caseInsensitive:) pattern. A shared parseEnvironment(from:) helper (or a ConfigReader extension) would remove the duplication and keep the error message consistent.

6. MockBackend.stubRecord uses try! with a comment acknowledging it. The // swiftlint:disable:next force_try workaround is acceptable for test helpers where the JSON is static, but consider a failable static factory (static func? stubRecord(...)) to keep test infrastructure error-free.

---

Potential Bugs / Issues

• tokenUpdates stream is nonisolated but the continuation is actor-isolated. The AsyncStream is correctly initialized in init and the continuation is only called from within update(_:) (actor-isolated), so this is safe. The nonisolated let tokenUpdates is needed for cross-isolation consumption. No bug, but worth a brief inline comment explaining why nonisolated is correct here.
• pollWebAuthToken in the HTML uses setInterval but only rejects after the deadline — it doesn't call reject inside the interval body on the deadline. Looking at the code: the condition Date.now() - pollStart >= pollDeadlineMs clears the interval but does not call reject(...). The reject is only called from... wait, looking again at lines 1552-1555, the reject is never called inside the interval; the interval stops and the Promise just hangs. This is a browser-side bug — pollWebAuthToken() will never reject if the token doesn't appear, leading to handleAuthentication hanging indefinitely. Should be:

  if (Date.now() - pollStart >= pollDeadlineMs) {
    clearInterval(handle);
    reject(new Error('Timeout waiting for web auth token'));  // ← missing call
  }

---

Security Considerations

• API token exposed in /api/config. The config endpoint returns the raw apiToken to the browser so CloudKit JS can be initialized. This is an existing pattern from auth-token-index.html and is consistent with how CloudKit JS is intended to be used (API tokens are public, not secret). The LoopbackOnlyMiddleware ensures only loopback callers can reach this endpoint. Acceptable for a localhost demo.
• sessionToken written directly to WebAuthTokenStore without validation. The /api/authenticate route accepts any string as sessionToken. Since the server is loopback-only and the token is only used to construct a CloudKitService on subsequent CRUD calls (which will fail if the token is invalid), this is acceptable for a demo context.

---

Test Coverage

Good overall. 13 new tests covering:

• WebAuthTokenStore lifecycle (empty init, pre-seeded, update, clear)
• WebServer index, config (including new environment field), authenticate (204 + 205 variants), tokenUpdates stream
• All four CRUD routes forwarding correctly via MockBackend
• Pre-auth 401 for all four CRUD paths (parameterized)
• Backend error → 500 with JSON body

Gaps worth tracking (not blocking):

• No test for LoopbackOnlyMiddleware rejecting a non-loopback Host header on /api/*. The existing WebServerTests use HummingbirdTesting's .router mode which bypasses real socket binding — a middleware test would need to synthesize the authority header.
• withTimeoutAndSignals(seconds: nil, ...) path isn't tested (no unit test for the no-timeout branch).
• WebConfig parsing happy-path and error-path tests are missing (only AuthTokenConfig has them; WebConfig parsing follows the same code path).

---

Summary

Solid PR. Architecture is clean, the refactor from AsyncChannel to WebAuthTokenStore is an improvement, and the test coverage is meaningful. The browser-side pollWebAuthToken reject-on-timeout bug is the one functional issue worth fixing before shipping. The other items are minor style/future-proofing notes.

🤖 Reviewed with Claude Code

[claude]

Code Review — PR #333: mistdemo web with Hummingbird CRUD routes

Overview

Phase 2 of the interactive demo stack. Promotes AuthTokenServer to the more general WebServer, adds four CRUD routes backed by the new WebBackend / WebBackendFactory abstraction, and ships a polished HTML page for browser-side testing. The actor-based WebAuthTokenStore cleanly replaces the previous AsyncChannel rendezvous pattern.

---

Strengths

• Clean separation of concerns. WebAuthTokenStore (state), WebBackend (capability protocol), WebBackendFactory (DI), and WebServer (routing) are each responsible for one thing.
• 204 vs 205 signal for lifecycle. Using HTTP status to distinguish "stay up" from "about to shut down" is a neat protocol-level trick and it's clearly documented.
• LoopbackOnlyMiddleware correctly scoped. Applying it to the /api group (not the root) keeps the index page publicly reachable while locking down all state-mutating endpoints. The refactoring from per-route isLoopback checks to a single middleware is a clear win.
• Test coverage is good. Parameterised 401-rejection test, per-route forwarding assertions, MockBackend error injection, and the tokenUpdates async-stream test all cover meaningful behaviour without over-testing implementation details.

---

Issues

Security

container?._auth?._ckSession accesses undocumented CloudKit.js internals (index.html, pollWebAuthToken)

const token = container?._auth?._ckSession;

This relies on a private, underscore-prefixed property. There's no public CloudKit.js API for extracting the session token directly, but this approach:

• Will silently break on any CloudKit.js internal restructure.
• Returns undefined in strict mode if the property moves, causing the 10-second timeout every auth attempt.

Recommended: Add a prominent // NOTE: accesses undocumented CK.js internals; may break on CDN update comment, and consider filing a companion issue to track finding a stable hook (e.g. the userIdentity passed to whenUserSignsIn may carry enough info to derive the session key server-side without polling).

XSS risk in renderTokenDisplay (minor / low probability)

card.innerHTML = `...${token}...`;

token is sourced from CloudKit.js internals so in practice it's an opaque string, but if it ever contained <script> content this would be XSS. Prefer document.createTextNode(token) / textContent for the token display element.

---

Code Quality

runOperation is declared throws but eats all errors internally

internal static func runOperation(
  _ operation: @Sendable () async throws -> Data
) async throws -> Response {

The only way the outer throws is reached is if JSONEncoder().encode(WebResponse.Error(...)) throws, which is essentially impossible for this simple struct. The throws annotation implies callers should handle errors, but route closures already use try await. Either drop throws or add a comment explaining the edge case.

WebBackend extension on CloudKitService hardcodes database: .private

Every method in WebBackend.swift passes database: .private. The scope note in the PR description acknowledges this, but there's no TODO comment in the code linking it to #275. A one-liner like // database hardcoded to .private — see #275 for public-DB toggle prevents this from quietly staying forever.

AuthTokenCommand creates a live backend factory it never uses

backendFactory: .live(
  apiToken: config.apiToken,
  containerIdentifier: config.containerIdentifier,
  environment: config.environment
)

auth-token captures the token and exits; CRUD routes are never reached. Building the factory is harmless but misleading. Either pass a .stub / no-op factory, or add a comment explaining that the token-capture and CRUD paths share WebServer intentionally.

JSONEncoder() instantiated per-request in each CRUD route handler and in runOperation. For a demo this is fine, but all four methods use the same default encoder — a shared private static let encoder = JSONEncoder() in WebServer would be cleaner.

---

Test Gaps

LoopbackOnlyMiddleware has no test. The middleware is the security boundary for all /api routes. A test that sends a request with a non-loopback :authority and asserts .forbidden would be cheap and high-value.

tokenUpdates stream finishes on deinit but there's no test for it. WebAuthTokenStoreTests covers the actor's storage methods well; adding one test that confirms tokenUpdates terminates when the store is deallocated would prevent a silent stream-leak regression.

MockBackend.stubRecord uses force_try on JSONDecoder (already silenced with swiftlint:disable:next). Fine for test code but the RecordInfo init is straightforward enough that a memberwise init (if exposed) would be cleaner. Not a blocker.

---

Minor Nits

• WebConfig parsing for noBrowser omits the ?? false fallback that all other optional configReader calls use. Functionally identical since the default is provided to configReader.bool, but inconsistent.
• WebServer.makeRouter() keeps the internal import Logging from AuthTokenServer but no logging calls appear in the new routes. If nothing in the unchanged parts of the file uses it, it should be removed.
• The JS pollWebAuthToken resolves on the first truthy container?._auth?._ckSession every 250 ms up to 10 s. Document the interval/deadline as named constants rather than magic numbers.

---

Summary

This is solid, well-structured work. The protocol/factory DI pattern is the right approach for keeping the routing layer testable, and the test suite covers the main paths cleanly. The three items worth addressing before merge:

1. Document (and ideally track via issue) the _auth._ckSession private-API dependency — silent breakage risk.
2. Add a loopback-rejection test for LoopbackOnlyMiddleware — it's the security gate for all API state.
3. Swap innerHTML token interpolation for textContent — low risk today, zero risk after.

The rest are polish/nits that can land incrementally.

🤖 Generated with Claude Code