Skip to content

chore(deps): bump the cobra-viper group in /apps/agent with 2 updates - #3

Closed
dependabot[bot] wants to merge 1 commit into
mainfrom
dependabot/go_modules/apps/agent/cobra-viper-d714232373
Closed

chore(deps): bump the cobra-viper group in /apps/agent with 2 updates#3
dependabot[bot] wants to merge 1 commit into
mainfrom
dependabot/go_modules/apps/agent/cobra-viper-d714232373

Conversation

@dependabot

@dependabot dependabot Bot commented on behalf of github Jan 14, 2026

Copy link
Copy Markdown
Contributor

Bumps the cobra-viper group in /apps/agent with 2 updates: github.com/spf13/cobra and github.com/spf13/viper.

Updates github.com/spf13/cobra from 1.8.0 to 1.10.2

Release notes

Sourced from github.com/spf13/cobra's releases.

v1.10.2

🔧 Dependencies

  • chore: Migrate from gopkg.in/yaml.v3 to go.yaml.in/yaml/v3 by @​dims in spf13/cobra#2336 - the gopkg.in/yaml.v3 package has been deprecated for some time: this should significantly cleanup dependency/supply-chains for consumers of spf13/cobra

📈 CI/CD

🔥✍🏼 Docs

🍂 Refactors

🤗 New Contributors

Full Changelog: spf13/cobra@v1.10.1...v1.10.2

Thank you to our amazing contributors!!!!! 🐍 🚀

v1.10.1

🐛 Fix

v1.0.9 of pflags brought back ParseErrorsWhitelist and marked it as deprecated

Full Changelog: spf13/cobra@v1.10.0...v1.10.1

v1.10.0

What's Changed

🚨 Attention!

