feat: single-machine TapScribe — attached taps, the login link, one tray with two roles, and the macOS Bundle - #449
Open
Vortiago wants to merge 36 commits into
Open
feat: single-machine TapScribe — attached taps, the login link, one tray with two roles, and the macOS Bundle#449Vortiago wants to merge 36 commits into
Vortiago wants to merge 36 commits into
Conversation
…in link Outcome of a grill on making TapScribe easy to run single-user / single-machine. Nothing is implemented yet; the three ADRs are `proposed` and the glossary entries that describe them say so. CONTEXT.md - Current session · attached tap — the `● live` badge is the Recorder's current session; a tap attaches by omitting `?session=`. Attached or in a bracketed meeting, never both. - Login link + a second credential form (session cookie) on the Basic scheme. - Bundle absorbs the retired Launcher; new Host role entry. ADR-0022 — one tray per OS carries both roles; a Bundle ships the Tray Bridge; the role is derived from install layout, not a build flag. Two artifacts per OS; the third host-only package is rejected with the reason. ADR-0023 — the Basic scheme accepts a session cookie minted by a single-use login link. Records why a co-located Recorder does NOT default to `--no-auth`: `allow_origins=["*"]` plus a middleware that returns before any scheme means any website the operator visits can read and write the Recorder. ADR-0024 — the macOS Bundle keeps its interpreter outside the signed `.app` (runtime pip would invalidate the signature) and copies it to the data root on first launch. Signing stays ad-hoc. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
…t, lifecycles A Fable review of 4f03c10 verified the CORS/no-auth claim (middleware order makes it stronger than written: auth sits OUTSIDE CORS, so a hostile cross-origin request 401s from a middleware CORS never decorates) and found two factual errors and a set of unaddressed lifecycles. Factually wrong, now fixed: - ADR-0023 attributed cookie safety to `allow_credentials` stopping the browser SENDING the cookie. It does not: `SameSite=Strict` stops the sending, `allow_credentials` stops the READING. Misattributed, someone relaxing SameSite while trusting the sentence ships the CSRF it claims to prevent. - ADR-0022 said `BundleLayout` resolving an interpreter and one wheel IS the host-role test. `Resolve` probes nothing and `ResolveWheel` is designed to THROW on zero-or-several wheels — a packaging-bug detector, not a boolean. Folded into a role probe, a wiped `wheel/` would silently demote a Bundle to a bridge-only tray. The test is payload-folder presence; ResolveWheel stays loud inside the role. - ADR-0023's "any website the operator visits" overstated: Chromium's Private Network Access interposes public→loopback. Narrows the attacker, does not close the hole (another localhost port crosses no boundary). Holes now recorded rather than left to discovery: - Login link: a 60 s TTL cannot serve the URL start.sh prints at boot, so the TTL is per mint site. Spent/expired renders a page, never a bare 401 (which would pop the Basic dialog the ADR exists to remove); a grace window survives a scanner or double-click; the cookie name carries the port; `WWW-Authenticate` is omitted when a cookie was presented. - macOS runtime copy: staged-then-renamed so a partial copy can't be mistaken for a good one, version-stamped so an upgrade re-copies instead of serving the old wheel, and repair-by-reinstall spelled out. - Supervision: JobObject goes BEHIND the per-OS seam, not in the Core the Linux CI leg tests. A POSIX process group does not equal KILL_ON_JOB_CLOSE — a crashed macOS tray orphans the Recorder on port 8001 — so the seam pairs it with a parent-death watch. - "Two artifacts per OS" is two INSTALLABLE ones; ADR-0012/0020's macOS .app zip stays, and ADR-0015's "two tray icons stay separate" consequence is reversed and must be annotated on acceptance. ADR-0025 (new) — the attached tap, which had no ADR despite a real rejected alternative. Records follow-current over pin-at-connect, and the two consequences that were nowhere: a rotation splits an attached conversation, and processing one means rotating first. CONTEXT.md: the Bundle entry described ADR-0022's end state in present tense while the Launcher still ships. It now describes today and points at the proposed replacement. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
start.sh / start.ps1 keep printing the generated password and nothing else. That leaves the tray as the only mint site, so the two-value TTL the boot banner forced collapses back to one. The 60 s stays, with the reason stated properly: single-use bounds a token that gets USED, and the TTL covers the one it does not — a token never spent (no default browser, a failed launch, a cancelled click) would otherwise be a live credential with no expiry. It also travels through the address bar and the OS's open-URL handoff, both of which can log it, where .auth-password sits behind file permissions. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
…ight seam The two groundwork pieces an attached tap needs before the runtime can open one (ADR-0025), both no-ops for a bracketed meeting: - `ResolveResult.ToTapOptions` takes `string?`. `BuildTapUri` already omits `session=` when null, so this is the whole wire-level change — an attached tap is the Bridge declining to name a session, not a new frame or a new route. - `BridgeDependencies.CheckConnection`, production-wired to the new `ConnectionTester.CheckSettingsAsync` in both shells. Start gets a pre-flight free (its mint round-trips the Recorder) and Connect has no mint, so without a seam here an unreachable Recorder or a refused token would stay silent until the first person spoke. Its own member, like the mint, so a test fakes both failures without a socket: `RuntimeDoubles` gains HoldPreflight / PreflightResult / PreflightError to match HoldMint / MintError. `CheckSettingsAsync` is named in Core rather than written as a lambda at each wiring site: the two would be identical and would drift the first time one of them learned something. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
`OpenMeetingAsync` becomes `OpenTapsAsync(settings, failureTitle, preflight)`. The two modes differ in exactly one step — a meeting mints the detached session its taps route into, an attached tap has nothing to mint and probes instead — so everything either side of it (the selection resolve and its hard stop, the per-device open, the one `CaptureSet` ownership rule) is shared rather than copied. A second copy is where the two modes would quietly stop agreeing about what a device is. The pre-flight delegate THROWS on failure, which is the mint's own documented contract, so its `string?` answer is unambiguously "no session named" rather than "something went wrong". `StartAsync` refuses a null one loudly: a meeting whose taps carried no session would silently become an attached one, recording into whatever the Recorder had open instead of into its own. `RuntimeBudgets.MintTimeout` → `PreflightTimeout`: it now bounds both round-trips. No test file changed and all 461 pass — which is the claim this commit is making. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
…rent session The tray gains Connect / Disconnect beside Start / End meeting (ADR-0025). An attached tap streams into whatever session the Recorder has open — the one the dashboard badges live — which is what a microphone in a meeting room wants, with nobody at the keyboard. No Recorder change: the mode is the Bridge omitting a query parameter. `_attached` is a second nullable field beside `_meeting`, and its record carries NO session id. An attached tap genuinely has none — a rotation moves the current session out from under it, so any id captured at Connect is stale by the next utterance — and leaving the field out makes writing that bug impossible rather than merely documented. It also makes "attached or bracketed, never both" a thing a test can check. Start from Attached is a TAKEOVER: drain, then mint. The order is the point, and the test sabotage-checks it — draining first flushes the last Utterance into the session it was recorded in, where minting first would leave a truncated WAV for the Recorder to strip and transcribe. Disconnect does NOT reuse the `triggerPipeline: false` path, contrary to ADR-0025's wording (amended in this branch): that path is session-keyed, and `MeetingController` throws on a null session id. It drains, says so, and returns to idle — no trigger, no poll, no history entry, no resume state, every one of which is keyed on a session an attached tap does not have. `ITrayView.SetMenuState(bool, bool)` becomes `SetCommands(TrayCommands)`: four bools at a call site read as noise where two already needed named helpers, and the legal combinations are four, not sixteen. `TrayCommands.Attached` keeps Start enabled — that IS the takeover. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
… session The real-Recorder E2E for ADR-0025, beside the existing meeting one and on the same Linux CI leg. Every other test in that file mints a detached session and then looks in it; here the whole claim is that the audio ends up somewhere this bridge never named, so the session id is read BACK from the Recorder's own `/api/state` — the same `current_session` the dashboard badges live from — and the WAVs are found under it, one per speaker. Sabotage-checked: making the pre-flight answer a session id instead of null fails it on the missing folder. Also asserts what did NOT happen — the real ControlClient is wired and was never asked to mint, and Disconnect left Past meetings empty, because an attached tap has no session id to key a pipeline, a history entry or a resume on. The Windows shell's command test covers Attached: Start stays live (the takeover) while Connect greys out — a shell that greyed Start out there would leave no way to turn a room mic into a meeting without disconnecting first. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
…m a login link ADR-0023's Recorder half. `POST /api/login-link` mints a single-use token; `GET /login?k=…` spends it, sets an HttpOnly SameSite=Strict session cookie and 303s to `/` so the token leaves the address bar. A spent or expired link renders a small HTML page and NEVER a bare 401 — a 401 on a navigation is exactly what pops the browser's native Basic dialog, which is the prompt this removes. BASIC gains a second credential FORM, not a fourth scheme: ADR-0008's three-way dispatch is untouched, and no route is gated differently by which form a caller used. A 401 omits `WWW-Authenticate` when a cookie WAS presented, or a Recorder restart would turn the dashboard's 500 ms poll into that same dialog. `login_links` is stdlib-only and HTTP-free so the state machine is tested directly: single use, a ~10 s grace window (a link scanner or a double-click otherwise burns the token before the operator's real navigation lands), a 60 s expiry for a token that is never spent at all, and a monotonic clock so an NTP step cannot retire a live link. It hangs off `app.state`, so tests never share sessions and "a restart logs you out" is a property rather than a promise. State-changing requests on the BASIC branch get an Origin check. SameSite=Strict stops a cross-SITE page attaching the cookie, but site scoping ignores PORTS, so a hostile page on another localhost port is same-site to this one. Scoped to that branch: the SpatialChat extension's cross-origin POSTs are all under /api/tap/, where `allow_origins=["*"]` stays load-bearing, and its /health probe is PUBLIC-exempt. `config.PORT` and `config.TLS_ENABLED` are stamped at boot beside AUTH_ENABLED: the cookie's NAME carries the port (cookies are host-scoped but not port-scoped, so 8001 and 8002 would clobber each other) and `Secure` had no importable source at all — `--tls` resolved to a local inside `main()`. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
…te live Two layers over ADR-0023's Recorder half. `tests/test_login_routes.py` drives the two routes behind the real middleware: minting needs the password, the link is a PATH (the tray knows its own host; a Recorder behind a proxy would guess wrong), the cookie is HttpOnly + SameSite=Strict with no Max-Age, `Secure` follows `--tls`, the name carries the port — and a spent link answers 200 HTML, never a bare 401. The browser test is the one that proves the point, and it needs a fixture that did not exist: every dashboard-UI test runs `AUTH_ENABLED = False`, against which this would pass whether or not the feature worked. `_serve_recorder(auth=…)` is now the shared body and `running_recorder_auth_on` its second caller. The context carries no `http_credentials`, for the same reason. It asserts what the browser is never asked for: no 401 reaches the page at all (a 401 on a navigation IS the native dialog), the token is gone from the address bar after the 303, and the poll's own endpoint still answers 200. Reopening the same link immediately lands on the dashboard again rather than on a dead-link page — the grace window, end to end, which is what makes the tray's launch survive a link scanner or a double-click spending the token first. Sabotage-checked: disabling the middleware's cookie branch fails it. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /// sentence the operator reads changes. Carried here rather than swapped at the render, | ||
| /// because the tally is what the status line is built from and a second place to decide it | ||
| /// is a second place to get it wrong.</param> | ||
| public DeviceTally(int total, bool attached = false) |
`packaging/windows/src/` is already cross-platform by design — `net10.0`, no P/Invoke, tested on the ubuntu CI leg — and its directory stops being true the moment macOS references it (ADR-0024). The .NET projects, their solution, the SDK pin and the build-output gitignore move to `packaging/bundle/`; `TapScribe.iss` stays under `packaging/windows/`, because Inno Setup IS Windows packaging. Its `..\..\staging\` sources are relative to its own directory and are unchanged. The two `global.json` pins turned out to be identical already, so there was nothing to converge. A separate Bundle solution is kept rather than folding these into the tray's: `TapScribe.TrayBridge.slnx` holds a `net10.0-macos` project, so no CI job builds it at all, and folding would buy no coverage while forcing `test_tray_bridge_ci_coverage.py`'s path regex open. The Bundle solution IS buildable end to end on Windows, which is what `bundle-build` already does. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
…ssReaper `RecorderSupervisor` and `RotatingLogWriter` move into `TapScribe.Bundle.Core` and `JobObject` into a new `TapScribe.Bundle.Windows`, behind an `IProcessReaper` seam — the same Core/Windows split `TapScribe.Bridge.*` already keeps for audio capture, and for the same reason: the lifecycle is identical on both platforms and only the reaping is platform-shaped (ADR-0022). The seam is where the two platforms stop being equivalent, which is worth naming in an interface rather than discovering on a Mac: KILL_ON_JOB_CLOSE reaps on process DEATH, crash included, and a POSIX process group only reaps on a clean exit — so the macOS side will have to reconstruct that out of a group plus a parent-death watch. `Adopt` answers false rather than throwing, and a null reaper stays a supported degraded mode: a Recorder with no backstop beats a Bundle that will not start. `Bundle.Windows` targets plain `net10.0` — DllImport into kernel32 needs no Windows TFM — so it COMPILES on the Linux leg while its tests, all `[RequiresWindows]`, say out loud which capability went unexercised. They assert the two limit flags' effect and that the tray is enrolled, never the kill itself: proving KILL_ON_JOB_CLOSE means letting a job close, which would take the test run with it. The Launcher still works — it gains the reference and the using — and goes in the commit that hands its role to the tray. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
…h probe Two decisions from ADR-0022, both now testable on the Linux CI leg. `BundleLayout.HostPayloadPresent` is the host-role test: `python/` OR `wheel/` present. Not "does Resolve succeed" (pure path construction, probes nothing) and not "does ResolveWheel return" (designed to THROW on a packaging bug the operator must see) — folded into a boolean role probe, a Bundle whose wheel/ was wiped would silently degrade to a bridge-only tray and the Recorder would just vanish from the menu. EITHER folder claims the role, because an AND is that same silent demotion and a python/-only probe is it mirrored; ResolveWheel's errors then stay loud INSIDE the role, which is where they belong. A test pins that pairing. `RecorderSupervisor` gains ownership-at-spawn and the unmanaged classification. When the Recorder this tray started exits immediately, `/health` answering means someone else holds port 8001 — shown as running-but-unmanaged, and Quit will not touch it — and nothing answering means a crash-loop, reported as failed rather than adopted. Deliberately NOT read out of the child's stdout: matching an "address already in use" string would mean owning uvicorn's wording forever. Getting that under test needed two seams. `IChildProcess` is what the supervisor uses of a process and nothing more; `ChildProcess` is the forwarding real one, and `Start()` now returns its task so a test awaits the boot instead of racing the thread pool. Nine tests cover the decisions an operator meets when something is wrong — which until now were reachable only by installing a Bundle on Windows and breaking it by hand. `packaging/bundle/Directory.Build.targets` gives the Windows-targeted project the same compile-on-Linux gate the tray tree has (MSBuild walks up from the project, so nothing here inherited it). It caught a missing `using` in `LauncherIcons` on its first run. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
| { | ||
| using var world = new Fake(); | ||
| world.Boot(); | ||
| var recorder = (FakeChild)world.Recorder!; |
| /// a <c>GET /health</c>. Consulted only when the Recorder this tray started exits | ||
| /// immediately, which is the one moment its answer changes what the operator is | ||
| /// told.</param> | ||
| public RecorderSupervisor( |
| public bool Adopt(IChildProcess child) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(child); | ||
| return AssignProcessToJobObject(_handle, child.NativeHandle); |
`HostController` is the `RecorderSupervisor`'s presentation half, the way `BridgeRuntime` is the capture lifecycle's: the supervisor decides what the Recorder is doing, this decides what the menu says and which commands are live. `IHostView` lives in **Bundle**.Core rather than as a member on the Bridge's `ITrayView` — which is where the plan put it, and which was wrong. A Bundle is not a Bridge; shipping one inside the other is composition, and the composition happens in the SHELL, the one place both roles meet. On `ITrayView` it would make the Bridge's core carry a view model for a Recorder it knows nothing about, or force a reference from the Bridge to the Bundle. The rules the tests hold it to are ADR-0022's: nothing is offered while the Recorder is coming up (a second Start would spawn a second unbounded pip install), an unmanaged Recorder is shown and cannot be stopped while Start stays live, and every render goes through the shell's marshaller — state arrives on the supervisor's background thread and both shells' UI is thread-affine. `IRecorderHost` is what made those tests possible without spawning anything. The first draft reached into a private method by reflection, which was the design telling me it had no seam; `Report` is now public because it IS the seam between the two halves. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
…-in dashboard `TrayHost` is the shell half of the host role (ADR-0022): built exactly when `BundleLayout.HostPayloadPresent` says a payload sits beside the tray on disk, and absent — not disabled — otherwise, which is why a bridge-only install's menu is exactly what it was before the role existed. A type of its own rather than more members on `TrayContext`, because the two roles are separate lifecycles that happen to share a menu. Menu, from the Launcher verbatim: Start Recorder / Stop Recorder (separate from Quit — stopping the server is not quitting the tray), Open dashboard, Copy password, Show log. The Recorder's state goes in that section's header line and NOT on the tray icon, which stays the Bridge's tap state: the icon is what an operator watches during a call. Open dashboard mints a login link (ADR-0023) and hands the browser that, so the Basic dialog never appears. Always the LOCAL Recorder, never the host in bridge settings — a tray may supervise one Recorder and tap into another, and a login link for the wrong one is a password sent where it does not belong. A failed mint falls back to the plain URL, which is exactly the prompt operators met before. The two behaviours that would silently regress are carried across with their reasoning: ShellOpen goes via `explorer.exe` (a direct ShellExecuteEx makes the operator's BROWSER a member of the KILL_ON_JOB_CLOSE job, so quitting the tray killed it), and the reaper is disposed LAST because releasing a job this process belongs to terminates it from inside the call. The shells are where the two roles meet — neither core references the other, so a Bundle stays not-a-Bridge — which is why the ProjectReference to the Bundle assemblies is on the SHELL and the view seam is Bundle.Core's `IHostView`. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
`TapScribe.Bundle.Launcher` is deleted. It named "the thing that starts the Recorder and isn't the Recorder", and once the Tray Bridge is that thing nothing is confusable enough to need naming — `RecorderSupervisor` names the mechanism in code (ADR-0022). The release `bundle` job now `needs: [build, tray]` and REUSES the `bridge-tray` artifact instead of publishing a tray of its own. One tray per OS is the whole decision, and building it twice is exactly where the two copies would drift: the same executable is a bridge-only tray in the zip and a Bundle's tray under the installer, because the host role switches on the payload staged beside it. `staging/launcher` becomes `staging/tray`, and the unpack step fails loudly if the zip does not contain the exe. The tray gains the single-instance mutex it never had — which matters now that it carries the host role, since two instances of a Bundle install would each boot a Recorder and fight over port 8001, the loser reporting its own sibling as "already running from somewhere else". `AppMutex` lists BOTH names for at least one release: an installer upgrading over a running old Launcher cannot see it under the new name alone, and would hit files-in-use instead of asking it to quit. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
The three ADRs this branch implements go to `accepted`. ADR-0024 (the macOS Bundle) stays `proposed` — it is not built. ADR-0025's Disconnect bullet is amended by what the implementation found: it CANNOT be "the existing `triggerPipeline: false` path", because that path builds a `MeetingController`, which requires a session id and throws without one, and every other thing End does is keyed on one too. ADR-0015's "two tray icons stay separate" consequence is struck through and annotated as reversed, which ADR-0022 required on acceptance — with the note that the topologies it protected are unaffected: a bridge-only install has no payload and shows no host section, and the SpatialChat extension still has no tray. ADR-0012's asset table gains `TapScribe-Setup-win-x64.exe`, which release.yml has attached all along and the table never listed. Not the macOS Bundle: it does not exist yet. CONTEXT.md's `Bundle` and `Host role` entries are rewritten rather than detagged — "Today that tray is a dedicated executable (the Launcher); ADR-0022 (proposed) replaces it" is historic narration once it has. `packaging/README.md` described a Launcher that is gone; it now describes the Bundle's assemblies, why they are not under `windows/`, and the staging table — including the row that IS ADR-0022 made concrete, the tray taken from the bridge-only artifact rather than rebuilt. The tray README gains a "Two roles, one tray" section naming the two behaviours that are easy to undo by accident, and saying plainly that macOS has no Bundle yet. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
Vortiago
changed the title
**One skeleton, not two.** `StartAsync` and `ConnectAsync` shared far more than the already-extracted `OpenTapsAsync`: the orchestrator wiring, the abandoned-publish race, the presentation tail, and — the actual hazard — the seven-type catch filter, typed twice. `RunTapsAsync` owns all of it and takes the four things the modes genuinely differ on. Typed twice, that filter drifts the first time a failure is added to one arm and not the other, and the symptom is a tray wedged on "Connecting…" forever. Net −19 lines, 472 tests unchanged. `MintAsync` now throws rather than answering an empty session id, which is what lets the shared path treat null as "an attached tap names no session" with no special case — the assertion moved to where the mint is. **`cookie_name` moves to `config.py`.** `auth.py` was reaching into `routes/` for it through a function-local import, re-run on every request — on the path the dashboard's 500 ms poll crosses, and three lines below `_TAP_PREFIX_SLASH`'s comment about pre-joining "once at import… no per-request concatenation". `config` is the leaf both sides already import, so one owner now costs no dependency edge and no per-request import. **`TrayHost` holds one `HttpClient`** instead of building one per call, the way `ControlClient` does; per-call timeouts come from a CTS. And `RecorderAnswers` goes through `ControlClient.CheckHealthAsync` rather than hand-rolling a second `GET /health` with its own timeout and its own idea of success. **Removed what nothing reads:** `HostController.Current`, `HostView.Managed` (the header says it in words and `CanStop` is the consequence), `_busy` (folded into the state claim, now made inside the lock), and `IChildProcess.Id` — the only reaper enrols by handle, and the POSIX one that would want a pid is ADR-0024's, not this branch's. `StartRecorder`'s guard now asks `Render(...).CanStart` rather than re-deriving it, so the menu and the command cannot disagree; two tests pin it, which nothing did before. **`test_login_routes.py` reuses `test_auth._mini_app`** instead of rebuilding it. The copy had already drifted — it named the tap token differently from the middleware's own tests. `routes/login.py` no longer manufactures a `LoginLinks` when `app.state` has none. That is a wiring bug, and papering over it would mint links against a store nothing reads: every one would fail to spend, with no clue why. Answered as a 503, the way the middleware answers a missing Recorder. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
Four gaps where a green suite proved less than it looked, each closed with a test that was checked by sabotage before it was kept. ADR-0025, against the real Python Recorder: - A rotation mid-attach. This is the claim the feature rests on — the tray names no session, so the RECORDER decides where each Utterance lands — and it was a manual step in the plan and nothing else. Connect, speak, rotate the session the way the dashboard's "+ new session" does, speak again: the audio follows into the new folder without the bridge being told. - The takeover drain. The fake-Recorder test asserts ordering; this asserts the operator's stake. Half a clip, cut mid-sentence so the Utterance is still open, then Start: the Recorder must have no tap left open (a takeover that skips the drain leaks it, and one microphone becomes two speakers across two sessions), and the live session must hold the bytes that were streamed. The first draft of this passed with the drain deleted: real speech has pauses, so an earlier Utterance had already closed and satisfied "a WAV exists". The header is no help either — `wave.writeframes` re-patches it on every write, so a streaming WAV already declares its size. Whether a tap is closed is asked of the Recorder, which is the only end that knows. ADR-0023, in a real browser: - A cookie-signed-in dashboard making a real SAVE. Nothing exercised the Origin check's PASSING side honestly: the middleware's own tests send `Origin: http://testserver`, which has no port, and every other dashboard-UI test runs the auth-off fixture. A wrong comparison 403s every save in the product and the suite stays green — confirmed by breaking it. - A dead link renders the used-up page, not a 401. Only a browser proves no native dialog was asked for. And the tray's own half of the login link, which had no test anywhere: the round-trip moves out of the WinForms shell into `Bundle.Core.LoginLink`, where the ubuntu leg runs it and ADR-0024's macOS Bundle inherits it. Six cases, including every way it can fail still opening the dashboard. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
Fifteen findings. Six were product breakage rather than polish.
The installer was pointing at a file that is never installed: the retired
Launcher published `TapScribe.exe`, the tray publishes
`TapScribe.TrayBridge.exe`, and every shortcut, the startup entry, the
uninstall icon and the post-install launch still named the old one. ISCC
resolves those at runtime and no CI job installs the setup, so it compiles
clean and ships three dead shortcuts. Worse on upgrade: the AppId is
unchanged and `[InstallDelete]` only covered the wheel, so the Launcher-era
exe survived and the shortcuts kept launching the RETIRED binary against the
new payload. `[InstallDelete]` now removes it.
`_stopping` was a one-way "the tray is quitting" latch, and this branch made
`Stop()` an operator-facing Stop Recorder that does not quit. Stop then Start
spawned a preflight, killed it on its own quit-race check, and returned with
no state report — the menu stuck on "Preparing TapScribe…" with both commands
greyed out until the process was restarted. `Start()` clears the flag; the
test that pins it fails with the exact wedge when the clear is reverted.
A 401 to a browser holding a dead session cookie omitted the challenge and
never cleared the cookie. The quiet 401 is right for the 500 ms poll and
wrong for a NAVIGATION, which the browser renders: the operator met raw JSON
with no prompt, no form and no way back, and a checkout install has no tray
to mint a fresh link from. A navigating request is challenged and its dead
cookie cleared; the poll stays quiet.
The Origin check compared against `str(request.base_url)`, which carries the
ASGI `root_path` and a trailing slash where an Origin has no path at all — so
behind any path-prefix proxy every same-origin POST/PUT/PATCH/DELETE was
refused 403. Scheme and netloc now, which is what an Origin is. (The
`origin.rstrip('/')` half was dead code: RFC 6454 serialises no path.)
`RunTapsAsync` had no `handled` backstop, and `before` had just widened what
runs inside its try to a TEARDOWN whose failure shapes are disjoint from the
catch filter. An out-of-filter throw escaped the fire-and-forget task
unobserved, leaving Busy and "Starting…" in place with Start, End, Connect
and Disconnect all disabled. `RunPipelineFlowAsync` already guarded exactly
this.
And `ShellOpen`'s failure path wrote the whole target — for "Open dashboard",
a login link whose `?k=` is a live single-use dashboard credential — into the
rotating log the menu invites the operator to open and paste, and into a
balloon. The query is stripped now, the same rule the password reads keep.
The rest: a takeover whose mint fails now says the attached tap stopped too
(the drain is not undoable, and "Could not start meeting" said nothing about
the room microphone that went quiet); a faulted Disconnect drain no longer
reports "the recording is saved"; `Running` is published before the Exited
subscription, so an instantly-dying Recorder's Unmanaged verdict is not
overwritten by a green menu over a dead dashboard; a Recorder that exits on
its own is disposed; `TryAttach` no longer takes the whole tray down —
bridge role included — when the user profile is unavailable; the host menu
items start disabled, so a click in the ~200 ms before Startup cannot boot a
second Recorder; a crashed Recorder balloons rather than waiting to be found
in a menu; `LoginLink`'s filter covers the shapes a 200 with the wrong body
takes, so "never throws" is true; the dropped-device sentence follows the
mode like the healthy one does; and `spend()` enforces its own TTL rather
than relying on the sweep having deleted the link first.
Cleanups alongside: a dead `Directory.Build.targets` whose `net10.0-windows`
condition can never match, the `download-artifact@v7` that was the only one
left behind at v7, `--no-build` on a test whose project the previous step
builds, the README's wrong macOS payload claim and a heading spliced into the
middle of its component list, and the ~17 surviving "Launcher" mentions
ADR-0022 retires — including an operator-visible log line and `--help` text.
Verified: pytest tests, ruff, dashboard-UI e2e, Bridge.Core 474/474 with the
5 real-Recorder E2Es actually running, Bundle.Core 74/74, every buildable C#
project. Not runtime-verified here: `TrayHost.cs` compiles but its WinForms
tests need Windows, and `TapScribe.iss` is edited, not compiled.
Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
Comment on lines
+125
to
+129
| catch (Exception error) | ||
| { | ||
| _log($"unhandled exception in supervisor: {error}"); | ||
| Fail($"TapScribe could not start: {error.Message} See the log for details."); | ||
| } |
`Adopt_RefusesAProcessAlreadyInTheJob_RatherThanThrowing` has never passed: it went in as a compile error, was fixed to compile, and has failed on the windows leg ever since with `Expected: False, Actual: True`. The premise is wrong. Since Windows 8 a process may belong to NESTED jobs, so assigning one to a job it is already in succeeds — the older documented refusal, which `CoversChildrenByInheritance` also still claimed in prose, is not what the kernel does. The contract worth pinning is the one the fallback path actually rests on: a refusal is an ANSWER, never an exception, because a tray that cannot enrol a child still has a meeting to record. So the refusal is now forced with a handle the kernel rejects outright, which is the one way to be refused that depends on neither the Windows version nor somebody else's job. The already-a-member case stays as a test, flipped to assert what actually happens, so the next reader does not rebuild `Adopt`'s fallback around a failure that never comes. Both tests are `[RequiresWindows]`, which skips them AT DISCOVERY — they report as three collected, zero run on this box, which is why five commits of local green said nothing about them. CI's windows leg is the only gate; this compiles here and is verified there. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
ADR-0024's first half, all of it in Bundle.Core so the ubuntu leg runs it — which is not incidental. The ADR rejects a pkg postinstall script *because* that would be untestable there, so a version of these rules that only ran on macOS would give the decision away. The Windows layout conflated two roots that macOS splits, so they are now named apart: - PAYLOAD — the interpreter and wheel as shipped. Beside the exe on Windows, inside the .app on macOS, where it is read-only: writing into a signed bundle invalidates its signature, which is the whole reason ADR-0024 exists. - RUNTIME — the interpreter pip targets. The payload itself on Windows (which is why that Bundle installs per-user rather than into Program Files), and a version-stamped copy under the data root on macOS. Python, Pythonw, WheelDirectory and ResolveWheel hang off the runtime, so RecorderCommand, preflight and /setup target the copy without knowing either platform's story. The macOS copy is a fact about the layout, not a branch anyone downstream carries. `Resolve` becomes `ForWindows` beside `ForMacOS`; with two shapes the old name said nothing. Three things the macOS shape settles: the data root is ~/Library/Application Support/TapScribe, where Bridge.MacOS/TrayStores already keeps the tray's settings — so settings, recordings, config, .auth-password, .tap-token and the runtime share one folder, and a bridge-only operator who later installs the Bundle keeps their settings. The interpreter is bin/python3, POSIX-shaped, and Pythonw is deliberately the same binary because a child of a GUI app gets no terminal to begin with. And the version reaching the stamp is whatever `-p:Version=` was given — a git tag, external text, becoming a directory name under the operator's home — so it is reduced to one safe segment rather than trusted. RuntimeCopy is the first launch. The atomic rename IS the completion marker: the copy lands in <version>.partial/ and becomes <version>/ only once every byte is written, so a crash partway through 300 MB leaves something nobody can mistake for a runtime. No sentinel file to write, forget to write, or write too early. The old runtime is kept until the new one is complete, then deleted; a failed integrity check deletes and re-copies, which is what makes "reinstall to repair" true on a platform where reinstalling replaces the .app and never the copy. It answers an outcome rather than a bool, because only Upgraded and Repaired lose the backends /setup pip-installed and a shell inferring that from paths would get it wrong the first time a fourth outcome existed. The atomicity test was checked by sabotage: copying straight into runtime/<version>/ instead of through the partial leaves the suite red on exactly one case, which is the one standing between a crashed copy and an interpreter that is broken forever — because its existence is the ONLY thing "already copied" is decided on. Bundle.Core 96/96. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
…aces ADR-0024 installs `TapScribe.app`; the bridge-only build published `TapScribe.TrayBridge.MacOS.app`. One tray per OS (ADR-0022) means they are the same app, so they get the same name and a Bundle upgrades a bridge-only install in place. The bundle IDENTIFIER stays `net.havso.tapscribe.traybridge`, which is what macOS actually treats as the app's identity — the path is not it. `AssemblyName` is what renames the bundle, and it renames the executable inside it too, so this is one commit or a broken one: the csproj, the `Info.plist`'s `CFBundleExecutable`, `ShellBundle.cs`, ci.yml's quarantine proof (the find, the installed path and the executable check) and its launch proof, release.yml's `APP:`, and the README. `ShellBundle` had ONE constant serving both the project on disk and the bundle it produces; they are two things now, and the day they diverged silently was going to be this one — the search would have looked for a bundle that no longer exists and failed every test in the project with it. `CFBundleDisplayName` drops "Bridge" for the same reason the rename happens: after ADR-0022 the app is not the bridge half of anything. The orphan is the part a rename alone does not fix. `installer` overwrites BY PATH and never removes what an older receipt put elsewhere, so an operator upgrading from today's .pkg would get the new app BESIDE a dead `TapScribe.TrayBridge.MacOS.app` — precisely the two-trays outcome the rename exists to prevent. So the successor removes it on first launch: one known path, only when the app doing the removing is itself the installed one, and never anything it did not ship. A cleanup that went looking for "things that look like old TapScribes" would eventually find something that was not one. In `Bundle.Core` rather than a pkg postinstall script, for the reason ADR-0024 already rejects those — and so the rule runs on the Linux leg. The contributor case is the one worth naming: a debug build out of `bin/` must not delete the operator's installed app, and it is tested. The removal is sequenced BEFORE the launch and AFTER the version floor. The launch does not return — quitting terminates the process from inside AppKit's run loop — so anything after it never happens; and a Mac below the floor is refusing to run, which is no moment to delete an app the operator may still want. Both pinned. Bundle.Core 104/104. The shell itself compiles only on CI's macos leg (the `macos` workload needs Xcode), which is also where the quarantine and launch proofs run — so that leg, not this box, is the gate for the rename. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
ADR-0024's second half: tasks 25 and 26. THE REAPER. Windows gets the crash path from the kernel — JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE fires on process DEATH, so a tray that is killed still takes the Recorder and its whisperlivekit-server grandchild with it. macOS has no equivalent and no PDEATHSIG, so it takes two pieces and neither is sufficient: - The tray LEADS a process group. A child inherits its parent's group and so does a grandchild, so leading it once before any spawn closes the window in which the Recorder forks WhisperLiveKit before the tray could have enrolled it — the exact analogue of the job object's self-enrolment, for the exact same reason. This is what reaps on an ordinary Quit. - A WATCHDOG holds the parent-death watch, because a group is only reaped by someone still alive to signal it. It is the tray's own binary re-invoked with `--reap-group <pid> <group>`: one thing to sign, one to install, nothing that can go missing on its own. It calls setsid() FIRST — it inherited the very group it is here to kill, so without that it signals itself and the Recorder outlives it, which is the leak this process exists to prevent wearing the one shape nothing else would catch. Then kqueue EVFILT_PROC/NOTE_EXIT, then SIGTERM, then SIGKILL for what ignored it. Bundle.MacOS is net10.0, not net10.0-macos: a DllImport into libc needs no platform TFM. That matters more here than it did for the Windows half — the shell that consumes it can only be built on a Mac, so without the split nothing about the reaper would compile anywhere but one CI job. The ubuntu leg now compiles BOTH reapers. The argv is the half that can be got wrong silently, so it is in Core where the Linux leg tests it, round-tripping through the builder and the parser that meet in two different processes. Everything malformed is refused rather than guessed at — and 0 and -1 especially, which are not merely invalid but the killpg arguments meaning "my own group" and "everything I may signal". Reaching either through a loose parse would take the operator's session down. THE HOST ROLE in the Mac tray mirrors TrayHost item for item, including the seven fixes the review pass just made to it — host items disabled until the first ShowHost, the mint off the UI thread, the login link's `?k=` stripped from the failure path, a guarded TryAttach that degrades to bridge-only rather than taking the bridge down with it, alert-once keying, and the ShowLog existence check. Written from the post-review TrayHost deliberately: from the pre-review one it would have re-shipped all seven. Two things are genuinely macOS's. The first launch copies the runtime before booting anything, off the main thread — 300 MB on AppKit's thread is a beachball for the length of it — and says so when an upgrade took the operator's model backends with it. And "Reveal recordings in Finder", which is not a nicety: the data root is under ~/Library/Application Support, which Finder hides, and that location was chosen precisely because ~/Documents and ~/Desktop are TCC-protected. This item is the cost of that choice. The version stamp is read from the assembly, where the release job's single -p:Version= already reaches, with the +<sha> build metadata trimmed — left on, every commit would be its own runtime and a contributor's Mac would re-copy 300 MB on every build. Bundle.Core 124/124, both reapers compile here. The SHELL does not: the macos workload needs Xcode, so CI's macos leg is its only compile gate, and the kernel behaviour underneath it is unverified until the Apple-silicon pass (task 36, HITL, which is in the plan for exactly this). Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
|
|
||
| StopWatchdog(); | ||
|
|
||
| if (Posix.killpg(_group, Posix.Sigterm) != 0) |
| ArgumentNullException.ThrowIfNull(log); | ||
|
|
||
| int self = Environment.ProcessId; | ||
| if (Posix.setpgid(0, 0) != 0) |
| // SIGTERM, then SIGKILL for whatever ignored it. The Recorder handles SIGTERM and | ||
| // shuts its own children down; WhisperLiveKit, when it is the one left, does not | ||
| // always. | ||
| Posix.killpg(request.GroupId, Posix.Sigterm); |
| // spawned by the tray and therefore INHERITED the very group it is here to kill, so | ||
| // without this it signals itself and the Recorder outlives it — the exact leak this | ||
| // process exists to prevent, in the one shape nothing else would catch. | ||
| if (Posix.setsid() == -1) |
Tasks 28 and 29, and ADR-0024 goes to accepted. The packaging turned out to need no second recipe. The payload lives INSIDE the .app (Contents/Resources), so `build-macos-pkg.sh` already packages a Bundle correctly — what was missing was the staging. So `packaging/macos/build-bundle-pkg.sh` copies the interpreter and the wheel in, re-signs ad-hoc over them, and DELEGATES. It restates none of the packaging flags: the --analyze → BundleIsRelocatable NO → --component-plist sequence and the <relocate> self-check keep one owner, and that owner is the script ci.yml's quarantine proof already runs. Which settles the question that gate raises. CI keeps proving the BRIDGE-only .pkg. It is the same script, so the recipe under test is the recipe the Bundle ships, and pulling ~300 MB of python-build-standalone into every PR to re-prove a property about `installer` and quarantine — which is not a property of payload size — would buy nothing. Written down in the plan rather than left as an omission someone has to reconstruct. Re-signing is not housekeeping. The SDK ad-hoc signs the .app at build time and writing into a signed bundle invalidates that signature — the exact failure ADR-0024 exists to keep off an operator's machine, so it must not be shipped into one either. The seal now covers what was added. The release job mirrors the Windows one, including the part that matters: `needs: [build, tray-macos]`, reusing the .app that job already published rather than publishing the tray a second time. One tray per OS is the whole of ADR-0022, and building it twice is exactly where two copies would drift. The bridge-only zip stays the bundle WITHOUT the payload, which is why the two macOS assets differ in size and not in tray. It asserts `staging/python/bin/python3` for the reason the Windows job asserts both its interpreters: BundleLayout hard-codes the path, and an upstream layout change has to fail there rather than as a silent "nothing happens when I click the tray icon". Docs: ADR-0024 accepted; ADR-0012 gains the .pkg row; CONTEXT.md's Bundle entry restated as current state — Windows AND macOS, differing in where pip may write; the tray README's "macOS has no Bundle yet" replaced by what the Mac shell now does; packaging/README.md gains the macos/ section and Bundle.MacOS. Bundle.Core 124/124. CI's macos leg has now compiled the shell and passed the quarantine and launch proofs under the TapScribe.app rename, which is the gate for everything in the previous two commits that this box cannot build. Still unverified, and recorded as such: the reaper's kernel behaviour, the copy against a real 300 MB interpreter, and the AppKit calls — all task 36, on Apple hardware. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
Vortiago
changed the title
…tartup Two gaps in what just landed. `build-bundle-pkg.sh` had never been executed. It runs only in a release job, which fires on a tag, so `bash -n` was the whole of its coverage — and the shared-recipe argument covers the pkgbuild FLAGS, not the staging and re-sign half that this script actually adds. This repo already ruled on exactly that shape: `bundle-build` exists on the Windows side because "the release job runs on a tag only, so without this a break here would not be discovered until release day." So the macos leg now runs it against a STUB payload and asserts the staged paths inside the built package — the three-way contract between BundleLayout.MacOSPayload, the tray's role probe and the script. A stub because what is under test is where files land and whether the signature survives being written over, neither of which is a function of 300 MB. The quarantine and launch proofs stay on the bridge-only package deliberately: installing a payload-bearing app would have the launch proof boot the host role against a fake interpreter, which is a different and flakier test. What the stub cannot stand in for — `codesign --deep` over a real interpreter's Mach-O dylibs — is now a named watch item for the Apple-silicon pass rather than a surprise on tag day. And `MacTrayHost.Startup` was missing the backstop `RunTapsAsync` was given in the review pass, for the same reason and with the same symptom: it is fire-and-forget, so anything outside its filter faulted the task unobserved — no report, no boot, and a menu stuck on "Starting…" with both commands disabled until the tray was restarted. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
Four cleanup passes over the ADR-0024 work, converging on one theme: the Mac
tray had been written as a mirror of the Windows one, and what got mirrored
was not only widgets. Every rule below now has one implementation, in the
assembly the Linux leg tests, instead of one per shell — of which exactly one
is compilable on any CI leg this repo runs.
The runtime copy moves from MacTrayHost.Startup into RecorderSupervisor.
RunCore, which is already on a background thread, already has both catch
tiers, and — the part that matters — is what the operator's "Start Recorder"
goes through as well as the first launch. From the shell it could never be
retried: a failed first copy left Start Recorder going straight to
ResolveWheel, which reads the RUNTIME's wheel folder, and answering "no
TapScribe wheel found" forever. Windows pays nothing; its runtime IS its
payload, so Ensure answers NotNeeded without touching the disk. The
"backends are gone" advice rides a notice channel rather than a state,
because the menu header renders a state and this outlives one.
The login link's password read and its secret redaction move into LoginLink.
The redaction is the one that had already gone wrong: Windows used Uri and
preserved file:// targets whole, macOS cut at the first '?' and truncated
them. That rule guards a live single-use dashboard credential on its way into
a log the tray invites the operator to paste, and it had two implementations
and one test.
Alert-once moves into HostController, beside IsBad, which is the other half
of the same question. The health probe moves to ConnectionTester, whose own
docstring already made this argument about a sibling — Bridge.Core rather
than Bundle.Core, because a Bundle is not a Bridge and must not reference
one. "recordings" joins every other folder name on BundleLayout.
IChildProcess gains ProcessId. The macOS reaper was doing
`(int)child.NativeHandle` — reinterpreting a Windows HANDLE, which happens to
work on .NET-for-Unix and is documented nowhere. Its failure is silent:
setpgid on a garbage id returns -1, Adopt answers false, and the orphan the
reaper exists to prevent is back with nothing in the log worth reading. The
seam's own comment had predicted this ("a POSIX reaper would want the pid
instead; it gets a member when there is one").
A REAL BUG, found by the simplification pass rather than by me. In
ParentDeathWatch.WaitForExit the "no kqueue" branch was commented "fall
through to the poll below" and was `return;` — so a kqueue that could not be
created had the watchdog skip the wait entirely and kill the Recorder's
process group immediately, while the tray was still running. Split into
TryWatchForExit + PollUntilGone, where the fallback is one readable
predicate instead of an interaction between an early return, a return inside
a try, and a loop after the finally.
Smaller: BundleLayout carries the three platform-varying values as state set
by the factories, rather than branching on Shape at each use — a third shape
then has to answer to the constructor instead of falling through a two-armed
branch to whichever side was written second (and the switch form it briefly
had warned CS8524 rather than erroring). RuntimeIsACopy reads the shape
instead of re-deriving it by comparing two strings. RuntimeCopy enumerates
the version folders once, not twice, and CopyTree walks the tree once rather
than sweeping it twice into two materialised arrays — tens of thousands of
entries for a real interpreter, in a menu-bar app that otherwise idles small.
ProcessGroupReaper.TryCreate takes a nullable path and owns its own
degradation rather than having the call site launder a null into a throw that
would take the whole host role down. The pasteboard incantation is shared
inside the macOS project, with the empty-text guard the second copy had lost.
ci.yml resolves the .app path once, in the step that already enforces
exactly-one-match. And packaging/README.md's ADR-0024 link was broken.
Not done, and why: parallelising the file copy. The efficiency pass found
that .NET's File.Copy uses copyfile with COPYFILE_CLONE_FORCE on APFS, so the
common case clones rather than copies and the win is small — while
Parallel.ForEach wraps failures in AggregateException, which is neither of
the types the supervisor's named filter catches, so a disk error would stop
being reported with its real message.
Bundle.Core 124/124, Bridge.Core 474/474, both shells and both reapers build
with zero warnings.
Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
| // No run-once dance of its own: Dispose is the only caller and already guards with | ||
| // _disposed, so a second mechanism tracking "the watchdog has been dealt with" would | ||
| // only make a reader check whether either can fire without the other. | ||
| if (_watchdog is not { } watchdog) |
| { | ||
| ArgumentNullException.ThrowIfNull(child); | ||
|
|
||
| return child.ProcessId > 0 && Posix.setpgid(child.ProcessId, _group) == 0; |
| } | ||
| finally | ||
| { | ||
| Posix.close(queue); |
| /// </summary> | ||
| private static bool TryWatchForExit(int pid) | ||
| { | ||
| int queue = Posix.kqueue(); |
| // always. | ||
| Posix.killpg(request.GroupId, Posix.Sigterm); | ||
| Thread.Sleep(GraceBeforeKill); | ||
| Posix.killpg(request.GroupId, Posix.Sigkill); |
A completeness audit over the whole branch — asking what was SKIPPED rather
than what is wrong, which is the question the review and simplify passes are
structurally bad at. Four things did not hold.
THE ONE THAT IS A REGRESSION. ADR-0023 moves the dashboard from Basic to a
session cookie, and that cookie lives in the Recorder's memory, so a restart
signs an open tab out — including via Stop Recorder / Start Recorder, which
this very branch adds to the tray menu and which an operator will use
casually. Under Basic the same restart recovered invisibly: the browser
re-sent cached credentials against the persistent .auth-password. Now the tab
polls a 401 every 500 ms behind a frozen page and says NOTHING — tick()
catches, writes into #spine only if the page has not rendered yet (never true
by then), console.errors, and retries forever. The operator sees stale
sessions and no captions, reads it as a hung dashboard, and reloads — and a
reload is a NAVIGATION, which auth.py deliberately DOES challenge, so the
silent version does not merely omit a cue: it walks them into the native
Basic dialog this feature exists to remove.
auth.py's own comment had already specified the missing half — "the
dashboard's own fetch sees the 401 and can say so quietly" — and nothing
implemented it. It does now: HttpError carries `status` as a field (the
message text was the only way to tell which failure it was), and the poll
paints a cue naming BOTH ways back in, because there are two installs — a
Bundle has a tray to mint a fresh link, a checkout has only the password.
#errbar is shared with wireErrorBar, so the BAR'S OWN TEXT is the state and
there is no flag beside it: the cue re-asserts itself when something displaced
it and clears only its own words. A flag would claim the cue was showing while
wireErrorBar had since replaced it — which is the common case, not a corner one,
because a signed-out spell is exactly when the other writer is busy (every lazy
import 401s too), and it would then both fail to restore the cue and refuse to
clear the message that displaced it. Not routed through wireErrorBar's own
`show` either, which beacons /api/client-errors per call — one beacon per tick,
forever, for a condition the server caused. Pinned by a browser test that signs
in with a real link, replaces the store, and requires the cue; sabotage-checked
by deleting the branch.
A GUARD MISSING IN THE MACOS WATCHDOG, the same shape as the one /simplify
fixed and one step narrower. kevent reports a failed REGISTRATION not through
its return value but as a returned event carrying EV_ERROR — there is room for
it in the eventlist, so the call answers 1, "one event". Reading only the
return value counts a refusal as an exit. For the likeliest errno (ESRCH: the
tray is already gone) killing the group is right by accident; for any other
one on a LIVE tray the watchdog reaps the operator's Recorder mid-meeting.
Answering false falls to PollUntilGone, which settles the dead-pid case on its
first kill(pid, 0) anyway. Compiles here, runs nowhere on this box — a task-36
watch item.
TASK 5 CLAIMED "StatusViewTests.cs extended" AND IT WAS UNTOUCHED. Attached,
Connecting and Disconnecting had no test at all; the one assertion naming
TrayStatus.Attached compares StatusView.For to StatusView.For, so it pins which
status the runtime EMITTED, not what it renders. §1.5's requirement — that
Attached reuse Streaming's Unheard rule — was therefore free to be deleted.
Six tests now, and flipping the Attached arm to TrayIcon.Streaming turns one
red (checked). A room mic IS the attached tap's usual shape, so a dismissed
grant is likelier here than in a meeting, not rarer.
THE LOGIN TOKEN STILL REACHED A LOG. The route answers a 303 precisely so the
token leaves the address bar, history and the next Referer; uvicorn's access
log renders the path WITH its query string, so it held it anyway — and in a
Bundle the tray pumps that stdout into the rotating recorder.log its own "Show
log" invites the operator to open and paste. Inside the grace window a
re-spend of it re-issues the live session cookie. The review pass closed the
tray-side twin of this in 34c; this is the Recorder side. Redacted rather than
silenced (dropping the line would drop the only record that somebody signed
in), and the WHOLE query goes rather than a parameter named `k`, so renaming
it cannot silently reopen the leak.
Smaller, from the same audit: task 32's own verify step ("grep for Launcher")
failed — eleven files still named the retired executable in the present tense,
including three lines of ADR-0015 that describe an artifact no release will
produce again. The word is now "the tray" everywhere it means the live thing;
ADR-0015's Decision keeps its own words with a superseded-in-part note, the way
its reversed consequence was already annotated. And a new
tests/test_bundle_ci_coverage.py: the Bundle solution IS built whole by
bundle-build, so the auditor's "a new project is never built" was wrong, but a
new TEST project would be compiled and never run — coverage that asserts
nothing, the same shape as the [RequiresWindows] discovery-time skip that let
this branch's CI stay red for five commits.
One knock-on: HttpError is NAMED, so `String(e)` renders "HttpError: 404 …"
where it rendered "Error: 404 …". `errText` is the house convention and already
covered most call sites; the seven operator-facing ones still interpolating a
bare `${e}` — four alerts, three failMessages and the spine's own state-error
line — now use it too. `/setup` is untouched: it has its own fetch layer and no
HttpError ever reaches it.
Not changed, and why: JobObjectTests' seam assertion and
RecorderSupervisorTests' never-reads-output test cannot fail (the first is a
compile-time truth, the second names a member IChildProcess does not have).
Both are harmless and both read as coverage; deleting them is a judgement call
for a quieter moment, not a fix. The macOS case of an OLD tray still RUNNING
when its successor removes the legacy bundle is unaddressed and unverified —
LaunchServices behaviour I cannot test here. Task 36.
Bundle.Core 124/124, Bridge.Core 480/480, dashboard UI green, zero warnings.
Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
| // and for every other one the watchdog reaps a live tray's Recorder mid-meeting. | ||
| // Answering false sends this to PollUntilGone, which settles the already-dead case | ||
| // on its first kill(pid, 0) anyway. | ||
| int events = Posix.kevent(queue, ref change, 1, ref fired, 1, IntPtr.Zero); |
… to expect An xhigh review over the whole changeset. Eleven findings fixed, one left as a decision. The two that would have been felt by an operator are both in the macOS first launch, and both are cases where one part of the branch already knew what the other part was getting wrong. THE COPY FOLLOWED SYMLINKS, on a tree the branch's own packaging script says is made of them. `build-bundle-pkg.sh` reaches for `ditto` "rather than cp -R, for the symlinks and exec bits a .app and a python-build-standalone tree are both made of" — and then `RuntimeCopy`, copying that same staged tree out of the .app on first launch, walked it with `File.Copy` and plain recursion. Three failures in one: a dangling link throws FileNotFoundException, which RunCore reports as "TapScribe could not start", so the Bundle simply never boots; a directory link pointing at an ancestor recurses until the stack goes, and a StackOverflow is not something any catch tier can absorb — it takes the tray with it; and every ordinary link (`bin/python3 -> python3.13`) is silently duplicated as a second full copy of the binary, which is also a codesign surprise waiting on real hardware. CopyTree now RE-CREATES links, checking the directory case BEFORE it recurses, which is the only order that helps. The new test builds all three shapes; before the fix it does not fail so much as never return. A QUIT DURING THE RECORDER'S SPAWN ORPHANED IT. `RunPreflight` already guards this exact window at its own spawn and `StartRecorder` did not: Stop() reads `_recorder` under the lock, so a Quit landing after RunCore's check and before the publish finds nothing to kill, and the child that lands a moment later is reachable by nobody. The reaper usually covers it — but a null reaper is an explicitly supported degraded path, and on THAT path the Recorder and its WhisperLiveKit grandchild hold port 8001 until the machine is rebooted, which is the one leak this class exists to prevent. Now it mirrors its twin: claim under the lock, kill and dispose outside it. The test drives Stop() from inside the spawn delegate, which is the only way to reach that window. Also fixed, from the review's own list: - A failure DELETING a superseded runtime aborted the boot. The new runtime is already complete and renamed into place by then, so a locked file in `runtime/1.3.0` turned a successful upgrade into "TapScribe could not start" over a folder nobody needed. Logged and carried past; the next launch retries. - `StartAsync`'s takeover cue moved into a `finally`. The drain is the likeliest thing to fail there and its failure shapes are disjoint from `RunTapsAsync`'s catch filter, so an IOException out of `EndMeetingAsync` — the exact shape `DisconnectAsync` catches around the same call — skipped the one notice written for it, and the operator heard nothing about the room microphone that had stopped feeding the current session and cannot be un-drained. Widening the filter was tried first and reverted: `BridgeRuntimeStartTests` uses IOException as its deliberate out-of-filter sentinel, and the test said so. - `ChildProcess.Start` disposes the Process when Start or BeginOutputReadLine throws, which is the Win32Exception path the supervisor's catch is written for and one retry per click of Start Recorder. - Three stale doc/comment blocks, two of them mine: `/simplify` orphaned `DescribeAsync`'s summary onto `AnswersOnLoopback` and left `SignedInUrl`'s `<param>` tags on `DashboardUrlFor`, and the Launcher→tray rename in the last commit broke a sentence in `routes/setup.py` into "the Bundle's the tray passes its wheel". Three the review reported and did not fix, which I did: - THE REAPER COULD KILL PROCESSES TAPSCRIBE NEVER STARTED. When `setpgid(0, 0)` fails, `TryCreate` fell back to `getpgrp()` — whatever group the tray happens to sit in — and the watchdog later SIGTERMs then SIGKILLs everything in it. For the ordinary errno (already a session leader) that group IS ours and nothing checks it; for any other, a non-leader adopts an inherited group holding strangers. Now the group must equal our own pid, and otherwise we go without the backstop — a state this type already supports and already says out loud. Killing somebody else's processes is strictly worse than having no parent-death watch. - `HostController.Report` computed its view under `_gate` and posted it outside, so two reports could reach the view in the opposite order to the state they recorded — the menu header saying "TapScribe is running." with `_state` recorded as Stopped, disagreeing with the commands the same render enabled, and nothing correcting it because for a stopped Recorder no further report comes. Stamped under the lock, stale posts dropped. Posting under the lock was the other option and is worse: one shell's post is an inline call, so it would run shell code holding `_gate`. Tested by holding the posts and running them backwards, which needs no threads to reproduce. - `BundleDefaults.RecorderPort`/`DashboardUser` and the mint response's `path` key restate `config.PORT`, `config.AUTH_USER` and `routes/login.py` across a language boundary with nothing holding them in lock-step — the shape CLAUDE.md answers with a mechanical check. Both sides are tested today and both stay GREEN through a drift: the C# asserts the key against a fake handler it writes itself, the Python asserts it against the real route, and no test crosses. Change AUTH_USER and every mint 401s, which LoginLink answers by silently opening the dashboard signed out — a failure that reaches the operator as a password prompt and reaches no log at all. `tests/test_bundle_recorder_contract.py` reads both sides and compares; each of the three drifts turns it red (checked). Not a stamper: the /tap wire has four languages and a dozen constants restated in prose, this is three values in one direction. Left as a decision, not a bug: a cookie holder can `POST /api/login-link` and renew their own session indefinitely, and issued cookies are never swept. That is ADR-0023's session model as written — `login_links._sweep`'s docstring states it and gives the reason (minting requires the password; the count is bounded by how often the operator has signed in) — and the missing piece is a sign-out route, which ADR-0023 does not promise. Worth a decision later, not a quiet change now. Every new test sabotage-checked independently of the review agent: reverting the line each one protects turns it red. Bundle.Core 127/127, Bridge.Core 480/480, both reapers build with zero warnings. Claude-Session: https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv
| // SIGTERMs then SIGKILLs everything in the group once the tray exits. Killing a | ||
| // stranger's processes is strictly worse than having no backstop, and no backstop is | ||
| // a state this type already supports and already says out loud. | ||
| int group = Posix.getpgrp(); |
…rts collection anyio 4.15 turned `anyio.abc.BlockingPortal` into a deprecating lazy alias for `anyio.from_thread.BlockingPortal`. starlette's testclient still names the old spelling at MODULE level, so under `filterwarnings = error` every collection that imports fastapi.testclient aborts: 26 modules here, before a single test runs. Nothing in this repo says `anyio.abc.BlockingPortal`, so there is nothing to migrate on our side. ci.yml's `tests` job installs anyio unpinned, which is why the last green run predates 4.15 rather than proving anything about it. Matched by MESSAGE, like the starlette httpx line above it: the class is anyio's own private deprecation warning, not a public symbol a filter can name. Remove when starlette migrates to the `from_thread` spelling.
`fs.globSync` answers platform-native separators, but every path predicate in
the checkers is spelled with `/`: the shared SKIP, check-conventions'
SKIP_EXTRA_DIRS, and the `split("/").pop()` that reduces a path to a basename.
On Windows none matched, so every exemption was a no-op and check-conventions
scanned the `web/js/lib/` and `web/tools/` files the exemptions exist to spare,
then reported findings against the vendored canon itself. That is not only CI
green and local red: `.claude/hooks/stop.sh` runs the three gates and exits 2,
so the turn was blocked on every Windows checkout, by a failure whose findings
pointed at files CLAUDE.md forbids editing.
Fixed upstream rather than here, because these are copy-verbatim files and
patching one in place is the `forked` state check-vendored exists to catch.
Canon now normalises at the single place paths ENTER (`scanPaths` in
js-scan.mjs, beside ROOT and SKIP), three tests pin it, and all five copies are
re-taken whole at @b05c263 rather than cherry-picked.
Taking them whole also brings check-slots' root-slot rule, which canon gained
since @9f20766 and this copy never had: a `data-slot` on a template's own root
element, read back through `.firstElementChild`, silently renders nothing.
It finds no violations here, so the rule arrives already satisfied.
Verified on Windows: all three gates exit 0 (check-conventions now reports 72
files, having previously scanned everything), 240 dashboard JS tests, and
`tsc --noEmit`. On Linux the change is inert, since `Path.GetFullPath` and the
separator are already what the predicates expect.
`BundleLayout` and `RecorderCommand` normalise what they are handed through `Path.GetFullPath`, and a POSIX-rooted literal like "/opt/prog" is not absolute on Windows: it is drive-RELATIVE, so it comes back as "D:\opt\prog" off whatever the current drive is. Eleven tests compared the layout's answer to the raw literal and failed on Windows alone. CI never saw it. ci.yml runs this project on the ubuntu leg only (line 793); the Windows leg builds the solution and runs Bundle.Windows.Tests, so a Windows contributor following packaging/README.md's own "anywhere" command got eleven red on a clean checkout. The production code was correct throughout. Both sides normalise now, through one `Rooted` helper, so an assertion is about the layout's shape rather than about .NET's drive-qualification rules. What it does NOT weaken: folder names, the joins, and data-outside-program all still fail if wrong — sabotaging `DataFolder` turns four of them red. The two cases that cannot call a helper carry it inline instead: `[InlineData]` wants a constant, and RecorderCommand's wheel spec is absolutised by the builder, which is its own assertion two tests down. On Linux every one of these is identity, so the ubuntu leg is unchanged.
…enance Verktoykasse #90 replaced the `@<rev>` stamp with `@<rev> sha256:<hash>` and made the hash what check-vendored classifies on, precisely because the rev could not do the job: it read an untouched copy as `forked` the moment canon moved, which is what these five files did here. Re-copied from the rebased canon and stamped in the new dialect, so they now report up-to-date rather than forked. check-vendored.mjs itself changed the most, since #90 rewrote it. The recorded hash is of the LF bytes. Verktoykasse's own fix in the same branch makes both sides of the comparison strip CR before hashing, because git hands the identical blob over as CRLF wherever core.autocrlf is true, and without that a stamp is only valid on checkouts whose line endings match the one that wrote it. LF is what the committed blob carries on either platform.
…run these on Windows Three files had grown three shapes for one fact — a `Rooted` method, a precomputed field, and an inline ternary — with the explanation restated at each. `FixturePath` holds it once; `RootedOrNull` covers the `[InlineData]` case whose expectation is null. BundleLayoutTests normalises its ROOTS rather than each expectation. Path.GetFullPath is idempotent, so a normalised root survives the layout unchanged and an assertion can name it directly; wrapping per assertion left every future one needing to remember the wrapper, which is the bug this file was fixed for, re-armed. The CI step is the actual root cause. The Core resolves a Windows-shaped layout while `bundle-core-crossplatform` runs it on ubuntu alone, so its tests were the one part of a Windows product never exercised on Windows. `bundle-build` already compiles the assembly, so running it there costs seconds. Gate tier re-stamped at the toolkit's a36aeda.
explorer.exe silently DROPS a URL carrying a query string and opens a folder window instead. Verified on Windows 11: http://host/p reaches the browser, http://host/p?k=v does not. Every minted login link is /login?k=<token>, so Open dashboard opened File Explorer every time and ADR-0023 — the whole no-password-prompt feature — never worked on Windows. The failure inverted, which is why it read as flaky rather than broken: when the mint SUCCEEDS you get a query string and a folder window, and when it THROWS the fallback is the plain signed-out URL, which opens fine and lands on the native Basic dialog the feature exists to remove. The feature worked only when it failed. The explorer indirection was not an accident and is kept in spirit: a direct ShellExecuteEx CreateProcesses the browser from inside this process, so it joins the tray's KILL_ON_JOB_CLOSE job and Quit takes the operator's browser, or every Notepad tab (Win11 Notepad is single-process). ShellTarget keeps the browser out of the job by asking to leave it explicitly — CREATE_BREAKAWAY_FROM_JOB, which is what JobObject's JOB_OBJECT_LIMIT_BREAKAWAY_OK was already there to permit and which nothing had asked for until now — and forwards through `rundll32 url.dll,FileProtocolHandler`, the documented shell entry point, which keeps the whole URL. Proven on this machine rather than reasoned about: a harness that creates a real KILL_ON_JOB_CLOSE job, enrols itself, opens a query-string URL and then exits (closing the job) leaves the tab open, with the query intact. It also stops lying about failure. explorer.exe returns 1 on success and on failure alike, so ShellOpen reported success either way; CreateProcess raises a real Win32Exception, and the existing catch turns that into the balloon the operator needed all along. Tests cover the command line, not the launch: a target's query string survives, a spaced log path stays one argument, and a target carrying a quote is refused rather than escaped, since nothing TapScribe opens can contain one.
Vortiago
added a commit
that referenced
this pull request
Sep 7, 2026
anyio 4.15 turned `anyio.abc.BlockingPortal` into a deprecating lazy alias for `anyio.from_thread.BlockingPortal`. starlette's testclient still names the old spelling at module level, so under `error` all 26 collections that import `fastapi.testclient` abort — every `tests` matrix job and the bridge E2E job go red on a tree nobody touched, on every OS and Python version. Matched by message rather than by class, for the same reason as the starlette line above it: the warning's class is anyio-private, not a public symbol to import. Remove when starlette migrates to the `from_thread` spelling. The same one-line fix rides along inside #449; this lifts it out so every open PR and main can go green without waiting on that feature branch. #449 will want the duplicate line dropped when it rebases. Refs #449
…tants Reported from a 150% display: descriptions truncated to one line, captions sitting on top of their inputs, "Test connection" showing as "Test", and Save/Cancel sliced off the bottom edge. Every control in SettingsForm was placed at a literal coordinate and every paragraph given a literal height, which is correct at exactly one scale. At 144 DPI the font comes back 24px tall instead of 15px while the constants do not move, so the whole dialog shears. Measured before the fix: the Devices paragraph had 44px for 125px of text, "Capture system audio ..." needed 469px of a 446px page, and the port spinner rendered 8001 as "800". Nothing is positioned by hand now. Every control sits in a TableLayoutPanel row and is AutoSize or docked, and the form declares AutoScaleMode Font against the 96-DPI metrics its sizes are written in -- one lever covering display DPI and the accessibility text-size slider alike. Tab bodies AutoScroll, which is the backstop that makes any remaining size safe to be wrong, and the dialog is Sizable rather than FixedDialog for the same reason. Three things AutoSize could not do on its own: - Wrapping paragraphs. WinForms has no AutoSize that wraps inside a TableLayoutPanel cell: a Percent column hands a control its width only AFTER the pass that decided the row height, so an AutoSize label reports its single-line width and never wraps. TrayLayout.Wrapped re-measures on resize, text change and font change -- one home for the trick. - NumericUpDown ignores AutoSize, so someone has to set its width. Measured from the largest value it accepts, once the scale has settled the font. - The pin grid collapsed to 1px at large text, because it sat in the row that takes what the others left and a Percent row cannot honour a child's MinimumSize. It is a content-sized row now, header plus four rows, and the page scrolls to reach it. MeetingForm had the same defect one file over -- a 26px caption row halving "Loading...", 28px buttons clipping "Copy". Both windows share TrayLayout, so there is one spelling of each primitive rather than a copy that drifts back. SettingsFormLayoutTests reports clipped / overlapping / escaped controls at 9, 11.25, 14.4 and 18pt, varying the scale through the FONT rather than the display so it holds on a 96-DPI runner. Sabotage-verified both ways: reinstating the fixed 44px paragraph height fails all four cases, and shrinking ClientSize fails the companion opens-unscrolled test. Worth knowing about that test: the first version was vacuous. On an unshown form every control reports Visible == false (the property is effective, not own-state), so it walked nothing and passed against a deliberately re-broken layout. Hence the Measured floor ahead of the fault check, and the forced handles -- a TabControl sizes its pages from OnHandleCreated, which never fires for an invisible child. Deleting AutoScaleMode breaks nothing the survey can see, since the pages simply scroll, which is why opening unscrolled is pinned separately. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
…achable Two bugs in the same file, both found by the gate refusing a green repo. The gate could not see its own tools. `command -v ruff` answers for the HOOK's PATH, which is the session's rather than the shell's, so on a local checkout that keeps its dev deps in .venv/ the answer was "not installed" for a machine that had ruff the whole time -- and nothing typed at a prompt could correct it, because a `PATH=... git push` prefix reaches the command, not the hook. It blocked every push on this box. Same cause one step down: `python3` on PATH is the system interpreter, which has no pytest, so a ruff-only fix would just have moved the block. Both now resolve PATH first (the web runner's session-start.sh pip-installs there) and then the repo venv, in both layouts. For the interpreter the import probe IS the resolution -- take the first candidate that can import pytest, rather than taking one and reporting the other's absence. The documented bypass never worked. The header has always said to prefix the push with CLAUDE_SKIP_PRE_PUSH=1, and the check read the hook's own environment, where a prefix never lands -- so the escape silently did nothing and the only way past a wedged gate was to not push. It now also matches the command text, which is where a prefix actually is. That match has to be narrow, and the first version was not: matching the marker anywhere in the command let `echo CLAUDE_SKIP_PRE_PUSH=1 && git push` open the gate, which is a bypass nobody wrote on purpose and one a reader skimming the line would not spot. It must be the env prefix OF THE PUSH, with a run of further assignments allowed between. Verified against a case matrix driving the hook with synthetic PreToolUse payloads -- prefix, prefix after &&, prefix plus a second env var, the value 0, the name inside a longer identifier, the marker merely mentioned in text, a non-push command, a push mentioned inside a string, and push-mirror. It is classified on what the hook PRINTS, because on a green repo a bypass and a real run both exit 0, so the exit code cannot tell "skipped" from "ran and passed" -- which is the whole point of the near-miss cases. The full gate then ran for real end to end: both ruff steps green, 3017 passed, push allowed. Also corrects a comment that claimed `echo "git push"` does not trip the gate. It does -- the match is text, not a shell parse -- and that is the safe direction to be wrong in, so the comment now says so rather than inviting someone to loosen the pattern. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
|
|
||
| [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] | ||
| [return: MarshalAs(UnmanagedType.Bool)] | ||
| private static extern bool CreateProcessW( |
Comment on lines
+65
to
+68
| if (!CreateProcessW( | ||
| null, commandLine, IntPtr.Zero, IntPtr.Zero, false, | ||
| CreateBreakawayFromJob | CreateNoWindow, | ||
| IntPtr.Zero, null, ref startup, out ProcessInformation info)) |
Comment on lines
+184
to
+192
| foreach (Control other in siblings) | ||
| { | ||
| if (ReferenceEquals(other, control)) | ||
| continue; | ||
| Rectangle hit = Rectangle.Intersect(Ink(control), Ink(other)); | ||
| // One pixel of shared edge is a rounding artefact, not a collision. | ||
| if (hit.Width > 1 && hit.Height > 1) | ||
| faults.Add($"overlapping: {Name(control)} and {Name(other)} share {hit.Width}x{hit.Height}px"); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes TapScribe reasonable to run on one machine, for one person. Four ADRs were
designed in this branch; three are implemented here, and the fourth (the macOS
Bundle) is left
proposed— see the bottom.What an operator gets
the Recorder has open, the one the dashboard badges
● live. What a microphone ina meeting room wants, with nobody at the keyboard. Zero Recorder changes: the
mode is the Bridge omitting a query parameter.
and the browser lands signed in. The native Basic dialog never appears.
supervise and reap a co-located Recorder) whenever a host payload sits beside it
on disk. The Launcher is retired, as a word and as an executable.
Decisions worth a reviewer's attention
_attachedis a second nullable field beside_meeting, and its record carriesno session id. An attached tap genuinely has none — a rotation moves the
current session out from under it — so leaving the field out makes that bug
unwritable. It also makes "attached or bracketed, never both" checkable.
triggerPipeline: false, contrary to ADR-0025'swording (amended here): that path is session-keyed and
MeetingControllerthrowswithout an id.
IHostViewlives in Bundle.Core, not onITrayView. A Bundle is not aBridge; the composition happens in the SHELL, the one place both roles meet.
Neither core references the other.
python/ORwheel/. An AND would silently demote aBundle with a wiped half to a bridge-only tray — the Recorder just vanishing from
the menu — and a
python/-only probe is that same bug mirrored./healthprobe, never byparsing the child's stdout: matching "address already in use" would mean owning
uvicorn's wording forever.
SpatialChat extension makes is under
/api/tap/or is/health, soallow_origins=["*"]stays load-bearing.bundlejob now reuses thebridge-trayartifact rather thanpublishing a second tray. One tray per OS is the decision; building it twice is
where the copies would drift.
Two behaviours were carried across from the Launcher with their reasoning intact,
because both regress silently:
ShellOpengoes viaexplorer.exe(a directShellExecuteExmakes the operator's browser a member of the KILL_ON_JOB_CLOSEjob), and the reaper is disposed last (releasing a job this process belongs to
terminates it from inside the call).
ADR-0024 — the macOS Bundle
A
.pkginstallingTapScribe.appto/Applications, carrying an embeddedCPython and the wheel as read-only payload. On first launch the tray copies the
interpreter out of the
.app, because/setuppip-installs at runtime andwriting inside a signed bundle invalidates its signature.
BundleLayoutsplits two roots the Windows shape conflated. Payload iswhat shipped; runtime is what pip targets. They are the same folder on
Windows and different on macOS, so
RecorderCommand, preflight and/setuptarget the copy without knowing either platform's story.
<version>.partial/and becomes<version>/only once every byte is written.Sabotage-checked: copying straight into
runtime/<version>/turns the suite redon exactly the case standing between a crashed copy and an interpreter that is
broken forever — because its existence is the only thing "already copied" is
decided on.
group (children and grandchildren inherit it — the analogue of the job object's
self-enrolment); a watchdog process holds a kqueue
EVFILT_PROCwatch, becausea group is only reaped by someone still alive to signal it. macOS has no
PDEATHSIGand noKILL_ON_JOB_CLOSE. The watchdog is the tray's own binaryre-invoked, and calls
setsid()first — it inherited the very group it is hereto kill.
TapScribe.TrayBridge.MacOS.app; both now installTapScribe.app, keeping thenet.havso.tapscribe.traybridgeidentifier, so a Bundle upgrades a bridge-onlyinstall in place.
installeroverwrites by path and never removes an olderreceipt's bundle, so the successor deletes the orphan on first launch — one
known path, only when it is itself the installed app.
TrayHostincluding the seven fixes the review passjust made to it, plus two things that are macOS's: the runtime copy off the
main thread, and Reveal recordings in Finder (the data root is under
~/Library/Application Support, which Finder hides — chosen over~/Documentsprecisely because those are TCC-protected).
Verified
Full
pytest tests(3234 passed, 27 skipped), the dashboard-UI suite, the headedbridge E2E under xvfb,
Bridge.Core.Tests(474) including the real-Recorder E2E(5/5 confirmed collected, not skipped),
Bundle.Core.Tests(124),Bundle.Windows.Testson CI's Windows leg, and every buildable project. Five newtests were sabotage-checked to prove they are not vacuous.
CI is green on the branch, which matters more than usual here:
TrayBridge.MacOSneeds Xcode's
macosworkload and cannot be built on Linux at all, so the macOSleg is the only compile gate for the shell — and it also runs the quarantine
proof, the launch proof, and a run of
build-bundle-pkg.shagainst a stub payloadasserting the staged paths inside the built package.
ADR-0012also gainsTapScribe-Setup-win-x64.exe, whichrelease.ymlhas beenattaching all along and the table never listed.
A completeness audit, and what it found
After the review and simplify passes, a third pass asked the question neither of
those is shaped for: not "is this wrong" but "what was skipped". It walked the
plan's task list and each ADR's consequences against the tree. Most of the branch
held — every mechanism the plan names is present, and the three attached-tap E2Es
really do run against a real Python Recorder on the Linux leg. Four things did not.
A regression this branch introduces. ADR-0023 moves the dashboard from Basic
to a session cookie, and that cookie lives in the Recorder's memory — so a restart
signs an open tab out, including via the Stop Recorder / Start Recorder menu this
same branch adds. Under Basic the identical restart recovered invisibly, the
browser re-sending cached credentials against the persistent
.auth-password. Thenew behaviour was to poll a 401 every 500 ms behind a frozen page and say nothing:
tick()caught it, wrote into#spineonly if the page had not rendered yet(never true by then), and retried forever. That reads as a hung dashboard, and the
reload it provokes is a navigation — which
auth.pydeliberately does challenge.So the silent version did not merely omit a cue; it walked the operator into the
native Basic dialog the whole feature exists to remove.
auth.py's own comment hadalready specified the missing half ("the dashboard's own fetch sees the 401 and can
say so quietly") and nothing implemented it. It does now, pinned by a browser test
that signs in with a real link, replaces the store, and requires the cue.
A missing guard in the macOS watchdog, the same shape as the one
/simplifyfound and one step narrower.
keventreports a failed registration not throughits return value but as a returned event carrying
EV_ERROR— there is room for itin the eventlist, so the call answers 1, "one event", and reading only the return
value counts a refusal as an exit. For the likeliest errno it is right by accident;
for any other one on a live tray the watchdog reaps the operator's Recorder
mid-meeting.
Task 5 claimed
StatusViewTests.cswas extended and it was untouched.Attached,ConnectingandDisconnectinghad no test at all, and the oneassertion naming
TrayStatus.AttachedcomparesStatusView.FortoStatusView.For— it pins which status the runtime emitted, not what it renders.§1.5's requirement that Attached reuse Streaming's never-heard rule was therefore
free to be deleted silently. A room mic is the attached tap's usual shape, so a
dismissed microphone grant is likelier here than in a meeting.
The login token still reached a log. The route answers a 303 precisely so the
token leaves the address bar, history and the next
Referer; uvicorn's access logrenders the path with its query string, so it held it anyway — and in a Bundle the
tray pumps that stdout into the rotating
recorder.logits own "Show log" invitesthe operator to open and paste. The review pass closed the tray-side twin of this;
this is the Recorder side. Redacted rather than silenced, because dropping the line
would drop the only record that somebody signed in.
Smaller, from the same pass: task 32's own verify step ("grep for
Launcher")failed — eleven files still named the retired executable in the present tense — and
a new
tests/test_bundle_ci_coverage.pycloses the narrow hole that a Bundle testproject would be compiled by the solution build and run by nothing, which is the
same shape as the
[RequiresWindows]discovery-time skip that let this branch's CIsit red for five commits.
Every new test was sabotage-checked: each goes red when the line it protects is
deleted.
What CI cannot reach
All four ADRs are
acceptedand the code is here. What remains is verificationthat needs physical machines, and it is worth being precise about which claims are
still unproven rather than merely untested:
dashboard, Quit reaping
whisperlivekit-server, a foreign Recorder showing asunmanaged, and an upgrade over a running old Launcher.
executes: the reaper's kernel behaviour (
setpgid/kqueue/killpg), theruntime copy against a real 300 MB interpreter,
codesign --deepover thattree's Mach-O dylibs, and every AppKit call in
MacTrayHost. CI's macOS legproves the shell compiles and that a package installs unquarantined and launches;
it does not exercise the host role, which needs a payload.
pass: an old tray still running when its successor removes the legacy
.appon first launch (LaunchServices behaviour, untestable here — the Windowsside handles the equivalent through
AppMutexcarrying both names), and theEV_ERRORguard above, which compiles on every leg and executes on none.https://claude.ai/code/session_012SV8YSNsYPtRgEFhUvgSVv