Repository navigation
fix: wire the console sidebar workspace switcher - #786
Merged
Merged
Conversation
The sidebar "WORKSPACE" control in ConsoleShell carried aria-haspopup="menu" but had no click handler and rendered no menu at all, so a signed-in user belonging to more than one workspace had no mouse or keyboard path to switch between them. A working switcher already existed (WorkspaceSwitcher, posting to the already-validated POST /console/account-switch handler) but was rendered visually hidden in app/console/layout.tsx for an unrelated e2e-locator reason. Wire that existing switcher into the sidebar instead of building a second one. It now renders as a native <select> (a listbox) rather than a hand-rolled ARIA menu: the browser already gives correct keyboard navigation, focus handling, and screen-reader semantics for picking one of N mutually exclusive workspaces, and it reuses the same server-validated POST flow. When a user belongs to exactly one workspace, the control renders as a static, non-interactive label instead, so it never presents as an empty menu. ConsoleShell now takes the viewer's memberships and the current account id as props; all 15 console pages already call getViewer() and pass viewer.memberships through with no extra network call. The now-redundant sr-only WorkspaceSwitcher in the layout is removed. Adds a regression test that fails against the previous dead button (no combobox for multi-workspace, aria-haspopup present for single-workspace) and passes once the switcher is wired. Visual proof of both states (menu open with two workspaces, and the single-workspace non-interactive control) is in docs/proof/console-workspace-switcher-785/, captured against the real component tree through the project's web-console Docker image; see evidence.txt for the exact substrate.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 21 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (20)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sakibsadmanshajib
added a commit
that referenced
this pull request
Aug 9, 2026
## Summary - `deploy-demo-box.yml`'s `paths:` trigger filter had no entry for `apps/web-console`, the source directory `web-console-prod` (the service serving console-hive.scubed.co under the `chat` profile this workflow deploys) is built from. PR #786 changed only files under that path, merged to main, and triggered no deploy at all. It sat on main, undeployed and silently absent from the box, until a manual `workflow_dispatch` shipped it. - Audited the full filter against every path each actively deployed service's Docker build reads (every build context in this compose file is repo root, `../../`) and found five more gaps of the same shape: `go.work` / `go.work.sum` (copied first by every Go service Dockerfile), `.dockerignore` (controls what every Docker build context sees at all; this file's own header documents it as the confirmed root cause of 16+ prior red deploy runs), and two scripts the workflow itself invokes directly by name (`scripts/derive-pooler-dsn.py`, `scripts/check-retention-schedule.sh`). - Added all six, each with a comment explaining what it covers and why it was missing, matching the file's existing convention for the `supabase/migrations` and `hive_jwt_forward` entries. ## Full path-to-service mapping (as of this audit) Deploy job runs `docker compose --profile local --profile chat --profile monitoring up -d --build`. Services live in that set: `edge-api`, `control-plane`, `litellm` (no profile gate, always on), `redis`, `open-webui`, `caddy-owui`, `caddy-artifacts` (`local`), `agent-console`, `web-console-prod`, `caddy-console` (`chat`), `prometheus`, `grafana`, `alertmanager` (`monitoring`). | Path | Covers | |---|---| | `apps/edge-api/**` | `edge-api` build | | `apps/control-plane/**` | `control-plane` build | | `apps/agent-console/**` | `agent-console` build | | `apps/agent-engine/**` | not itself deployed here (`agent` profile, not in this job's profile set), but a real Go module dependency `control-plane`'s and `edge-api`'s Dockerfiles `COPY` in full | | `apps/web-console/**` (added) | `web-console-prod` build — the gap that caused this PR | | `packages/**` | shared Go modules (`storage`, `audit-canonical`, `embedmodel`) + `openai-contract` matrix/openapi files, all `COPY`ed into `edge-api`/`control-plane`/`agent-engine` images | | `deploy/docker/**` | every Dockerfile, `docker-compose.yml`, all three Caddyfiles, `open-webui` patch scripts, and this workflow's own compose invocations | | `deploy/litellm/**` | LiteLLM seed config | | `go.work`, `go.work.sum` (added) | first `COPY` in every Go service Dockerfile; a workspace `use` change is a real build input | | `.dockerignore` (added) | governs every Docker build context (`../../`) for every service above | | `supabase/migrations/**` + 3 scripts | `migrate` job | | `scripts/check-retention-schedule.sh`, `scripts/derive-pooler-dsn.py` (added) | invoked directly by name in the `migrate` and `deploy` jobs | | `scripts/install-owui-jwt-forward.py`, `scripts/owui-mint-admin-token.py` | `deploy` job's OWUI JWT-forward install step | | `.github/workflows/deploy-demo-box.yml` | itself | No gap found for `apps/desktop`, `apps/desktop-sandbox`, `website/`, `docs/`, or the root `package.json` — none of them feed any Docker build this job runs. ## Is a `paths:` allowlist even the right mechanism here? No, not as the long-term shape, though this PR keeps it. An allowlist fails silent by construction: every miss (this one, the `supabase/migrations` gap from before, the `hive_jwt_forward` gap from before) looks identical to "nothing relevant changed" until someone notices the box is stale, sometimes a day later. A denylist (`paths-ignore:` naming `website/`, `docs/`, `apps/desktop*/`, `.wolf/`, and other paths genuinely known not to feed this stack) fails safe instead: an unlisted new path defaults to triggering a deploy rather than defaulting to being silently skipped, which matches the owner's stated preference that an unnecessary deploy is cheap and a silently skipped one is not. I did not make that switch here. It changes the trigger semantics for the whole workflow rather than closing named gaps, so it deserves its own review and its own verification pass (confirming the denylist doesn't have to be as exhaustively risk-assessed as an allowlist, which paths genuinely never touch this stack, whether removing `apps/agent-engine/**`'s special case still holds, etc.) rather than riding along on a fix framed as closing specific holes. Recommend it as a fast-follow, reviewed on its own. ## Test plan - [x] `python3 -c "import yaml; yaml.safe_load(open('.github/workflows/deploy-demo-box.yml'))"` — valid YAML - [x] Diff reviewed: additive only, no existing entries touched, comments match the file's established convention - [ ] Not merging this PR per the runbook; a fresh `main` deploy is being triggered separately via `workflow_dispatch` regardless of this PR's merge state, since main already carries undeployed changes independent of this fix
sakibsadmanshajib
added a commit
that referenced
this pull request
Aug 9, 2026
…796) (#808) Closes the three protections an audit confirmed had no test that could fail. Each was confirmed the same way: delete or neuter the guarded code, run the suite, watch it stay green. Refs #793, #794, #796. ## What was wrong, and what now catches it ### #793 — three gates whose attachment was never exercised The platform admin gate on `/v1/admin/credit-grants` could be deleted and `go test ./apps/control-plane/... -count=1 -short` still exited 0 across 52 packages. The same held for the RAG and Cowork gates in `apps/edge-api/cmd/server/main.go`. The cause was structural, not an oversight in any one test. All three gates were applied inline inside `main()`, and every test that touches this code builds its own middleware and calls it directly. That proves the middleware works. It proves nothing about whether the middleware is on the route, which is the only thing that was broken. Each route family's registration now lives in a named function that a test can drive through a real `http.ServeMux`: - `apps/control-plane/cmd/server/grant_routes.go` — `registerCreditGrantRoutes` - `apps/edge-api/cmd/server/gated_routes.go` — `registerRAGRoutes`, `registerAgentTaskRoutes` `main()` calls them with the same arguments it used inline, so runtime behaviour is unchanged. The new table tests assert an outcome per path and per caller, and every denial is paired with a positive control, so a gate that denied everyone, or a route that had stopped existing, cannot pass as a success. ### #794 — a fix with no guard PR #786 added the `onChange` handler that makes the sidebar workspace switcher do anything at all. Deleting it again left all 423 unit tests passing, because the only test on the component read its source text for the `"use client"` directive. `tests/unit/workspace-switcher.test.tsx` now renders the control and intercepts `requestSubmit` to capture the live `FormData` at that instant. It asserts the submitted `account_id`, not the select's value: the select's value is what the test itself just wrote into the DOM, so asserting on it passes with no handler at all. ### #796 — three assertions that checked rendering, not function - `console-workspace-admin.spec.ts` asserted `aria-checked` matched `/true|false/`. Unanchored, and those are the only two values the attribute can hold, so it was true of a toggle nobody had ever clicked. The spec never clicked one. It now clicks the toggle, asserts the flip, reloads, asserts the flip came back from the server, then flips it back and reloads again, which also proves the control moves in both directions rather than latching once. - `console-budgets.spec.ts` used `toBeAttached()` for a read-only claim. That is equally true of an enabled field. It now asserts `toBeEnabled()` for the owner and `toBeDisabled()` plus the read-only notice for a member, reaching both roles by switching workspaces rather than branching on whichever role the run happens to produce. - `rbac-unverified.spec.ts` branched on whether a redirect had happened and asserted something different in each arm, so it passed either way, and both arms would also have passed on a 500 or an empty page. `app/console/api-keys/page.tsx` redirects unconditionally when `can(viewer, "api_keys.write")` is false, so the outcome is one specific URL. It now asserts exactly that, with a verified-user positive control on the same route. ## Persistence No spec in this repository called `page.reload()`, so no setting anywhere had a test that it survived one. Three now do: the feature gate toggle, the budget read-only state, and a new `tests/e2e/console-workspace-switch.spec.ts` that switches workspace, reloads, and asserts the value came back from the `hive_account_id` cookie the `/console/account-switch` handler wrote, rather than from client state the next navigation would have thrown away. ## Incidental finding: a third dark suite `console-budgets.spec.ts` and `rbac-unverified.spec.ts` are in the tree but appear in no workflow file. Neither has run in CI since it was written. That is the same hole as #796 one level up, and a third dark suite alongside the two already tracked as #708 and #659. Both need only the `E2E_VERIFIED_*` and `E2E_UNVERIFIED_*` identities the Web E2E job already sets, so both are added to that job here, along with the new switch spec. ## What I executed, and what I did not Stated plainly, because a clean-looking PR is worth less than an accurate one. **Executed, red and green both observed.** For each break below I ran the suite, saw it fail, restored the code, and saw it pass. | Break | Result | | --- | --- | | Removed the `gate.Require(FeatureRAG)` wrapper | RED, 2 subtests, 200 instead of 403 | | Removed the `gate.Require(FeatureCowork)` wrapper | RED, 3 subtests | | Swapped Cowork for RAG on `/v1/agent/tasks` | RED, 4 subtests, both directions | | Removed `RequirePermission(PermPlatformAdmin)` from credit grants | RED, 3 subtests: a verified workspace owner reached the grant minting surface | | Widened that gate to `PermBillingView` | RED, same 3 | | Deleted the `onChange` from `workspace-switcher.tsx` | RED, `expected [] to have a length of 1` | Also executed: `go vet` on both `cmd` trees and the full `go test ./apps/control-plane/... ./apps/edge-api/... -count=1 -short`, all green; and the full web-console unit suite, 425 passed, 1 skipped. **Not executed: the three #796 Playwright rewrites and the new switch spec.** They need a booted Next.js server against a seeded Supabase fixture, and I was not able to start one in this environment. I have not run them, and I am not claiming a green I did not see. They are statically sound and the assertions are written against the real component markup (`aria-label` on `GateSwitch`, `#budget-soft-cap`, the seeded workspace display names), but the first CI run on this branch is their first real execution. Treat that run as the verification step, not this description. **Two pre-existing unit failures, unrelated to this branch.** `tests/unit/control-plane-host.test.ts` fails with `ENOENT /app/.env.example` inside the test container, and one members-page assertion fails. Neither file is touched here, and this branch changes no `apps/web-console/components/` source at all. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added protected credit-grant administration and authenticated self-service access. * Added feature-gated access for RAG and agent-task capabilities. * Improved workspace switching with selection persistence across page reloads. * Added clearer read-only behavior for non-owner workspaces. * Restricted API key creation to verified users and improved redirects for unverified users. * **Bug Fixes** * Improved workspace admin settings persistence and restoration after changes. * **Tests** * Expanded end-to-end coverage for workspace switching, budgets, permissions, feature gates, and API key access. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
sakibsadmanshajib
added a commit
that referenced
this pull request
Aug 10, 2026
…) (#829) Fixes #826. ## Verdict, in one sentence **The product is not broken; the two specs were failing on their own shared helper, which waited for a URL that a same-URL redirect never changes and so never waited for the switch at all.** **One root cause, not two separate defects.** `console-budgets` "disabled for a member" and `console-workspace-switch` "survives a reload" are the same defect seen from two angles: both call `switchToWorkspace`, both then race the in-flight POST, and each fails at whatever it happened to do next. One helper is fixed and both go green, and no other caller of it exists. On the specific worry in #794: the switcher **does** survive a reload for a real user, so #786 did not replace a dead button with a control that silently does nothing. Screenshot proof is in the PR comment and committed under `docs/proof/workspace-switch-race-826/`. Nothing was rotated to produce any of this, and no token, key, JWT, or connection string appears in the captures, the committed evidence, or this body. ## Detail: the specs were wrong, the product is not Issue #826 said the direction mattered, because the two possibilities call for opposite fixes and one of them is an authorization hole on a money surface. It is neither of those. The workspace switcher works, the switch survives a reload, and a workspace member cannot edit that workspace's budget caps. The shared spec helper both specs drive was never waiting for the switch it asked for. ### The defect `tests/e2e/support/workspace-switch.ts` waited like this: ```ts await Promise.all([ page.waitForURL((url) => url.pathname.startsWith("/console"), { timeout: 25_000 }), workspaceSelect(page).selectOption(accountId), ]); ``` `/console/account-switch` always answers a switch with a 303 back to `/console`, so on the console the URL is identical before and after. And `Frame.waitForURL` short-circuits when the current URL already matches (`playwright-core/lib/client/frame.js:162`): ```js async waitForURL(url, options = {}) { if (urlMatches(this._page?.context()._options.baseURL, this.url(), url)) return await this.waitForLoadState(options.waitUntil, options); await this.waitForNavigation({ url, ...options }); } ``` The page is already on `/console` when the helper is called, so that wait returned immediately, every time, without ever observing the navigation it existed to wait for. Whatever the caller did next then raced the in-flight POST. That single cause produces both reported failures and every reported variant of them: | Where | Symptom | | --- | --- | | CI | `page.reload: net::ERR_ABORTED; maybe frame was detached?` | | Local, `console-workspace-switch` | `toHaveCount` failing with `waiting for navigation to finish...` | | Local, `console-budgets` | `toBeDisabled()` received `enabled`, because `page.goto("/console/billing/budget")` aborted the switch and the budget page still rendered the workspace the user owns | **The two failures share one root cause.** Only these two specs call the helper, so the fix is one function. ### Proof the product is correct Against a stack booted from `main` at `f7d9293f` (control-plane plus redis from `deploy/docker`, a production `next build` of `apps/web-console` pointed at it), driving the same build with a correctly awaited navigation: ``` PROBE options: ["E2E Verified Workspace (current)","E2E Shared Workspace"] PROBE after switch->member, value: 2cc1a48f-... PROBE after reload, value: 2cc1a48f-... PROBE after reload, options: ["E2E Verified Workspace","E2E Shared Workspace (current)"] PROBE persisted: true PROBE budget soft-cap disabled on member workspace: true ``` `docs/proof/workspace-switch-race-826/01-member-workspace-budget-read-only.png` shows all of it in one frame: the sidebar reading `E2E Shared Workspace (current)` after a reload and a further navigation, both cap inputs greyed out, Save budget disabled, and the notice "Only the workspace owner can edit budget caps." On the deployed box (`console-hive.scubed.co`, signed in through the audited `live-auth` helper, nothing rotated) the demo account holds a single workspace, so the sidebar renders the static label branch and no interactive control at all. **No user is losing a workspace selection right now**, and the static label confirms the deployed build carries #786. ### The fix Wait for the next `load` event instead of a URL. A load event is per document, so it cannot be satisfied by the document that is already open, and it fires after the replacement document is committed. Waiting on the redirect's response was tried first and is not enough: the bytes arrive before the browser swaps documents, and a `page.goto()` in that window still dies with `interrupted by another navigation`. The helper also returns early when the target workspace is already selected. React's change plugin does not dispatch `onChange` for an unchanged select value, so no form submit and no navigation happen, and waiting for a load event there would hang until the timeout. The two specs are also given a 120s budget. Each drives seven or eight full server-rendered navigations and the 30s default is a budget for a single page, so on a slower runner the clock decides the result before the assertions do. A timeout budget cannot turn a failing assertion green. ### Every assertion proven able to fail Four deliberate breaks, each rebuilt and re-run against the live stack. All product-code breaks were reverted; `git diff` against `main` touches no product code. | Break | Result | | --- | --- | | Deleted the `onChange` handler in `workspace-switcher.tsx`, the exact regression #786 fixed and #794 said had no guard | **Both specs red.** `page.waitForEvent: Timeout 25000ms exceeded while waiting for event "load"` | | `readOnly={!isOwner}` to `readOnly={false}` in `app/console/billing/budget/page.tsx` | **`console-budgets` red** at line 115, `toBeDisabled` `Expected: disabled / Received: enabled`. `console-workspace-switch` stayed green, so the budget assertion is about the owner gate and not about the switch | | `/console/account-switch` stops setting the `hive_account_id` cookie | **`console-workspace-switch` red** at line 92, `toHaveValue` `Expected: 2cc1a48f-... / Received: 33ac1fa0-...`, which is the pre-switch workspace. This is the reload-persistence guard #794 asked for, doing its job | | Forced the new "already selected" early return to always fire | **Both specs red**, with the same two assertions above. The short-circuit cannot silently swallow a real switch | Green before and after on the same stack: 3 passed, including `budget page renders` which was already green in CI. ### Not changed No assertion was weakened, removed, or rewritten to match current behaviour. The only spec-level change beyond the helper is the timeout budget. No product code is touched. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved workspace switching reliability by ensuring navigation completes before subsequent actions. * Prevented unnecessary navigation when the selected workspace is already active. * Workspace selection now persists more reliably across page reloads. * Improved enforcement and verification of read-only budget permissions for members. * **Tests** * Increased end-to-end test timeouts to better support multi-step workspace and budget workflows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 11, 2026
Closed
Backlog inventory 2026-08-11: 13 issues were fixed and left open, and the backlog is 281 not 90
#853
Closed
sakibsadmanshajib
added a commit
that referenced
this pull request
Aug 18, 2026
…#976) ## Summary `deploy-demo-box.yml`'s `push.paths` filter had no entry for `vendor/open-webui`, the forked chat frontend's source tree that `Dockerfile.open-webui` compiles directly from (`COPY vendor/open-webui ./`). A change under that tree merges to `main` and triggers no deploy at all. PR #971 (nav fixes, merged clean) is the live example: it touched only `vendor/open-webui`, and no deploy run followed. The fork build in PR #938 deployed only because it also touched `deploy/docker/Dockerfile.open-webui`, which the filter does cover, so past deploys were incidental rather than evidence the path worked. The workflow's own comment already documents this exact failure class for `apps/web-console` (PR #786) and `supabase/migrations`. This is the same defect recurring a second time in the same file. I audited every other Dockerfile the deploy job builds (`edge-api`, `control-plane`, `agent-console`, `web-console.prod`, `agent-engine`) plus the ones it does not (`sdk-tests-*`, `toolchain`, `desktop-linux`, non-prod `web-console`). Every COPY/ADD source across all of them falls under `apps/**`, `packages/**`, `deploy/docker/**`, or `go.work(.sum)`, all already present in the filter. `vendor/open-webui` was the only gap. ## Changes - `.github/workflows/deploy-demo-box.yml`: add `vendor/open-webui/**` to `push.paths`, with a comment pointing at the new guard below. - New `.github/ci/lint-deploy-paths-filter.mjs`: parses the deploy workflow's `push.paths` and every `deploy/docker/Dockerfile.*`'s COPY/ADD sources, fails if any source is not covered by the filter. Skips `--from=` stage copies, which read a prior build stage rather than the host filesystem. - Wired into `.github/workflows/ci.yml`'s existing `repo-policy-lints` job (already a required check, already installs `node`+`yaml`), right after the sibling `lint-workflow-check-names.mjs` step. No new CI job. Deliberately not added to `.github/branch-protection-main.json`'s required-checks list: `repo-policy-lints` is already required, so this step riding inside it already fails the PR loudly on a gap. Promoting it to its own named required check is a separate, more sensitive branch-protection change and out of scope here. ## Verification Ran the new guard locally (`npm ci --ignore-scripts && node .github/ci/lint-deploy-paths-filter.mjs`): - Passes after this fix: `Deploy path-filter coverage OK: every COPY/ADD source across 14 Dockerfiles under deploy/docker/ is covered...` - Stashed only the `deploy-demo-box.yml` change and reran: fails loud, naming `vendor/open-webui/package.json`, `vendor/open-webui/package-lock.json`, and `vendor/open-webui` itself as uncovered. Confirms the guard would have caught the original bug. - Re-ran the sibling `lint-workflow-check-names.mjs`: still passes, no regression (23 check names, 6 required contexts). - Both edited workflow YAMLs parse cleanly. ## Deploy note Merging this PR changes `.github/workflows/deploy-demo-box.yml` itself, which is already in its own filter, so the merge will trigger a real deploy to the demo box. That deploy will also carry every other merged-but-undeployed change currently sitting on `main` (including PR #971). Given the open P0 on the shared Supabase connection pool, hold this merge for explicit go-ahead rather than merging on green CI alone. ## Buglog entry Second instance of the same defect class in the same file (first: PR #786, `apps/web-console`). ```json {"error_message":"vendor/open-webui had no entry in deploy-demo-box.yml's push.paths filter; a change under that tree merged to main and triggered no deploy","root_cause":"the paths filter is a hand-maintained allowlist and the fork's frontend source tree (added when the chat frontend was forked to compile from source) was never added to it, so PR #971 (nav fixes touching only vendor/open-webui) merged clean with zero deploy run","fix":"added vendor/open-webui/** to the filter; added .github/ci/lint-deploy-paths-filter.mjs (wired into ci.yml's repo-policy-lints required check) which parses every deploy/docker/Dockerfile.*'s COPY/ADD sources against the filter and fails loud on the next missing entry instead of silently never deploying","tags":["ci","deploy","paths-filter","open-webui","recurring"]} ```
5 tasks done
sakibsadmanshajib
added a commit
that referenced
this pull request
Aug 25, 2026
## Summary
Two `apps/web-console` unit test files failed to load on every
clean-tree run of the mandated `docker compose run --rm --build
web-console npm run test:unit` command, and a third test silently
skipped, all for the same reason: `deploy/docker/Dockerfile.web-console`
builds with the repo root as its build context (`context: ../../` in
`docker-compose.yml`) but only `COPY`s `apps/web-console/`. Anything a
test reads from outside that subtree does not exist in the image.
- `tests/unit/control-plane-host.test.ts` reads `.env.example` and the
whole `deploy/` tree (to cross-check every hostname under `deploy/`
against `deploy/cloudflare/tunnel-ingress.json`) at module load time.
Missing `.env.example` threw synchronously, so the entire file failed to
load.
- `tests/unit/chat-coverage-lib.test.ts` reads
`docs/proof/chat-interaction-coverage-2026-08-10/coverage.run.json`
inside the "is never below what any recorded live run enumerated" test,
which reproduced the reported ENOENT.
- `components/catalog/model-catalog-table.test.ts` guards its last
assertion with `it.skipIf(constraintValues.length === 0)`, and
`constraintValues` comes from `existsSync(supabase/migrations)`. That
directory is also outside the copied subtree, so the test silently
skipped instead of failing loud. Same root-cause class as the two
ENOENTs above, just surfaced as a quiet skip; the test file itself
already documents this ("Skipped only where supabase/migrations is not
on disk, which is the deploy/docker web-console image").
## Fix
Added three narrow `COPY` lines to `Dockerfile.web-console`, after the
existing `COPY apps/web-console/ ./`, landing each fixture at the same
repo-root-relative path the tests already resolve against (`/app/...`,
matching `WORKDIR /app/apps/web-console`'s `/app` root):
- `.env.example` (64K)
- `deploy/` (2.5M, needed whole since the test recursively scans it for
hostnames)
- `docs/proof/chat-interaction-coverage-2026-08-10/coverage.run.json`
(76K, the one file the test reads, not all of `docs/proof/` which is
17M)
- `supabase/migrations/` (652K)
None of these paths are excluded by the repo's `.dockerignore`. Total
added context: about 3.2M, chosen to keep the image copy narrow rather
than pulling in the rest of the repo (`docs/proof/` alone is 17M, of
which the tests need one 76K file).
Deliberately did not make either test skip when its fixture is missing,
and did not touch the assertions themselves: this repo has an explicit
rule that a loud failure is preferable to a quiet absence, and the fix
here is to make the fixture actually present rather than to soften the
check.
## Investigation of the reported "1 failed" test
Re-running the mandated command against `origin/main` before this fix,
three times, produced 0 failures beyond the two ENOENT load failures and
the one skip described above; the module-level reads in
`control-plane-host.test.ts` throw at import time (whole-file load
failure), while `chat-coverage-lib.test.ts`'s ENOENT throws lazily
inside a single `it()` body, so depending on how a given tool renders
vitest's collect-error vs test-failure distinction, that single
assertion can show up as either "file failed to load" or "1 failed test"
in a summary. After the fix, all three (both ENOENTs and the skip)
resolve together; no separate, unrelated failing test was found across
repeated runs.
## Required-check fix: `deploy-demo-box.yml` paths filter
The first push failed the required `Repo policy lints (tenant + audit)`
check, specifically `node .github/ci/lint-deploy-paths-filter.mjs`. Read
the real failure (`gh run view --log-failed`) rather than guessing: the
lint is genuinely working as designed. It walks every `COPY`/`ADD`
source in every `deploy/docker/Dockerfile.*` and requires a matching
entry in `deploy-demo-box.yml`'s `on.push.paths`, because that filter
has twice before silently swallowed a real change (`apps/web-console`
via #786, `vendor/open-webui` via #971) with no failure at all, just a
merge that never triggered a deploy. My three new `COPY` sources
(`.env.example`, `deploy/`, the one `docs/proof/...` file) were exactly
the next instance of that gap. `supabase/migrations/` was already
covered by an existing `supabase/migrations/**` entry, so the lint did
not flag it.
Fixed by adding three entries to the paths filter (with a comment
matching this file's existing convention, referencing this PR):
`.env.example`, `deploy/**`,
`docs/proof/chat-interaction-coverage-2026-08-10/coverage.run.json`.
`deploy/**` is deliberately the whole tree, not narrowed to the
pre-existing `deploy/docker/**`/`deploy/litellm/**` entries, for the
same reason the Dockerfile copies the whole tree (see next section).
Verified locally: `npm install` at repo root, then `node
.github/ci/lint-deploy-paths-filter.mjs` reports `Deploy path-filter
coverage OK: every COPY/ADD source across 15 Dockerfiles under
deploy/docker/ is covered`. Also re-ran the sibling
`lint-workflow-check-names.mjs` in the same job to confirm it is
unaffected.
## Security review of the `COPY deploy/ /app/deploy/` scope
Answering the three questions raised on this PR directly:
**1. Does anything under `deploy/` that now enters the image carry a
credential, token, or internal hostname that should not be baked into a
built image?** No. Grepped the whole `deploy/` tree for AWS-style keys,
PEM private-key headers, `sk-...` style tokens, and literal
`password:`/`secret:` values with a non-`os.environ` right-hand side:
zero hits. Read `deploy/litellm/config.yaml` specifically since it was
named directly: every `api_key:` line in it is
`os.environ/OPENROUTER_API_KEY`, `os.environ/GROQ_API_KEY`, or the
literal `"none"` (for a route that takes no key); there is no hardcoded
credential anywhere in that file. It does carry internal hostnames and
routing/pricing commentary, which is operational detail rather than a
secret, and no different from what already ships in the image today via
`deploy/docker/**`/`deploy/litellm/**`, both already `COPY`ed by other
Dockerfiles in this same directory before this PR.
**2. Is `hive-web-console:ci` (the image this Dockerfile builds)
genuinely CI/test-only, or does it also serve the console on the demo
box?** Confirmed CI/test-only, and distinct from what ships.
`docker-compose.yml` defines two separate services: `web-console` (image
`hive-web-console:ci`, this Dockerfile, gated behind `profiles: [dev]`,
runs plain `next dev`) and `web-console-prod` (image
`hive-web-console-prod:ci`, a *different* file,
`Dockerfile.web-console.prod`, sitting behind its own Caddy origin,
`caddy-console`). The header comment on the paths-filter's existing
`apps/web-console/**` entry says this explicitly:
"console-hive.scubed.co is web-console-prod... built... by
Dockerfile.web-console.prod." `deploy-demo-box.yml` never references the
`web-console` service or the `:ci` tag at all, only `apps/web-console`
as a source path (because `web-console-prod` is built from that same
source tree). `docker-bake.hcl`'s `web-console` target tags the image
`hive-web-console:ci` and is never pushed to any registry in `ci.yml` or
`deploy-demo-box.yml` (grepped both; no `docker push`/registry step
references this tag). So the image this PR's `COPY deploy/` lands in
never leaves the CI runner or a developer's machine, and never reaches
the demo box.
**3. Would a narrower `COPY` satisfy the test?** No, not without
defeating the test's purpose, and this is deliberate, not laziness.
`control-plane-host.test.ts`'s "deploy configuration hostnames" suite
exists specifically to recursively scan every text file under all of
`deploy/` (excluding `deploy/cloudflare/` itself, which is the registry
it checks against, and `node_modules`) for any `*.scubed.co` hostname
not declared in `deploy/cloudflare/tunnel-ingress.json`. That is the
whole point of the suite: catch a stray or retired hostname in *any*
file under `deploy/`, including ones that do not exist yet. Copying only
today's known files would make the test blind to the next Caddyfile or
compose fragment added later, which is exactly the silent-gap failure
mode this repo's own rules warn against. `supabase/migrations/` is the
same shape (whole directory, needed because the constraint-value scan
unions across every migration file). The two single-file copies
(`.env.example`, the one `coverage.run.json`) are already as narrow as
the tests need.
## Test plan
- [x] `cd deploy/docker && docker compose run --rm --build web-console
npm run test:unit` run to completion against the fixed image, with a
clean/idle host: `Test Files 60 passed (60)`, `Tests 653 passed (653)`,
0 failed, 0 skipped.
- [x] Confirmed via `.dockerignore` review that none of the four
newly-copied paths are excluded from the build context.
- [x] `node .github/ci/lint-deploy-paths-filter.mjs` passes locally
after the paths-filter fix.
- [x] Grepped `deploy/` for hardcoded secrets (AWS keys, PEM headers,
`sk-...` tokens, literal password/secret values); none found.
`deploy/litellm/config.yaml` confirmed to use `os.environ/...`
indirection for every API key.
- [x] Confirmed via `docker-compose.yml`, `docker-bake.hcl`, and both
`ci.yml`/`deploy-demo-box.yml` that `hive-web-console:ci` is
CI/dev-only, never pushed to a registry, and not the image serving the
demo box (`web-console-prod`/`Dockerfile.web-console.prod` is).
Note on later local reruns: after rebasing onto a newer `main`, repeated
non-`--build` and `--build` runs on this same dev box intermittently
showed 2-5 unrelated test failures
(`analytics-billing-page-wiring.test.tsx`, `members-page-rbac.test.tsx`,
others), each a different set each run, with no code in this diff
touching those files. The box was running 20+ other containers from
concurrent sessions at the time (`docker ps` showed multiple
`hiveverify-*`, `composerfix-*` stacks, 4.2G swapped) and the same run's
own "environment" setup phase ballooned from ~150s to ~360s between
attempts, consistent with resource-contention flakiness in
`waitFor()`-based React tests rather than a regression from this PR's
Dockerfile/workflow-only diff. GitHub's isolated CI runner ran the `Web
console (type + unit + build)` job clean (60/60) on the pre-fix commit
of this same PR, which is the authoritative signal for this class of
test.
## Buglog entry
```json
{"date":"2026-08-25","error_message":"ENOENT open '/app/.env.example' and ENOENT open '/app/docs/proof/chat-interaction-coverage-2026-08-10/coverage.run.json' during `docker compose run --build web-console npm run test:unit`; separately, components/catalog/model-catalog-table.test.ts silently skips its last assertion via it.skipIf","root_cause":"Dockerfile.web-console uses the repo root as build context but only COPYs apps/web-console/, so any test reading a repo-root path (.env.example, deploy/, docs/proof/..., supabase/migrations/) gets ENOENT or a false existsSync inside the image, even though those files are present on disk in every other run context","fix":"Added narrow COPY lines for .env.example, deploy/, the one coverage.run.json fixture under docs/proof/, and supabase/migrations/ into Dockerfile.web-console, landing each at the same /app-relative path the tests already resolve against; added matching entries to deploy-demo-box.yml's push.paths filter so a change to any of these still triggers a demo-box deploy; left the tests themselves untouched so a genuinely stale/missing fixture still fails loudly","tags":["web-console","docker","test-fixtures","vitest","dockerfile","ci-paths-filter"]}
```
This was referenced Aug 26, 2026
sakibsadmanshajib
added a commit
that referenced
this pull request
Aug 28, 2026
…1246) ## Summary A merge to main can trigger no `deploy-demo-box` run at all, and the absence is invisible on a green PR page. PR #1222 squash-merged as `483ba7983`, touched `apps/edge-api/**` (which the paths filter matches explicitly), and produced no `deploy-demo-box` run and no `CI` run whatsoever, resolved only by a manual `workflow_dispatch` once someone happened to notice. This is the third occurrence of the symptom "merge to main, no deploy" with a different cause each time; the first two (PR #786, PR #971) were paths-filter gaps, already guarded by `.github/ci/lint-deploy-paths-filter.mjs`. This one was not a config error, so no paths-filter fix can close it. ## What this adds `deploy-drift-watchdog.yml`: a **scheduled** (every 30 minutes) reconciliation, deliberately not push-triggered, because a push-triggered guard would be silenced by the exact same webhook/dispatch anomaly it exists to catch. Each run: 1. Resolves main's tip SHA and the last successful `deploy-demo-box` run's `headSha` from GitHub's own API/run history (not from anything that could have gone silently stale on the local box). 2. Computes which changed paths between those two SHAs are covered by `deploy-demo-box.yml`'s own `on.push.paths` filter, reusing `lint-deploy-paths-filter.mjs`'s exported `pushPaths`/`isCovered` (new file: `list-covered-deploy-changes.mjs`) so the watchdog and that lint can never disagree about the same commit. 3. Stays quiet (exit 0) when: no covered path changed since the baseline (a real "correctly never deployed" case), a deploy is currently queued or in progress, or the newest covered commit is inside a 15-minute grace window (ordinary webhook/queue latency). 4. Otherwise files or updates a deduped GitHub issue (title-matched, same list/comment/create shape `agent-visual-proof.yml` already uses) and fails the run, so both a persistent issue and a red scheduled check exist. Deliberately does **not** add an on-box "record the deployed SHA" mechanism (the optional ask in the issue): `post-deploy-verify.yml`'s existing image-freshness step (issue #869) already answers the harder version of that question ("is the box actually serving what was built", not just "did a workflow run"), so a second, narrower ledger here would duplicate it rather than extend it. ## Verification - `node .github/ci/lint-deploy-paths-filter.mjs` still passes after exporting `pushPaths`/`isCovered` (no behavior change to the existing lint). - `list-covered-deploy-changes.mjs` tested locally against real repo history: 200 commits back from main's tip produces thousands of covered changed paths (confirms the "fires" path finds real matches); base == head produces zero (confirms the "quiet" path). - This PR is labeled `run-deploy-drift-watchdog` plus `deploy-drift-test:simulate-divergence` to exercise the real workflow end to end from this PR (workflow_dispatch is not usable before this merges): the run should go **red**, and file/comment on the real tracking issue, proving the alert mechanism itself, not just the detection logic. A follow-up push removes the simulate label and re-runs to prove the same workflow **stays quiet** against real, current repo state (main's tip already equals its own last successful deploy's headSha as of this PR). - The test-created tracking issue is closed by hand once the red run is confirmed, noted in a PR comment. ## Buglog entry ```json {"ts":"2026-08-28","error_message":"PR #1222 squash-merged as 483ba79, touched apps/edge-api/** which deploy-demo-box.yml's paths filter matches, and produced no deploy-demo-box run and no CI run whatsoever","root_cause":"a workflow that never runs does not fail, it is absent, and a green PR page cannot distinguish that from a real skip; not a paths-filter gap (issue #1238 confirms the filter matched), a one-off webhook or workflow-dispatch delivery anomaly with no config to blame","fix":"added deploy-drift-watchdog.yml, a schedule-triggered (not push-triggered, deliberately, since the same anomaly would silence a push-triggered guard) job comparing main's tip against the last successful deploy-demo-box run's headSha every 30 minutes, quiet on legitimate no-deploy cases (no covered path changed, a deploy already in flight, within a 15-minute grace window) and filing a deduped tracking issue otherwise","tags":["ci","deploy","observability"]} ``` ## Test plan - [x] `lint-deploy-paths-filter.mjs` still passes locally - [x] `list-covered-deploy-changes.mjs` verified locally (fires on a real divergence, quiet on a real match) - [ ] Labeled PR run shows the workflow firing red + filing/updating the real tracking issue - [ ] Follow-up run (simulate label removed) shows the workflow staying quiet - [ ] Test tracking issue closed after verification
4 of 5 tasks
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
1 participant
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.
Fixes #785.
Root cause
apps/web-console/components/app-shell/console-shell.tsxrendered thesidebar "WORKSPACE" control as a static
<button aria-haspopup="menu">with no
onClick, no state, and no menu markup anywhere in thecomponent. Clicking it did nothing, for any membership count.
A working switcher already existed:
apps/web-console/components/workspace-switcher.tsx, a<select name="account_id">posting to the already-validated/console/account-switchhandler. It was renderedsr-onlyinapp/console/layout.tsxto keep an e2e locator working, with acomment claiming "the redesigned shell carries its own visible
switcher button" — that never happened.
Fix
Wired the existing switcher into the sidebar instead of building a
second one:
WorkspaceSwitchernow renders as a real, visible<select>(alistbox) styled to match the sidebar card, rather than a hand-rolled
ARIA menu. A native select already gives correct keyboard nav, focus
handling, and screen-reader semantics for "pick one of N mutually
exclusive options" — a more correct ARIA pattern for this control
than
aria-haspopup="menu"ever was.static, non-interactive label instead — no
<select>, noaria-haspopup, nothing that looks clickable with nothing behind it.ConsoleShellnow takesmembershipsandworkspace.idas props.All 15
/console/*pages already callgetViewer()and hadviewer.membershipsin scope, so this is prop-threading with zeroextra network calls.
sr-only<WorkspaceSwitcher>inapp/console/layout.tsxis removed; the visible one in the sidebarcarries the same
select[name='account_id']shape the existing e2esuite locates.
ChevronGlyphhelper removed.Test
apps/web-console/tests/unit/console-shell.test.tsxis new. Verifiedfailing first against the pre-fix code (stashed the fix, rebuilt the
Docker image, ran the test):
getByRole("combobox", ...)found nothing —no menu was ever wired.
[aria-haspopup]was still present on thedead button.
Restored the fix, rebuilt, reran: both pass. Full
npm run test:unitsuite: 412 passed, 1 pre-existing unrelated failure
(
control-plane-host.test.ts,ENOENT .env.example— the web-consoleDocker build context doesn't copy the repo-root
.env.example; nottouched by this change).
npm run build(type check +next build)succeeds for all
/console/*routes.Visual proof
docs/proof/console-workspace-switcher-785/(seeevidence.txtforthe exact capture substrate — the real component tree run through the
project's own web-console Docker image, screenshotted with a real
Chromium via Playwright; this sandbox has no live Supabase
credentials, so it isn't the hosted demo box, and the harness route
used to reach it was deleted before this commit and never shipped):
No real credential, session, or workspace data appears in any image;
account/user names are fabricated placeholders. No address bar is
present in a component-level screenshot, so no URL redaction applies.
Note on CI
GitHub Actions is reported down at the time of this PR. Required
checks may not run. This PR should not be merged until checks report
green through the normal process — flagging explicitly so a missing
check is not mistaken for a passing one.
Test plan
docker compose build web-console(picks up source; the imagebakes source with no volume mount)
docker compose run --rm web-console npm run test:unit— newtest fails against pre-fix code, passes against the fix; full
suite otherwise green (1 pre-existing unrelated failure)
docker compose run --rm web-console npm run build— type check+ production build succeeds
and single-workspace (non-interactive) states