This version of pflag carried a breaking change: it renamed ParseErrorsWhitelist to ParseErrorsAllowlist which can break builds if both pflag and cobra are dependencies in your project.

  • If you use both pflag and cobra, upgrade pflagto 1.0.8 andcobrato1.10.0`
  • or use the newer, fixed version of pflag v1.0.9 which keeps the deprecated ParseErrorsWhitelist

... (truncated)

Commits

Updates github.com/spf13/viper from 1.18.2 to 1.21.0

Release notes

Sourced from github.com/spf13/viper's releases.

v1.21.0

What's Changed

Enhancements 🚀

Bug Fixes 🐛

Dependency Updates ⬆️

Other Changes

New Contributors

Full Changelog: spf13/viper@v1.20.0...v1.21.0

v1.20.1

What's Changed

Bug Fixes 🐛

Full Changelog: spf13/viper@v1.20.0...v1.20.1

v1.20.0

... (truncated)

Commits
  • 394040c ci: build on go 1.25
  • 812f548 chore: update dependencies
  • d5271ef ci: update stale workflow
  • dff303b feat: add a stale issue scheduled action
  • 1287976 build(deps): bump github.com/spf13/pflag from 1.0.7 to 1.0.10
  • 38932cd build(deps): bump github.com/go-viper/mapstructure/v2 in /remote
  • 6d014be build(deps): bump github.com/stretchr/testify from 1.10.0 to 1.11.1
  • b74c7ee build(deps): bump github.com/fsnotify/fsnotify from 1.8.0 to 1.9.0
  • acd05e1 fix: linting issues
  • ae5a8e2 ci: upgrade golangci-lint
  • Additional commits viewable in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore <dependency name> major version will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself)
  • @dependabot ignore <dependency name> minor version will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself)
  • @dependabot ignore <dependency name> will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself)
  • @dependabot unignore <dependency name> will remove all of the ignore conditions of the specified dependency
  • @dependabot unignore <dependency name> <ignore condition> will remove the ignore condition of the specified dependency and ignore conditions

@dependabot @github

dependabot Bot commented on behalf of github Jan 14, 2026

Copy link
Copy Markdown
Contributor Author

Labels

The following labels could not be found: dependencies, go. Please create them before Dependabot can add them to a pull request.

Please fix the above issues or remove invalid values from dependabot.yml.

Bumps the cobra-viper group in /apps/agent with 2 updates: [github.com/spf13/cobra](https://github.com/spf13/cobra) and [github.com/spf13/viper](https://github.com/spf13/viper).


Updates `github.com/spf13/cobra` from 1.8.0 to 1.10.2
- [Release notes](https://github.com/spf13/cobra/releases)
- [Commits](spf13/cobra@v1.8.0...v1.10.2)

Updates `github.com/spf13/viper` from 1.18.2 to 1.21.0
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.18.2...v1.21.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/cobra
  dependency-version: 1.10.2
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: cobra-viper
- dependency-name: github.com/spf13/viper
  dependency-version: 1.21.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: cobra-viper
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot
dependabot Bot force-pushed the dependabot/go_modules/apps/agent/cobra-viper-d714232373 branch from 24aced3 to bbf9ecf Compare January 26, 2026 15:36
@ToddHebebrand

Copy link
Copy Markdown
Collaborator

@dependabot rebase

@dependabot @github

dependabot Bot commented on behalf of github Feb 8, 2026

Copy link
Copy Markdown
Contributor Author

The dependabot.yml entry that created this PR has been deleted so this PR can't be rebased. Please close the PR so Dependabot can create a new one with the current dependabot.yml.

@ToddHebebrand

Copy link
Copy Markdown
Collaborator

Closing: the Go agent was moved from apps/agent/ to agent/. These dependency updates target the old path. Dependabot should auto-create new PRs for agent/ (see dependabot.yml).

@dependabot @github

dependabot Bot commented on behalf of github Feb 8, 2026

Copy link
Copy Markdown
Contributor Author

This pull request was built based on a group rule. Closing it will not ignore any of these versions in future pull requests.

To ignore these dependencies, configure ignore rules in dependabot.yml

@dependabot
dependabot Bot deleted the dependabot/go_modules/apps/agent/cobra-viper-d714232373 branch February 8, 2026 18:11
ToddHebebrand added a commit that referenced this pull request Apr 11, 2026
Database-layer hardening following the RLS security review. The app
previously connected as POSTGRES_USER (SUPERUSER with BYPASSRLS),
making every RLS policy a no-op. Closes that gap and fixes every
hole that was hiding behind it.

- Add unprivileged breeze_app role (NOSUPERUSER, NOBYPASSRLS) via
  new ensureAppRole bootstrap in autoMigrate. API connects via
  DATABASE_URL_APP with a startup warning if the connected user
  still has BYPASSRLS. (Finding #1)
- Rewrite broken backup/DR/C2C/vault policies that referenced an
  unset session variable app.current_org_id. 15 tables now use
  breeze_has_org_access(org_id) + FORCE ROW LEVEL SECURITY.
  (Finding #2)
- Add org_id column + RLS policies to device_metrics, backfilled
  from parent devices table. (Finding #3)
- Restrict cis_check_catalog write policies to system scope.
  (Finding #4)
- Replace withSystemDbAccessContext with org-scoped contexts in
  AI agent services (aiAgentSdk, aiAgentSdkTools, scriptBuilder,
  streamingSessionManager). Capture canonical orgId on
  ActiveSession. (Finding #5)
- Fix 13 runOutsideDbContext sites in commandQueue and backup
  restore/vmrestore that would have silently failed once BYPASSRLS
  is removed. (Finding #7)

audit_logs NULL org_id (Finding #6) is intentional and unchanged —
system-level events are only visible to system scope by design.

Verified: 3114/3120 API tests pass; same 6 pre-existing failures in
enrollmentKeys_installer.test.ts on main. Manual DB check: breeze_app
sees 0 devices without scope, 6 with system scope — RLS now enforced.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Apr 11, 2026
…e fixes

Addresses review findings on PR #389 (heartbeat starvation instrumentation):

CRITICAL #1 -- timedRWMutex now instruments write-lock HOLD time in addition
to wait time. Long holds under contention (e.g. handleConnection holding
b.mu.Lock across a 15s SendCommandAndWait) were invisible to the existing
wait-only instrumentation, defeating the diagnostic goal for the exact
bug class it was designed to surface. Implementation tracks acquiredAt
inside Lock()/Unlock() -- safe because exactly one writer holds the lock
at a time. Read-lock hold time is NOT instrumented: a single field is racy
with multiple concurrent RLock holders, and alternatives (gid maps,
tokens, atomic pointers) either race or uglify the API. RLock holders in
this broker never perform long-blocking work, so instrumenting Lock holds
alone captures the dangerous class of bug. slowLockThreshold extracted to
a package var (1s default) so tests can shorten it.

CRITICAL #2 -- snapshotSessions now copies the live map inside the locked
fallback path instead of returning it and iterating after the deferred
RUnlock fires. A one-time WARN log is emitted via atomic.Bool if the
fallback ever fires in production (tests construct Broker{} directly).

IMPORTANT #3 -- sendHeartbeatWithWatchdog uses `defer close(done)` so a
panic in sendHeartbeat cancels the watchdog instead of letting it dump a
misleading "exceeded 15s" warning. sync.Once removed (the select fires at
most once). heartbeatWatchdogTimeout extracted to a package var and a
sendHeartbeatFn field added to *Heartbeat so tests can inject fast/slow
implementations and a short timeout without waiting 15 real seconds.

IMPORTANT #4 -- Session.GetCapabilities() added. It takes s.mu.Lock() and
returns a copy. All snapshot-path readers (FindCapableSession,
preferredDesktopSessionFromSnap, preferredDesktopSessionLocked,
reapIdleSessions) now go through it instead of reading s.Capabilities
directly. Before PR #389, b.mu.RLock() accidentally serialised those reads
with SetCapabilities writers; with the snapshot refactor the accidental
protection is gone and direct reads race under -race. The log site inside
TypeCapabilities handling now reads from the locally-held sanitized copy
instead of a post-Set read of s.Capabilities.

IMPORTANT #5 -- TCCStatus now loads b.snap directly and only falls back
to snapshotSessions when snap is nil. Eliminates the ad-hoc partial
sessionSnapshot construction (byIdentity was nil) that would silently
break any future reader touching snap.byIdentity. publishSnapshotLocked
is now the sole construction site for sessionSnapshot outside the fallback.

IMPORTANT #6 -- BenchmarkFindCapableSessionUnderConcurrentConnections now
asserts FindCapableSession returns non-nil. The benchmark pre-registers
capture-capable sessions in WinSessionID "1" and write-storm goroutines
only churn storm-* sessions, so nil indicates a regression.

Suggestions applied: noCopy sentinel on timedRWMutex for go vet,
sync/atomic merged into the sync import group, callerName closure-suffix
stripping fixed to handle both "-" and "." separators,
CloseSessionsByDesktopContext stale-snapshot latency window documented.

All tests pass with -race. No public API changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Apr 11, 2026
…t path

IMPORTANT #7 from PR #389 review.

Adds three sets of tests exercising the new correctness fixes:

1. TestSnapshotReadAfterWrite -- uses New() so snap is initialised, registers
   a session through the same locked-write + publishSnapshotLocked pattern
   handleConnection uses, then asserts SessionCount, AllSessions, and
   FindCapableSession all observe the session via the atomic snapshot path
   (not the nil-fallback). Verifies b.snap is non-nil and
   snapFallbackWarned stays false. Also verifies removal is reflected.

2. TestTimedRWMutex{WarnsOnLongHold,NoWarnOnFastHold,WarnsOnSlowAcquire} --
   uses withShortLockThreshold(25ms) via the new setSlowLockThreshold
   atomic helper so the hold-time and wait-time branches can be exercised
   in ~100ms rather than 1s+. Asserts the exact warning message and
   held_ms / waited_ms / caller structured fields.

3. TestSendHeartbeatWatchdog{FiresWhenBlocked,DoesNotFireOnFastPath,
   CancelsOnPanic} -- injects a fast/slow/panicking sendHeartbeatFn and a
   tiny 50-100ms watchdog timeout via setHeartbeatWatchdogTimeout. Asserts
   the expected dump/no-dump behaviour. CancelsOnPanic specifically proves
   the `defer close(done)` fix from CRITICAL #3 works -- a panic unwinds
   through the wrapper, close fires, and the watchdog select takes the
   cancelled path instead of emitting a misleading "exceeded" warning.

Both test files add a local syncBuffer wrapping bytes.Buffer because
multiple goroutines (the acquiring goroutine + the holding goroutine, or
the caller + the watchdog goroutine) can emit log lines concurrently, and
bytes.Buffer is not safe for concurrent Write. captureLogs and
watchdogTestHarness use t.Cleanup for deterministic teardown.

Passes with go test -race -count=10 on sessionbroker + heartbeat.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Apr 11, 2026
Database-layer hardening following the RLS security review. The app
previously connected as POSTGRES_USER (SUPERUSER with BYPASSRLS),
making every RLS policy a no-op. Closes that gap and fixes every
hole that was hiding behind it.

- Add unprivileged breeze_app role (NOSUPERUSER, NOBYPASSRLS) via
  new ensureAppRole bootstrap in autoMigrate. API connects via
  DATABASE_URL_APP with a startup warning if the connected user
  still has BYPASSRLS. (Finding #1)
- Rewrite broken backup/DR/C2C/vault policies that referenced an
  unset session variable app.current_org_id. 15 tables now use
  breeze_has_org_access(org_id) + FORCE ROW LEVEL SECURITY.
  (Finding #2)
- Add org_id column + RLS policies to device_metrics, backfilled
  from parent devices table. (Finding #3)
- Restrict cis_check_catalog write policies to system scope.
  (Finding #4)
- Replace withSystemDbAccessContext with org-scoped contexts in
  AI agent services (aiAgentSdk, aiAgentSdkTools, scriptBuilder,
  streamingSessionManager). Capture canonical orgId on
  ActiveSession. (Finding #5)
- Fix 13 runOutsideDbContext sites in commandQueue and backup
  restore/vmrestore that would have silently failed once BYPASSRLS
  is removed. (Finding #7)

audit_logs NULL org_id (Finding #6) is intentional and unchanged —
system-level events are only visible to system scope by design.

Verified: 3114/3120 API tests pass; same 6 pre-existing failures in
enrollmentKeys_installer.test.ts on main. Manual DB check: breeze_app
sees 0 devices without scope, 6 with system scope — RLS now enforced.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Apr 11, 2026
Adds a Status section at the top of the plan that maps every bucket
to its shipping commit, plus inline ✅ DONE markers on each bucket
header. The original plan body is preserved for historical context
and the execution-order section is annotated with notes on how the
actual rollout diverged from the plan (mostly: shipped in one tight
session rather than across multiple separate PRs, and Phase 1-6
clustering was by write-hotness instead of pure subsystem).

Remaining next-session work captured in the new "Still open"
subsection:
1. policy_compliance DROP migration (dead code cleanup)
2. mobile_sessions — deferred until mobile auth backend is built
3. device_commands — intentionally system-scoped, no action
4. Production rollout plan — row-count checks + batched backfill
   for the largest tables before applying Phase 1-4 migrations
5. Flip contract test from continue-on-error to blocking in CI
6. Fix pre-existing rls.integration.test.ts failing case

Open questions updated: #1, #3, #4 marked resolved/moot.
New #5 asks whether policy_compliance can be dropped.

No code changes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Apr 15, 2026
…435)

## Summary
- `POST /api/v1/agents/:id/commands/:commandId/result` validated `:id`
as `z.string().uuid()`, but agents send their 64-char SHA-256 hashed ID
(e.g. `ab3c20eddb47…`), not a UUID. Every HTTP-heartbeat-mode agent's
command result was silently 400'd.
- Fix: validate `:id` as `z.string().min(1)` to match `heartbeat.ts` and
other `agents/*` routes.

## Impact
Every HTTP-heartbeat-mode agent's command results stayed in
`status=sent` forever, with `stdout`/`stderr`/`exitCode`/`error` all
null. WS-connected agents were unaffected because they go through
`agentWs.ts:516`, a parallel result path.

Undiagnosed for ~1 month because (a) most prod agents are WS-connected,
(b) the agent logs the 400 at `log.Error` without capturing the response
body, so it looks like a transient network blip in the diagnostic log
stream, (c) nobody hits this unless they're specifically running
command-feedback tests against a heartbeat agent.

Introduced in commit `6f612977` (PR #220) on 2026-03-13.

## How I found it
Tried to query BCD state via a `bcdedit` script during an end-to-end
test of the `reboot_safe_mode` feature against a Windows Server 2022 VM
enrolled to local docker. Result POST kept 400'ing. Switched local
compose to `docker-compose.override.yml.dev`, added a zValidator error
hook on the JSON body, expected to see the schema failure. The hook
never fired, which pointed at the *previous* validator in the chain —
`zValidator('param', commandResultParamSchema)`. The agent URL path had
`ab3c20eddb470acffd33bbe00f25e0348e89298ab80cece542bb1fbf921e5776`
(64-char hex, SHA-256 of enrollment token per `devices.agent_id`), which
isn't a UUID.

## Verification
Before fix — probe script `35510331` → POST /result returns 400, command
stays `sent`:
```
POST /api/v1/agents/ab3c20ed.../commands/35510331.../result  400
```

After fix — re-ran the same probe (`a4b22f23`) against hot-reloaded dev
API:
```
POST /api/v1/agents/ab3c20ed.../commands/a4b22f23.../result  200
```
`device_commands.status` moved to `completed`, `result.stdout` populated
with the agent's script output.

## Test plan
- [x] Local dev: reproduce the 400 with a heartbeat-mode agent, apply
the fix, confirm 200 + populated result
- [ ] Regression: add an integration test exercising the POST path with
a non-UUID agent ID
- [ ] Consider a contract test that enumerates all `agents/:id/*` routes
and verifies they accept a 64-char hex `id`, to prevent this class of
regression

## Follow-ups (out of scope for this PR)
- **Observability gap (bug #3 in the linked test log)**:
`agent/cmd/breeze-agent/main.go` initializes the log shipper *after*
several startup events including the Safe Mode BCD auto-clear, mTLS cert
renewal, and config-permission fixes. Anything logged in that window is
only in the local agent log file, never shipped. This made me chase a
nonexistent bug in `IsSafeMode()` for an hour — the auto-clear was
running fine; its logs just weren't reaching the server. Should move
`logging.InitShipper` to run right after `initLogging(cfg)`.
- `POST /devices/:id/commands` with `{type:"script",
payload:{scriptId}}` silently runs with empty content and returns
`"script content is empty"` from the agent — the route should either
reject with a clear error directing callers to `/scripts/:id/execute`,
or hydrate `content` server-side.

See `docs/testing/FEATURE_TEST_LOG.md` for the full test writeup.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Apr 22, 2026
…500)

## Summary

Closes 13 of 14 findings from the 2026-04-20 security review of
`feature/device-org-move-wip`. 12 discrete tasks + 2 correction commits,
each independently revertible.

Full plan:
`docs/superpowers/plans/2026-04-20-security-review-remediations.md`.

## Findings addressed

| # | Sev | Area | Fix |
|---|---|---|---|
| #1 | HIGH | uninstall scripts | Publish SHA256SUMS + surface
hash/verify cmd in `AddDeviceModal` |
| #2 | HIGH | public-download token in URL | One-time Redis handle via
new `POST /enrollment-keys/:id/download-handle`; back-compat `?token=`
path retained with deprecation warning |
| #3 | HIGH | enrollment-key write routes unrated | New `userRateLimit`
middleware (10/60s per user) on all 5 write routes (create / rotate /
delete / installer-link / bootstrap-token) |
| #4 | MEDIUM | 8h viewer JWT | Dropped to 2h; added `jti` claim;
Redis-backed revocation + per-tunnel-session revoke-on-close |
| #5 / #6 | MEDIUM | tunnels.ts `:id` / `siteId` unvalidated |
`zValidator('param'/'query', …)` on 7 routes |
| #7 | MEDIUM | install.bat template | `assertValidEnrollmentKey`
(64-hex regex) + quoted `ENROLLMENT_KEY` |
| #8 | MEDIUM | viewer HTTP allowlist | `isLocalhost()` replaces
`isPrivateHost()` — 127.0.0.1 / ::1 / localhost only |
| #9 | MEDIUM | VNC server reason → UI | `KNOWN_VNC_REASONS` allowlist +
`friendlyReason()` fallback (incl. ARD form) |
| #10 | MEDIUM | siteId ownership on enroll create | Query `sites` WHERE
`sites.orgId = orgId` before insert |
| #12 | MEDIUM | short-link claim race | Atomic UPDATE folds expiry +
max-usage into WHERE; claim-before-insert |
| #13 | LOW | helper install templates | `Exec=%q` on Linux;
`xml.EscapeText` on Darwin plist |
| #14 | LOW | session config perms | 0644 → 0600 on 3 write sites |

Finding #11 (encrypted `enrollment.json` inside installer zip) and other
low-severity items were intentionally deferred — documented in the
plan's "Out of scope" section.

## Test plan

- [x] API: `pnpm --filter @breeze/api test:run` — 3247 passed, 25
skipped
- [x] Viewer: `npx vitest run` in `apps/viewer` — 114 passed
- [x] Web: `npx vitest run` in `apps/web` — 279 passed
- [x] Agent helper: `cd agent && go test -race ./internal/helper/...` —
ok
- [ ] Manual smoke: trigger a macOS installer download from staging and
confirm the agent enrolls (exercises finding #2 + #7 + #10 end-to-end).
- [ ] Manual smoke: open a WebRTC session, hit DELETE `/tunnels/:id`,
confirm the viewer sees a 401 on its next poll (exercises finding #4).
- [ ] Visual review: uninstall-script SHA256 block renders correctly
below each download link.

## Notes for the reviewer

- Two commits (`7db3b1fb`, `845e2483`, `6be03849`) have muddled
boundaries because several subagents staged changes concurrently in the
same worktree. The tree state is correct; only the commit-per-task
narrative got blurred. Squash-merge will render this moot.
- `enrollmentKeys.ts` grew to 1333 lines. It's cohesive but approaching
hard-to-navigate; candidate for a follow-up split of
`publicEnrollmentRoutes` + `publicShortLinkRoutes` + `serveInstaller`
into a `enrollmentKeys.public.ts`.
- `/:id/download-handle` uses `===` on SHA-256 digests rather than
`timingSafeEqual`. Low risk given both sides are digests + the route is
auth-gated + rate-limited, but the convention elsewhere
(`apiKeyAuth.ts`) uses `timingSafeEqual`. Trailing TODO worth picking
up.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bdunncompany added a commit to bdunncompany/breeze that referenced this pull request May 14, 2026
…lver

Adds resolveRemoteAccessLaunch returning {launchUrl, providerId, scheme,
skipReason} so callers can distinguish expected-empty
(no_provider_configured), configuration error (missing_device_identifier,
empty_url_template, provider_disabled), and potential security event
(scheme_not_allowed) instead of a bare null.

buildRemoteAccessLaunchUrl is preserved as a thin wrapper for existing
callers and tests.

Addresses Todd's review item LanternOps#3 (silent fallback on launcher lookup
failure). The new shape lets the GET /devices/:id response surface the
reason so the UI can render appropriate copy.
bdunncompany added a commit to bdunncompany/breeze that referenced this pull request May 14, 2026
Moves remote-access launch URL issuance off the GET /devices/:id response
so the substituted password-bearing URL is never broadcast on every
device-detail fetch.

GET /devices/:id now returns hasRemoteAccessLauncher (bool) and
remoteAccessLaunchSkipReason (config_error | no_provider_configured |
provider_disabled | missing_device_identifier | empty_url_template |
scheme_not_allowed). The substituted URL is gone from this response.

POST /devices/:id/remote-access-launch:
- requires the same organization/partner/system scope GET has
- resolves the partner-configured provider, builds the substituted URL,
  and returns {launchUrl, scheme, providerId}
- emits device.remote_access_launch_url.issued audit on success with
  only deviceId, providerId, and scheme in details (NEVER the URL or
  password)
- returns 422 with code=scheme_not_allowed AND emits
  device.remote_access_launch_url.scheme_rejected (result=denied) AND
  routes to Sentry when a tampered template resolves to a disallowed
  scheme only after substitution
- registered before PATCH/DELETE /:id so Hono's
  match-in-registration-order resolves the path correctly

Includes tests for success/issuance audit, no-provider 404, scheme-
rejected 422/audit/Sentry, and verification that the substituted URL
and password never appear in the audit details JSON.

Addresses Todd's review items #1, #2, and LanternOps#3.
bdunncompany added a commit to bdunncompany/breeze that referenced this pull request May 14, 2026
…lver

Adds resolveRemoteAccessLaunch returning {launchUrl, providerId, scheme,
skipReason} so callers can distinguish expected-empty
(no_provider_configured), configuration error (missing_device_identifier,
empty_url_template, provider_disabled), and potential security event
(scheme_not_allowed) instead of a bare null.

buildRemoteAccessLaunchUrl is preserved as a thin wrapper for existing
callers and tests.

Addresses Todd's review item LanternOps#3 (silent fallback on launcher lookup
failure). The new shape lets the GET /devices/:id response surface the
reason so the UI can render appropriate copy.
bdunncompany added a commit to bdunncompany/breeze that referenced this pull request May 14, 2026
Moves remote-access launch URL issuance off the GET /devices/:id response
so the substituted password-bearing URL is never broadcast on every
device-detail fetch.

GET /devices/:id now returns hasRemoteAccessLauncher (bool) and
remoteAccessLaunchSkipReason (config_error | no_provider_configured |
provider_disabled | missing_device_identifier | empty_url_template |
scheme_not_allowed). The substituted URL is gone from this response.

POST /devices/:id/remote-access-launch:
- requires the same organization/partner/system scope GET has
- resolves the partner-configured provider, builds the substituted URL,
  and returns {launchUrl, scheme, providerId}
- emits device.remote_access_launch_url.issued audit on success with
  only deviceId, providerId, and scheme in details (NEVER the URL or
  password)
- returns 422 with code=scheme_not_allowed AND emits
  device.remote_access_launch_url.scheme_rejected (result=denied) AND
  routes to Sentry when a tampered template resolves to a disallowed
  scheme only after substitution
- registered before PATCH/DELETE /:id so Hono's
  match-in-registration-order resolves the path correctly

Includes tests for success/issuance audit, no-provider 404, scheme-
rejected 422/audit/Sentry, and verification that the substituted URL
and password never appear in the audit details JSON.

Addresses Todd's review items #1, #2, and LanternOps#3.
bdunncompany added a commit to bdunncompany/breeze that referenced this pull request May 14, 2026
…lver

Adds resolveRemoteAccessLaunch returning {launchUrl, providerId, scheme,
skipReason} so callers can distinguish expected-empty
(no_provider_configured), configuration error (missing_device_identifier,
empty_url_template, provider_disabled), and potential security event
(scheme_not_allowed) instead of a bare null.

buildRemoteAccessLaunchUrl is preserved as a thin wrapper for existing
callers and tests.

Addresses Todd's review item LanternOps#3 (silent fallback on launcher lookup
failure). The new shape lets the GET /devices/:id response surface the
reason so the UI can render appropriate copy.
bdunncompany added a commit to bdunncompany/breeze that referenced this pull request May 14, 2026
Moves remote-access launch URL issuance off the GET /devices/:id response
so the substituted password-bearing URL is never broadcast on every
device-detail fetch.

GET /devices/:id now returns hasRemoteAccessLauncher (bool) and
remoteAccessLaunchSkipReason (config_error | no_provider_configured |
provider_disabled | missing_device_identifier | empty_url_template |
scheme_not_allowed). The substituted URL is gone from this response.

POST /devices/:id/remote-access-launch:
- requires the same organization/partner/system scope GET has
- resolves the partner-configured provider, builds the substituted URL,
  and returns {launchUrl, scheme, providerId}
- emits device.remote_access_launch_url.issued audit on success with
  only deviceId, providerId, and scheme in details (NEVER the URL or
  password)
- returns 422 with code=scheme_not_allowed AND emits
  device.remote_access_launch_url.scheme_rejected (result=denied) AND
  routes to Sentry when a tampered template resolves to a disallowed
  scheme only after substitution
- registered before PATCH/DELETE /:id so Hono's
  match-in-registration-order resolves the path correctly

Includes tests for success/issuance audit, no-provider 404, scheme-
rejected 422/audit/Sentry, and verification that the substituted URL
and password never appear in the audit details JSON.

Addresses Todd's review items #1, #2, and LanternOps#3.
bdunncompany added a commit to bdunncompany/breeze that referenced this pull request May 14, 2026
…lver

Adds resolveRemoteAccessLaunch returning {launchUrl, providerId, scheme,
skipReason} so callers can distinguish expected-empty
(no_provider_configured), configuration error (missing_device_identifier,
empty_url_template, provider_disabled), and potential security event
(scheme_not_allowed) instead of a bare null.

buildRemoteAccessLaunchUrl is preserved as a thin wrapper for existing
callers and tests.

Addresses Todd's review item LanternOps#3 (silent fallback on launcher lookup
failure). The new shape lets the GET /devices/:id response surface the
reason so the UI can render appropriate copy.
bdunncompany added a commit to bdunncompany/breeze that referenced this pull request May 14, 2026
Moves remote-access launch URL issuance off the GET /devices/:id response
so the substituted password-bearing URL is never broadcast on every
device-detail fetch.

GET /devices/:id now returns hasRemoteAccessLauncher (bool) and
remoteAccessLaunchSkipReason (config_error | no_provider_configured |
provider_disabled | missing_device_identifier | empty_url_template |
scheme_not_allowed). The substituted URL is gone from this response.

POST /devices/:id/remote-access-launch:
- requires the same organization/partner/system scope GET has
- resolves the partner-configured provider, builds the substituted URL,
  and returns {launchUrl, scheme, providerId}
- emits device.remote_access_launch_url.issued audit on success with
  only deviceId, providerId, and scheme in details (NEVER the URL or
  password)
- returns 422 with code=scheme_not_allowed AND emits
  device.remote_access_launch_url.scheme_rejected (result=denied) AND
  routes to Sentry when a tampered template resolves to a disallowed
  scheme only after substitution
- registered before PATCH/DELETE /:id so Hono's
  match-in-registration-order resolves the path correctly

Includes tests for success/issuance audit, no-provider 404, scheme-
rejected 422/audit/Sentry, and verification that the substituted URL
and password never appear in the audit details JSON.

Addresses Todd's review items #1, #2, and LanternOps#3.
ToddHebebrand added a commit that referenced this pull request May 16, 2026
…apper, consent race, schema drift (#741)

Stacked on `feat/mobile-approval-mode` (base = PR #696's branch, same
pattern as #698). Branched off `ecc5350d` so #698's fixes are preserved.
Fixes the four Critical/scoped items from the #696 review that made the
feature **non-functional in production** despite green (mocked) unit
tests.

## Fixes

- **#1 — account-deletion admin queue returned empty.** Two layered
defects: (a) the `account_deletion_requests` Shape-6 RLS policy had no
system-scope OR branch, and (b) `accountDeletion.ts`'s
`runWithSystemDbAccess` lacked `runOutsideDbContext`, so inside an admin
request it inherited the admin's own scope. Both fixed (migration +
wrapper now mirrors `lifecycle.ts`'s `asSystem`).
- **#2 — expiry reaper never expired anything.** `approval_requests`
Shape-6 policy had no system-scope OR branch → under SYSTEM scope
`breeze_current_user_id()` is NULL → FORCE RLS hid every row from
`breeze_app`. Same migration adds the branch (the reaper's own wrapper
was already correct — BullMQ worker, no ambient context).
- **#3 — mobile biometric consent race.** A focus swap during the OS
biometric prompt could rebind consent to a different approval. Request
id is now captured at press time, threaded `ApprovalButtons →
ApprovalScreen`, and gated by a pure `decisionTarget()` — aborts with a
re-review prompt instead of deciding the wrong action.
- **#7 — schema drift.** `oauth.ts` now declares the
`oauth_client_blocks_org_client_uniq UNIQUE(org_id, client_id)` index
the migration creates (the lifecycle upsert relies on it). *Note: not a
CI blocker — `ci.yml`'s drift step is `continue-on-error`.*

The migration mirrors the `oauth_authorization_codes`/`sessions` `OR
breeze_current_scope() = 'system'` pattern. Request-scoped callers never
have `scope='system'`, so the `user_id` predicate still governs — no
cross-user exposure (the unchanged cross-user forge tests prove this).

## Verification (test-driven, RED→GREEN)

- New `approval-system-scope.integration.test.ts` — #1 + #2 against a
real DB (watched fail with the policy reverted).
- New `decisionTarget.test.ts` — pure consent-binding guard, 4/4.
- RLS contract + cross-user forge **13/13**;
accountDeletion/admin/autoMigrate units **38/38**; full mobile suite 137
pass; no new type errors.
- Stale "no system-scope OR branch" comments in `rls-coverage` corrected
(comment rot this migration introduced).

## Scope

Criticals + #7 only. The review's Important items (report-suspicious
non-atomic flip, Expo-push silent auto-deny, receipts, `oauthRevocation`
coverage, cross-user denial tests, `azp` block bypass,
biometric-on-deny) are intentionally **out of scope** — for the #696
split.

Refs #696, #698.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request May 31, 2026
…rd controls (#1020)

## Remote-desktop session security hardening

Implements the 8 confirmed findings from the remote-desktop (WebRTC)
pipeline security review. Root cause: authorization was established only
at connect/offer time and was never re-enforced against, nor revocable
from, a live session.

**Findings fixed**
- **#1** Policy re-checked at both WebRTC offer endpoints (JWT `/offer`
+ viewer-token path).
- **#2** Server sends `stop_desktop` on end/suspend; agent gains idle +
max-duration watchdog timers (enforces the previously-dead
`idleTimeoutMinutes`/`maxSessionDurationHours`).
- **#3** Live remote sessions terminated on user deactivate/suspend and
partner-abuse-suspend (new `remoteSessionTeardown` service).
- **#4** Viewer-revocation rechecked in the desktop-WS ping loop → live
legacy socket closes within one interval.
- **#5** No session resurrection: reject `disconnected`/`failed` in
`validateViewerSessionAccess`, drop `endedAt:null` resurrection from
both offer sinks, revoke on the passive disconnect paths.
- **#6** Single-sourced viewer-token TTL (advertised == real 2h).
- **#7** Per-direction clipboard policy capability; agent-enforced
(channel/Watch/Receive gated). Host→viewer defaults **off** on hosted
(`IS_HOSTED`).
- **#8** Clipboard + filedrop transfer audit (direction/size/filename).

**Tests added:** `viewerTokenTtl.test.ts`, `clipboard/gate_test.go`,
`handlers_desktop_policy_test.go`; updated existing mocks/test calls for
the new signatures.

**Local verification:** API `tsc --noEmit` = 0 errors; targeted vitest
118 passed + `remote.test.ts` 15/15; agent `go test`
clipboard/filedrop/heartbeat green (clipboard/heartbeat also under
`-race`).

**Pre-existing failures (not from this PR):** the full local `pnpm test`
shows 12 failures across 10 files in unrelated subsystems
(`oauthInteraction`, `partner_multi_org_orgid`/discovery-scan,
`encryptedColumnRegistry`, …) — disjoint from this diff (which only
touches remote-desktop), and all 5200 other tests pass. Relying on CI
for the authoritative full-suite gate.

**Decisions to confirm**
- Clipboard host→viewer defaults **off on hosted** / on for self-hosted
(preserve behavior); admin-overridable via policy.
- #5 means a viewer that drops (tab crash / network blip) must create a
**fresh** session to reconnect — flag for the web viewer UI.

**Follow-ups:** route #8 audit into the central tamper-evident
`audit_logs` (currently agent diagnostic logs); add a web UI toggle for
the clipboard capability.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Jun 9, 2026
## Summary

Adds four grounded implementation plans for the PAM (Privileged Access
Management) work tracked on [project board
#3](https://github.com/orgs/LanternOps/projects/3), covering the
complete Windows end-to-end elevation flow. These came out of a triage
pass that mapped the merged PAM work (#905/#906/#959/#960/#907) against
the board and found the remaining tracks had only design-level coverage
in Discussion #858 — no per-track implementation plans.

Each plan is grounded in real `file:line` patterns (via code-recon of
the actual agent/API/web source) and includes a self-review checklist.

## Plans

| Plan | Issue | Effort | Notable finding |
|---|---|---|---|
| `pam-backend-control-plane` | #1163 | L | The admin REST API +
decisioning wiring + lifecycle jobs + `elevation.*` events that gate the
UI. Surfaced that the Rules tab needs a **new `pam_rules` table**
(`pamBridge` consults `software_policies`, which is distinct). |
| `pam-web-admin-ui` | #1159 | L | `/pam` 4-tab page mirroring the DNS
Security scaffold (#847). Adds **role-gating + `data-testid` coverage**
the reference page lacks. Blocked by #1163. |
| `pam-dormant-admin-account` | #1150 | M | `~breeze_elev` lifecycle.
Establishes the **agent as credential authority** — a server can't set a
local account's password — deprecating #960's server-sent creds to a
`{elevationRequestId, timeoutMs}` "go" signal. |
| `pam-dialog-user-helper` | #1152 | M | Native approval dialog in
`breeze-user-helper.exe` driven over IPC from the ETW subscriber (#959).
Uses the real ETW IDs (4100/4101/4102, not the issue body's
15006/15007); recommends a `MessageBoxW` MVP. |

## Build order

**#1163#1159** (Phase 1, no Windows-broker changes — techs approve
from the web) in parallel with **#1150 + #1152** (agent end-to-end:
detect → dialog → promote → actuate → demote). Together this is the
Windows MVP from Discussion #858 §7 Phases 1-2.

## Scope

Docs only — four new files under `docs/superpowers/plans/`, 386
insertions, no code changes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ToddHebebrand pushed a commit that referenced this pull request Jun 17, 2026
, #5)

Closes the remaining #1368 catalog gaps.

Gap #3 — ticket-part → catalog link:
- ticketPartSchema gains optional catalogItemId (uuid|null); addTicketPart
  / updateTicketPart persist it to the existing ticket_parts.catalog_item_id
  column.
- TicketPartsCard: a catalog typeahead in the Add-part form (reusing
  CatalogItemPicker, bundles excluded). Picking an item prefills
  description / unit price / cost and links catalogItemId; Unlink detaches
  it while keeping the fields free-text. Catalog and ticket parts share the
  partner/system scope gate, so the picker is always reachable here.

Gap #5 — archive confirm:
- CatalogItemsTab now routes Archive through a ConfirmDialog ("hidden from
  active pickers… restore from Archived") instead of acting on one click.
  The archived view + restore already existed.

Tests: parts route (catalogItemId passthrough + non-UUID 400), TicketPartsCard
(catalog prefill+link, unlink), CatalogItemsTab (archive now gated by confirm).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Jun 17, 2026
…ket-part link, archive confirm (#1368) (#1467)

## Summary

Completes #1368. Wires the remaining backend-ready surfaces into the UI.
(Two of the five gaps — bundle component picker #2 and live
margin/derived pricing #4 — already shipped after the issue was filed;
this closes the rest.)

### Gap #1 — per-org price override
- catalog API lib: `setOrgPriceOverride` / `removeOrgPriceOverride`
(`unitPrice` as a number to match `orgPriceOverrideSchema`).
- `CatalogItemEditorDrawer`: a **Per-organization pricing** section for
an existing non-bundle item (partner-scope writers) — lists overrides,
set a customer-specific price via an org picker + price, remove one;
each applied immediately via `runAction`. Hidden for bundles (price
derives from components).

### Gap #3 — ticket-part → catalog link
- `ticketPartSchema` gains optional `catalogItemId` (uuid|null);
`addTicketPart`/`updateTicketPart` persist it to the existing
`ticket_parts.catalog_item_id` column.
- `TicketPartsCard`: a catalog typeahead in the Add-part form (reusing
`CatalogItemPicker`, bundles excluded). Picking prefills description /
unit price / cost and links `catalogItemId`; **Unlink** detaches it
while keeping the fields free-text.

### Gap #5 — archive confirm
- `CatalogItemsTab` routes Archive through a `ConfirmDialog` instead of
a single click. (The archived view + restore already existed.)

## Tests
- API: `parts.test.ts` (catalogItemId passthrough + non-UUID → 400);
`CatalogItemEditorDrawer.test.tsx` (load/set/remove overrides, hidden
for new/bundle).
- Web: `TicketPartsCard.test.tsx` (catalog prefill + link, unlink);
`CatalogItemsTab.test.tsx` (archive now gated by confirm); completed
`CatalogItemsTab` auth mocks for `orgStore.registerOrgIdProvider`.

**Verification:** web settings+tickets 410 passed (39 files) +
no-silent-mutations 55; API parts 16 + timeEntryService 53; shared
validators 20; `tsc --noEmit` (shared/api/web) clean on touched files.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Todd Hebebrand <todd@lanternops.io>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pixlflip referenced this pull request in PixlFlip-Enterprises/remuda-rmm Jun 20, 2026
…ope)

PR #4: global env-configured PixlFlip OIDC consumer (not a per-org sso_providers
row). New /api/v1/sso/pixlflip/login + /callback reuse services/sso.ts helpers;
verify id_token via PixlFlip JWKS, read breeze_* claims (PR #3), provision into
the breeze_org_id org as organization_users, mint org-scope tokens, hand off via
the existing #ssoCode -> /sso/exchange grant. New pixlflip_sso_sessions table
(no RLS, no provider FK) + migration; new PIXLFLIP_SSO_* env (disabled by
default, fail-closed). Partner/system scope -> PR #5; user_sso_identities
linking -> PR #6; web button + #ssoCode handoff -> PR #4b. Could not run pnpm
typecheck/tests in sandbox (no node_modules) — rely on CI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Jun 21, 2026
…1709)

**Root cause:** `run_as_user` script/exec routing is bound to the active
console session (the #1009 fix #3, shipped in PR #1242) — but the
binding drops delivery *silently*. `Broker.PreferredRunAsUserSession()`
returns `nil` both when there is genuinely no user-role helper connected
AND when a user helper exists only in a *non-console* session. Both live
callers — `handlers_script.go:resolveRunAsSession` and
`heartbeat.go:makeUserExecFunc` (winget user-context exec) — treat that
`nil` as "no helper" and downgrade to local SYSTEM execution / error
with no signal.

On an unusual multi-session / RDS / terminal-server host where the
operator's helper genuinely lives off the active console session (e.g.
the physical console sits at the lock screen while the operator works
over RDP), this manifests as an **unobservable dropped delivery** —
exactly the breaking-change risk the security tracker flagged for the
still-open `run_as_user` slice of #1009.

This is the still-open slice of #1009 (the closed issue's fix #1/#2/#4 —
the `roleIdentityRejection` console binding + helper-token gate + tests
— already shipped). There is **no server/payload target to bind to**:
`runAs` is an enum (`system`/`user`/`elevated`) carrying no target
session id, SID, or username, so `runAs=user` means "the interactive
console user" and the active console session **is** the binding target.
A target-scoped selector would have no input to bind on, so the residual
gap is observability, not a new binding axis.

**Fix:** In `preferredRunAsUserSessionForOS` (Windows path), count
run_as_user helpers excluded *solely* for being off-console. When that
exclusion is the only reason nothing was selected (`best == nil &&
excludedNonConsole > 0`), emit a clear `WARN` — `"run_as_user delivery
suppressed: no helper in the active console session"` with
`consoleWinSession` and `excludedNonConsole` fields — so the dropped
delivery is diagnosable. When no run_as_user helper is connected at all,
the selector stays quiet (no per-poll noise). The fail-safe behaviour
(never deliver a run_as_user script to the wrong principal) is
**unchanged** — only its observability is added. `log.Warn` is emitted
outside the broker read-lock.

**Tests:** 3 table-driven cases in
`agent/internal/sessionbroker/console_session_gate_test.go`:
- off-console-only helper ⇒ `nil` returned **and** a suppression `WARN`
with diagnostic fields fires (the RDS-host drop is now observable, not a
silent wrong-session/SYSTEM downgrade),
- no user helper connected ⇒ `nil`, **no** WARN (not noise),
- console helper present ⇒ selected, **no** WARN.

RED proven against the pre-fix silent selector (the suppression-WARN
assertion failed; the no-WARN assertions passed because the old code was
silent everywhere). Full `go test -race ./internal/sessionbroker/...`
and `./internal/heartbeat/...` green; `go vet` and cross-compile
(`GOOS=windows`) clean.

**Risk / release-notes:** behaviour-possibly-affecting only in the sense
that it adds a diagnostic `WARN` on the rare RDS/multi-session host
where a run_as_user script can't reach a console helper — no
routing/selection change, no new drops, Unix path untouched. Worth a
release-notes line so operators know to look for the new "run_as_user
delivery suppressed" warning when a `runAs=user` script silently runs as
SYSTEM on a terminal-server host.

Refs #1009.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Todd Hebebrand <todd@lanternops.io>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Jun 24, 2026
#1802) (#1817)

Implements two of the deferred follow-ups from #1802 (the #1800 watchdog
auto-update tracking issue).

**Scope:** addresses **item #2** (telemetry lag) and **item #4** (local
watchdog registration). Item #3 (download.ts S3 error masking) already
shipped in #1807. Item #1 is a **manual Windows canary / release gate**
that this PR cannot satisfy in code — so this PR **does not close the
issue**; it stays open and assigned for the canary.

### Item #2 — main agent reports installed watchdog version in its
normal heartbeat
`devices.watchdog_version` was written **only** from watchdog FAILOVER
heartbeats (`heartbeat.ts`). A recovered, healthy watchdog returns to
monitoring and stops failover-heartbeating, so the dashboard kept
showing the OLD version and the server re-sent `watchdogUpgradeTo`
indefinitely (the agent deduped it in-memory).

- **Agent:** `HeartbeatPayload.watchdogVersion` is now populated by
`installedWatchdogVersion()`, which prefers the version swapped in this
run, else a **cached** read of the on-disk binary via `breeze-watchdog
status` (exec'd at most once per process run, 5s timeout, best-effort —
never fails or stalls the heartbeat). Added per-OS
`watchdogBinaryPath()` and a `watchdogVersionReader` test seam (mirrors
the existing `watchdogInstaller` seam).
- **Server:** heartbeat schema gains `watchdogVersion`; the main-agent
branch **persists** it to the device row, and the `watchdogUpgradeTo`
computation now **prefers the reported version** over the stored column
(mirroring the helper path) so a successful swap stops the re-send on
the very next heartbeat. Old agents that omit the field keep the
existing stored-column behavior.

### Item #4 — `BINARY_SOURCE=local` watchdog registration
`syncFromGitHub` registered the `watchdog` component but the
local-binaries scan path did not, so self-hosters on
`BINARY_SOURCE=local` never got watchdog auto-update. The local path now
scans + registers `breeze-watchdog-*` siblings (served by the existing
`/download/watchdog` route). Extracted `registerLocalBinaries()` and
**scoped its `isLatest` demote per-component** — the inline agent path
demoted ALL components for a platform/arch, a latent clobber once a 2nd
component is registered locally (the GitHub `upsertVersion` path already
scoped this way).

### Tests
- Agent: `parseWatchdogStatusVersion` table tests +
reader-priority/caching unit tests (`watchdog_version_test.go`).
- API: heartbeat `watchdogVersion` persistence, old-agent omission,
reported-version re-send suppression, and genuine-behind-still-upgrades
tests.
- binarySync: local watchdog-registration present/absent tests.

Verification: `go test -race ./internal/heartbeat/...
./cmd/breeze-watchdog/...` green; affected API vitest files green (71
passed); `tsc --noEmit` clean; Windows + host Go builds clean.

Addresses #1802 (items 2 and 4).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Todd Hebebrand <todd@lanternops.io>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Jul 7, 2026
…emand recompute throttle, #1105 ingest context (#2266)

## Problem

Production US (v0.91.0) showed intermittent high API latency — `GET
/devices/:id/events` averaging **22.4s** (max 29s) while the same query
`EXPLAIN ANALYZE`s in 0.3ms. Root cause was **event-loop contention**,
not slow SQL: the in-process device-reliability scoring worker read each
device's **entire 90-day history via `SELECT *`** (including the
~2KB/row `rawMetrics` JSONB) and bucketed it in JS, on every ingest
POST, at worker concurrency 5. One runaway device posting every ~18s had
accumulated **13,774 rows** and pegged a core on each post. Reliability
ingests also sat **idle-in-transaction 5–20s** (the #1105
held-transaction pattern), flooding 1,215 warnings in 2 hours.

Full design:
`docs/superpowers/specs/2026-07-06-reliability-scoring-event-loop-hardening-design.md`.

## Changes

1. **Projected history read** — `getHistoryForDevice` selects only the 7
columns the scorer consumes and drops the `rawMetrics` JSONB from every
90-day read. Enforced by a narrowed `ScoringHistoryRow` type + a
`satisfies Record<keyof ScoringHistoryRow, AnyPgColumn>` guard on the
projection so the SQL can't silently drift back to fetching the blob.
2. **On-demand recompute throttle** —
`ON_DEMAND_RELIABILITY_DEDUPE_WINDOW_MS` 30s → 10 min. A device
recomputes at most once per fixed 10-min bucket regardless of post rate
(the jobId slot keys on this window). This is what bounds the runaway
device.
3. **Worker concurrency** 5 → 2 — caps simultaneous heavy computes.
4. **#1105 ingest context** — `POST /agents/:id/reliability` joins
`SELF_MANAGED_DB_CONTEXT_ACTIONS`; a short org-scoped
`withDbAccessContext` wraps only the device lookup + history insert,
while the BullMQ enqueue and audit write run **outside** any open
transaction (no pooled connection pinned idle-in-transaction across
Redis/non-DB work).

> **Note on the O(days) reduction:** the design floated pushing per-day
aggregation into SQL. It was **intentionally deferred** — the #1904
global event-dedup needs per-event keys across the whole window, which a
plain `GROUP BY` can't express without changing persisted counts. JS
bucketing stays row-based; the runaway load is instead bounded by
changes #2 + #3. Documented inline and in the design doc.

## Hardening from PR review (multi-agent pass)

**Critical**
- The Redis-outage **inline fallback** originally ran under an **org**
context — but `computeAndPersistDeviceReliability`'s ml-feature-flag
gate reads `organizations INNER JOIN partners`, invisible under
org-scope RLS, so the compute **silently no-op'd** (persisted nothing).
Now runs `runOutsideDbContext(() => withSystemDbAccessContext(...))`,
mirroring the worker. A new integration test proves org scope no-ops
while system scope persists, against **real RLS-enforced Postgres**.
- The fallback was un-`try/catch`'d: after the history row was
committed, a throw flipped the request to **500 → agent retries →
duplicate inserts** during an outage. Now best-effort (log +
`captureException`, response stays 200), matching the enqueue path.

**Important**
- `captureException` added to the history-insert catch (was stdout-only,
so a persistent RLS misconfig would be Sentry-blind).
- Fail fast with **401** when an agent token carries no `orgId`
(previously a silent 404 masquerade from a vacuous org RLS context).
- `LookupResult` collapsed to two arms + exhaustive `never`-check
consumer so a new failure reason can't silently become a 500.

**Comment accuracy** — removed non-unique `(Task 1)` and dead
`task-2-report.md` references; corrected "each compute is cheap"
(projection cut I/O, not the O(rows) CPU) and the fixed-bucket dedupe
wording.

## Testing

- **Unit** (route/worker/agentAuth/scoring): fallback runs under
**system** scope; fallback-throw stays 200; `insert_failed → 500` skips
enqueue/audit; missing-`orgId → 401`; `#1105` depth-0 for enqueue +
audit; dedupe-window jobId reuse; concurrency pinned to 2. ✅ 190 pass
- **Integration** (real Postgres): golden-value score from a projected
read of a high-frequency device (50 posts/day, populated 2KB blobs);
org-scoped compute no-ops (regression lock). ✅ 2 pass
- `tsc --noEmit` clean.

## Rollout

Normal release — no migration, no new env vars, no infra/compose
changes. Relieves prod even while the runaway agent is still
misbehaving.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Todd Hebebrand <todd@lanternops.io>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Jul 7, 2026
…#2274)

Closes #2273

## Problem

On macOS the user-session helper (`com.breeze.agent-user`, uid 501) is
repeatedly evicted by the daemon's session broker with `keepalive pong
timeout, closing stranded session` (`ageMs ≈ 60000`). A healthy helper
is a prerequisite for `computeDesktopAccess` to return `mode:
"user_session"`, so `desktopAccess` stays `unavailable` and the web
"Connect Desktop" button renders greyed out ("Desktop Unavailable") even
though the device is online and in use.

## Root cause

`ipc.Conn.Send()` took the write mutex `c.mu` and held it across two
blocking `conn.Write()` calls **with no write deadline**
(`SetWriteDeadline` existed but was never used). One stalled write held
`c.mu` forever, so the helper read loop's keepalive `TypePong` reply
(`userhelper/client.go`) could never send → broker evicts at ~60s. The
likely trigger is macOS App Nap suspending/throttling the helper (a
background LaunchAgent with zero activity assertions), which stalls the
socket; the `broken pipe` on resume confirms the socket genuinely
wedges.

On macOS, desktop frames stream over WebRTC/pion, not this IPC socket,
so IPC carries only small control messages — the stall is OS suspension,
not write load.

## Fixes

**1. Write deadline in `ipc.Conn.Send()` (the load-bearing fix).**
Set `SetWriteDeadline(now + writeTimeout)` before the header/payload
writes and clear it (`SetWriteDeadline(time.Time{})`) afterwards, all
under the existing `c.mu`. This converts a permanent mutex wedge into a
recoverable error — the caller tears down and reconnects instead of the
helper going silent until the broker evicts it. Applies to every
`Send()` caller (daemon, watchdog, user helper, backup helper).

- **Value chosen: 30s.** A legitimate `MaxMessageSize` (16 MiB) payload
over a local unix socket completes in well under a second, so 30s never
trips a healthy transfer, yet it's bounded so a genuinely wedged socket
surfaces as an error. Defined as a package `var writeTimeout`
(overridable in tests).
- Clearing the deadline afterwards (writes are serialized by `c.mu`, so
no race) prevents one call's deadline from leaking onto the next `Send`.

**2. App Nap guard for the macOS helper.**
At helper startup (`Client.Run()`) the helper takes a process-lifetime
`NSProcessInfo.beginActivityWithOptions:reason:` activity assertion so
macOS doesn't suspend/throttle the IPC + keepalive goroutines.
Implemented in CGO/Objective-C behind `//go:build darwin && cgo`
(`appnap_darwin.go`), with a no-op stub for other platforms and the
nocgo path (`appnap_other.go`), following the existing `-x objective-c
-fobjc-arc` cgo pattern used elsewhere in the agent.

- **Option choice: `NSActivityUserInitiatedAllowingIdleSystemSleep`.**
It suppresses App Nap and sudden/automatic termination (the behavior
causing the eviction) while deliberately **not** setting
`NSActivityIdleSystemSleepDisabled` — holding an idle-sleep-disabling
assertion for the entire lifetime of an always-on background LaunchAgent
would stop a laptop from ever idle-sleeping and drain the battery. When
the machine legitimately sleeps, the broker sleeps with it, so no
keepalive is due.
- **Honesty note:** this guard is a best-effort OS-behavior mitigation
and isn't meaningfully unit-testable (it asserts against the OS
scheduler). The tested, load-bearing fix is the write deadline; the App
Nap assertion addresses the likely trigger.

The optional "keep the pong off the shared write-mutex" idea (proposed
fix #3 in the issue) is left as a potential follow-up — the write
deadline already breaks the permanent-wedge failure mode, so it's not
needed for correctness here.

## Tests

- `TestConnSendWriteDeadline` — a `Send()` whose underlying `net.Pipe`
write stalls (peer never reads) returns an error within the deadline
instead of blocking forever.
- `TestConnSendClearsWriteDeadline` — a second `Send()` after the
(short) timeout elapses still succeeds, proving the deadline is cleared
between sends.

```
go build ./...                                                  # ok (darwin+cgo)
go vet ./internal/ipc/... ./internal/userhelper/...             # clean
go test -race ./internal/ipc/... ./internal/userhelper/... ./internal/sessionbroker/...   # ok
CGO_ENABLED=0 go build ./internal/userhelper/...                # ok (nocgo stub)
GOOS=linux CGO_ENABLED=0 go build ./internal/userhelper/...     # ok (other stub)
```

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Todd Hebebrand <todd@lanternops.io>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Jul 12, 2026
Hardens the agent→API trust boundary against a **compromised enrolled
agent**, from a focused security review. 10 findings (5 HIGH, 5 MEDIUM)
were independently verified, then fixed and re-verified.

## Findings & fixes

**HIGH**
- **#1 renew-cert quarantine bypass** — the route read org mTLS policy
and wrote device state *after* its DB context closed, so under forced
RLS as the unprivileged role those reads/writes silently returned 0 rows
and a `quarantine` policy fell back to `auto_reissue`. Now reads policy
+ all device writes inside `withSystemDbAccessContext`, fails closed on
a missing org row or any 0-row write, and only returns cert material
after metadata persists (else revokes the issued cert).
- **#2 previous-token self-renewal** — a superseded token (5-min grace)
could mint durable new agent/watchdog/helper credentials. Now rejected
(401) on both rotate-token and renew-cert; rotate-token UPDATE is a
compare-and-swap bound to the authenticating token hash (409, mints
nothing, on a losing race).
- **#3 established WS bypass containment** — quarantine and tenant
suspension/deletion didn't sever a live agent socket. Now they do; plus
a lightweight per-message lifecycle recheck on the command-claim and
command-result paths.
- **#4 duplicate WS orphan** — a second socket replaced the tracked one
without closing the first, leaving an orphan revocation couldn't reach.
Now single-socket-per-agent (superseded socket closed on connect) with
connection-identity-guarded ping state.
- **#5 private-key redaction** — the agent regex missed PKCS#8 and left
the key body intact. Now removes whole PEM blocks (PKCS#8 +
RSA/EC/DSA/OPENSSH/ENCRYPTED), plus a server-side redaction layer at
result ingest for pre-update agents.

**MEDIUM**
- **#6** CSV formula-injection — 5 web exporters now neutralize
agent-controlled cells via the shared helper.
- **#7** watchdog diagnostics never reached the API (wrong wire shape /
status / false "completed"); now speaks the log contract and reports
failure honestly.
- **#8** WS command-result completions weren't audited (REST path was);
now emit `agent.command.result.submit` once per real transition.
- **#9** `logs.ts` ingest had no audit event; added a content-free one.
- **#10** heartbeat overwrote security-relevant state with no history;
now emits `agent.heartbeat.state_change` on genuine transitions only.

## PR-review follow-up (same branch)
A multi-agent review verified the auth cluster clean (no
bypass/RLS/silent-mint) and caught issues in the redaction layer, all
fixed here: a **ReDoS** in the new server-side redactor (bounded →
linear), an unredacted `error` field, truncated-key passthrough, and
redaction parity (ported the agent's
AWS-key/bearer/JWT/connection-string/secret-pair patterns server-side).

## Verification
- `apps/api` typecheck clean; **171** unit tests pass
- `apps/web` **15** tests pass
- Go build + `-race` (executor / watchdog / breeze-watchdog) pass

## Pre-merge follow-ups (not done here)
1. **Real-Postgres integration test for renew-cert (#1).** The fix's
premise — a contextless read returns 0 rows under forced RLS, a
system-context read returns the real row — is proven only via mocks.
Should be closed with one integration test against real Postgres per the
repo's RLS-contract discipline.
2. **Cross-replica socket severance.** `disconnectAgent` is in-process
only, so a live socket on another API replica isn't severed immediately
on quarantine/tenant-suspension (mitigated by the per-message recheck;
matches the existing decommission convention). Fine if prod is
single-API-replica per region — worth confirming.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Todd Hebebrand <todd@lanternops.io>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Jul 13, 2026
…eat_ack frames (#2407) (#2412)

## Summary

On WebSocket connect, `agentWs.ts` claimed up to 10 pending commands —
marking them `status='sent'` + `executedAt` — and embedded them as
`pendingCommands` in the `connected` welcome frame. The agent's
`handleConnectedMessage` has only ever parsed `type`/`capabilities`
(verified back to the original WS client in b0ef2c9), so those
commands were never executed. The WS `heartbeat_ack` path had the
identical latent bug: no shipped agent sends WS heartbeats, and the
agent's readPump skips ID-less frames, so any batch embedded there would
also be dropped.

The stranded rows sat falsely `sent` — never delivered, never executed —
until `staleCommandReaper` eventually flipped them to `failed` with a
misleading "no response from agent" timeout error, misattributing the
loss to agent unreachability.

## Fix — option (b) from the issue

- **Remove the connect-time claim + `pendingCommands` embed** from the
welcome frame (now capabilities negotiation only).
- **Remove the `heartbeat_ack` claim**; the ack keeps `commands: []` for
wire-shape stability with the REST heartbeat response.
- Commands stay `pending` for the delivery paths that work with **every
shipped agent**: the HTTP heartbeat claim (the agent heartbeats
immediately on startup, so on-connect latency is unchanged in practice)
and `executeCommand`'s direct per-command push (frames *with* an id)
while the socket is live.
- **Finding #3 containment preserved**: the sever that lived inside the
removed claim path now runs directly in the WS heartbeat handler via
`isAgentDeviceStillAuthorized` — and now *before* the status write, so a
contained device can no longer be flipped online by its own heartbeat
moments before disconnect.

Server-only fix — old agents in the field are fixed immediately on
deploy; option (a) would have required a fleet-wide agent update while
old agents kept losing commands.

## Supersedes the #2399/#2405 WS batch budget

With no WS frame ever carrying a command batch, the
`maxTotalPayloadBytes` budget on `claimPendingCommandsForDevice` (added
in #2405) has zero callers and is removed along with its pinning tests —
the oversized-batch-frame hazard is now structurally impossible rather
than budgeted. The agent-side 16MB read limit from #2405 is untouched.

## Tests

- New `WS frames never claim pending commands (#2407)` regression suite:
welcome frame has no `pendingCommands` + no claim; `heartbeat_ack`
always `commands: []` + no claim. (Review verified by mutation these
fail if a WS-side claim is reintroduced.)
- Finding #3 heartbeat-sever coverage extended: suspended-token case
updated to the new direct check, new `decommissioned` case pinning that
the sever fires before the status write.
- Removed the #2399 budget pinning tests (behavior superseded).
- `agentWs.test.ts` + `commandDispatch.test.ts`: 51 passed;
`commandQueue.test.ts` + `routes/agents/heartbeat.test.ts` (callers of
the changed claim signature): 102 passed; `tsc --noEmit` clean.

Closes #2407

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Todd Hebebrand <todd@lanternops.io>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ToddHebebrand pushed a commit that referenced this pull request Jul 16, 2026
…-snapshot errors, surface dashboard degraded state (#2556 review)

IMPORTANT #1 — agentWs: a malformed or oversize verify/test-restore result set
device_commands to failed but early-returned before the per-type handler ran,
stranding the backup_verifications / restore_jobs row in running/pending until
the 30-min stale-timeout sweep. On a validation rejection we now still dispatch
the handler for the verification and restore families so the linked record
transitions to a terminal failed state via its normal failure path.

IMPORTANT #2 — legacy snapshots with configId=null (predating destination
tracking) previously 422'd/threw a misleading "destination configuration not
found". Added resolveBackupDestinationError() to backupProviderConfig.ts and
threaded it through restore.ts, aiToolsBackup.ts and verificationService.ts to
return a distinct "predates backup destination tracking" message (with a
reason discriminator), without any current-config fallback that could read the
wrong destination.

SUGGESTION #3 — dashboard resolveAttentionItems swallowed all errors and
returned [], rendering a transient DB failure as "healthy". It now returns a
degraded flag; the /dashboard response carries an additive, backward-compatible
attentionError signal so the UI can distinguish "no failures" from "couldn't
compute".

Tests added/updated for all three across agentWs, restore, verificationService
and dashboard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ToddHebebrand added a commit that referenced this pull request Aug 7, 2026
…#3214) (#3224)

Closes #3214

## Root cause — found, reproduced, and pinned

The issue listed the `Immediate.nextWrite` TypeError as a *likely*
poisoner. It is the whole story, and the mechanism is now reproduced
deterministically with no database, no network and no timing luck
(`apps/api/src/db/postgresJsPoolPoisoning.test.ts`).

In **postgres.js 3.4.9** (`src/connection.js`), small writes are batched
behind a `setImmediate`, guarded by an "already scheduled?" check:

```js
function write(x, fn) {                                        // :246
  chunk = chunk ? Buffer.concat([chunk, x]) : Buffer.from(x)
  if (fn || chunk.length >= 1024)
    return nextWrite(fn)
  nextWriteTimer === null && (nextWriteTimer = setImmediate(nextWrite))
  return true
}

function nextWrite(fn) {                                       // :254
  const x = socket.write(chunk, fn)
  nextWriteTimer !== null && clearImmediate(nextWriteTimer)
  chunk = nextWriteTimer = null                                // <- the ONLY reset
  return x
}
```

`nextWrite` actually running is the only thing that ever resets
`nextWriteTimer`. But both teardown paths cancel the immediate **without
clearing the variable**, and without dropping the buffered `chunk`:

```js
function terminate()            { ... clearImmediate(nextWriteTimer) ... }        // :427
async function closed(hadError) { ... clearImmediate(nextWriteTimer) ...          // :440
                                      socket = null ... }
```

So a connection whose socket dies with a deferred write pending is left
with `nextWriteTimer` permanently non-null. The guard at `:250` is false
forever after, no flush is ever scheduled again, and the
`StartupMessage` written by `connected()` on **every** subsequent
reconnect is appended to `chunk` and never reaches the socket. The
handshake cannot complete, `connect_timeout` expires,
`connectTimedOut()` reports `CONNECT_TIMEOUT` and destroys the socket,
`closed()` reconnects — forever.

That accounts for every symptom in the report:

| Observation | Explanation |
|---|---|
| `TLSSocket.closed` errors starting days early | each one poisons one
pool slot |
| pool decay 35 → 9 over hours | slots poisoned cumulatively, never
recovered |
| ~144 `CONNECT_TIMEOUT`/min against a healthy DB | poisoned slots
reconnect-looping |
| `Immediate.nextWrite` TypeError | the same broken lifecycle (`socket`
nulled / stale `chunk`) |
| restart fixes it in seconds | fresh closures |

**Verification of the repro** — the control (undisturbed connection)
writes 77 bytes of `StartupMessage`; kill the socket while the flush is
queued and the *reconnect* writes **0 bytes**.

## Suggested fix #1 — answered, and it is a dead end

> *check whether postgres.js > 3.4.9 fixes it, or file upstream*

**3.4.9 is the latest published release.** `npm view postgres version` →
`3.4.9`. There is nothing to bump to, and the broken state lives inside
a closure that nothing outside the driver can reach. The reproduction in
this PR is dependency-free and ready to attach to an upstream report.

## What this PR ships (fixes #2 and #3)

**`apps/api/src/services/dbConnectTimeoutStats.ts`** — a rolling
`CONNECT_TIMEOUT` count, fed from `safeDiagnoseConnectTimeout` (the
wrapper both production call sites already use). Deduped per error
object with a `Symbol`, because `app.onError` and `captureException`
classify the *same* error — without that the rate would be silently 2x
on the request path and 1x on the worker path, making any threshold
meaningless. Zero-import leaf, so it does not recreate the `services →
db` back-edge the classifier's docblock warns about.

**`apps/api/src/db/dbPoolHealthMonitor.ts`** — the watchdog. A sustained
timeout rate on its own is ambiguous: unreachable DB and poisoned pool
look identical. So when the rate breaches the threshold it opens **one
brand-new connection** to the same database, outside the pool. A fresh
client has fresh closures and is immune to the poisoning:

- fresh connection **succeeds** → `pool-degraded`: the DB is fine, the
pool is not, **restart the API**
- fresh connection **fails too** → `database-unreachable`: real fault,
**restarting will not help**
- probe could not be *attempted* → `unknown`, explicitly refusing to
blame either

That is exactly the manual step that ended the incident ("`psql` from
the same host connected in under a second"), performed automatically.
Steady state costs one unref'd timer tick — the probe only opens a
socket once the threshold is already breached.

**`apps/api/src/routes/metrics.ts`** —
`breeze_db_connect_timeouts_total{cause}`,
`breeze_db_connect_timeout_rate_per_min`,
`breeze_db_pool_health{verdict}`. Seeded at 0 so alert rules never query
a nonexistent series. All verdict series stay 0 before the first
evaluation — absence of a verdict must not read as `healthy`.

Plus `.env.example` + `deploy/environment.mdx` for the six new knobs,
and a bootstrap-wiring guard (`dbPoolHealthWiring.test.ts`) — a watchdog
that is defined but never started fails completely silently, which is
how this class of blindness ships.

## What I did NOT do, and why

- **Recycling the `sql` instance.** The issue offers this as optional; I
left it out. `db/index.ts` hands the client to Drizzle at module load,
and the resulting `baseDb` is captured by the exported proxy, by every
open `withDbAccessContext` transaction, and by the AsyncLocalStorage
stores. A mid-flight swap would abort in-flight tenant transactions, and
a bug in it is a whole-API outage — a worse failure than the one being
fixed. Wants its own PR.
- **Patching the driver in `node_modules`.** The actual repair is ~2
lines (reset `chunk`/`nextWriteTimer` in `closed()` and `terminate()`),
and the pin test would prove it. I left it out: this repo has **no**
`patchedDependencies` today, so it would be the first — introduced on
the DB driver, touching the Docker and CI install surface, and I cannot
validate a driver-lifecycle patch against real production traffic from
here. It deserves a dedicated PR with deploy validation.
- **Anything that claims to fix the pool.** This PR makes the failure
*visible and correctly attributed*. It does not repair it.
`pool-degraded` still means someone restarts the API.

The pin test is deliberately asserting the **bug**. The day the driver
is fixed or patched, it goes red with a message naming exactly what to
delete (the pin and the watchdog).

## Review round

Three passes (`code-reviewer`, `silent-failure-hunter`,
`pr-test-analyzer`) found real defects, all sharing one shape — the
change reintroduced, in its own code, the blindness it exists to remove.
Fixed in `914926027`:

- **`captureException` classified connect timeouts *below* its
`initialized` guard.** On any instance without a Sentry DSN — the
self-hosted default — `app.onError` was the counter's only feed, so
every worker, scheduler and `unhandledRejection` path contributed
nothing. That is exactly the incident's profile ("the loudest signature
in Sentry was the patch scheduler, not any route"), meaning the watchdog
would have reported the pool fine right through it. Classification now
happens above the guard.
- **The quiet verdict `healthy` is now `below-threshold`.** The count is
a documented floor and slot occupancy is never observed, so an
affirmative all-clear was a claim the evidence cannot support. There is
deliberately no `healthy` series to alert on the negation of.
- **A failed evaluation left the previous verdict standing**, so
`/metrics` republished a stale reading for hours about a dead watchdog.
It now clears the verdict, counts the failure, and reports itself to
Sentry (it was console-only).
- **A probe timeout under event-loop starvation reported
`database-unreachable`** with "restarting will not help" attached — the
#3022 misdirection one layer up, since the probe's own budget needs the
same blocked loop. Now a distinct error type and an `unknown` verdict.
- **In-flight guard + probe budget clamped to half the interval.** Legal
env combinations (`INTERVAL=5000, PROBE=30000`) could stack probes, each
opening another connection to the database being diagnosed. Probe close
failures are now counted, not silently swallowed.
- Stable Sentry title (interpolated rates minted a new issue per
capture, so no alert bound to one could fire twice), a
suppressed-since-last-capture count, and sample retention that trims to
the widest window any reader requested rather than the caller's.
- **The six `DB_POOL_HEALTH_*` vars are threaded through both compose
files.** The docs promise they work, and an unmapped var set in `.env`
is silently inert (the IS_HOSTED / #570 failure mode).

### Tests

The test pass found that **five mutations survived the entire suite**:
deleting the counter feed, the recorder binding, the scrape refresh, the
interval body, or the throttle guard all stayed green — i.e. every line
of production wiring could be removed without CI noticing. Each is now
covered, and I re-applied the mutations to confirm they fail. Also added
hermetic tests for the real probe (no database needed) and a **negative
control** on the driver pin, so "the reconnect wrote nothing" now
demonstrably means "*because* the socket died with a buffered write"
rather than for any unrelated reason.

### Second review round (delta re-review)

Because the first round's fixes touched `captureException` — every error
path in the API — the delta got its own pass. Three more real findings,
fixed in `57898f941`:

- **The Sentry captures were arriving contentless.** `scrubEvent`
deletes `message`, `logentry` and `extra` from *every* outgoing event,
and my tag key was not in `ALLOWED_TAG_NAMES` — so it was dropped twice
over. A `pool-degraded` alert reached Sentry as an unlabelled blank that
grouped with every other blank. The verdict is now allowlisted as
`db_pool_health_verdict` (the same precedent `connect_timeout_cause` set
for #3022), with a scrubber test proving it survives. The docs no longer
claim more than Sentry actually receives.
- **The probe-timeout downgrade tested the wrong direction.** It asked
"does starvation dominate?", which leaves open exactly the hole it
exists to close: with the event-loop monitor disabled or warming up,
*every* timeout classifies as `unknown`, so the starvation count is 0,
no dominance is found, and the confident `database-unreachable` fires in
the configuration with the least evidence. Now inverted to require
positive evidence *for* connectivity.
- **A throwing `captureMessage` erased a valid verdict.** It fell
through to the outer catch, which (correctly) clears `lastAssessment` —
so a reporter fault would blank a real degraded verdict from `/metrics`
at the moment it fired.

Plus: the two failure counters were `Gauge`s carrying the `_total`
suffix OpenMetrics reserves for counters (renamed), and the check-failed
key never drained its suppressed count.

## Verification

- `vitest run src/db src/services src/routes/metrics.test.ts
src/config/envComposeParity.test.ts --no-file-parallelism` → **622
files, 10084 passed, 35 skipped**
- `tsc --noEmit -p apps/api/tsconfig.json` (with the CI
`--max-old-space-size=8192`) → clean
- `eslint` on all touched files → clean
- Driver pin run 8x consecutively → 8/8 stable (the `setImmediate`
ordering is guaranteed by the driver's microtask-only path from socket
factory to `connected()`, not merely usual)
- Both compose files parse and carry all six new vars

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Todd Hebebrand <todd@lanternops.io>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant