Board tags and tag filter, seven catalog corrections, and the #1006 OAuth pack record (supersedes #1495, #1496, #1499) - #1501
Conversation
Data-only fixes from running fleet's own discovery against every official entry (#1006 audit, 2026-09-13/14), each verified live against the vendor: - expensify: URL is /mcp/ — the vendor 308s the slash-less form and fleet refuses redirects, so Add failed. - cartesia: the MCP endpoint is /mcp; the bare origin discovers (via the legacy origin fallback) but 404s every tool call (measured live). - octagon: the server's own metadata names mcp.octagonai.co as the resource; the entry's mcp.octagonagents.com host serves the same server but a token must be bound to the audience the vendor declares. - globalping: no longer open — initialize answers 401 with protected-resource metadata and a self-registering, public-client authorization server. - zerodha-kite: no OAuth at the transport (initialize answers 200, no metadata anywhere); users sign in through the server's `login` tool → open, with a hint saying so. - sage-intacct: no dynamic registration and no `none` auth method → client_registration: manual + client_secret: required, with a setup hint; pinned in TestBuiltinRemoteCatalogClientSecretRequired. - openrouter: docs_url 404 → https://openrouter.ai/docs/mcp. Not changed: square and smartlead document SSE-only endpoints, which fleet's hosted-connector transport (streamable HTTP) cannot speak — a product decision (hide, or keep with a warning) and possibly a transport gap, kept out of a data-only change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…pendix, and an operator runbook for hosted connectors docs/MCP-CATALOG-STATUS.md is the record #986 asked for: the live-run table for the twelve hosted connectors exercised against real vendor accounts (add, consent, tools, tool calls from chat and scheduled tasks, refresh, sign-out, seats and sharing, with the fleet PR each one needed), the fleet changes the pack produced, the audit findings with their disposition, and a per-entry appendix from re-probing all 231 official OAuth/tenant/open entries with fleet's own discovery on the current code and catalog. docs/HOSTED-CONNECTORS-RUNBOOK.md is what an operator needs before and after Connect: how the callback URL is derived and which vendors refuse plain http, what the encryption key protects, the three shapes of Connect, the measured per-vendor prerequisites (GitHub, Google Workspace, Slack, Notion, Linear, Azure DevOps, Stripe, Grafana Cloud, Uptime Robot, Plaid, Intercom, Square, Smartlead), how chat and scheduled runs mount connections, seats and sharing, and how to read fleet.log and the Add/sign-in errors. Both pages are indexed in docs/README.md. Live results are separated from discovery-only probes throughout, and the vendors that were deliberately skipped (Square, Smartlead, the Plaid/Intercom fallback) are named as such. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Tags were write-only. The create form accepted them, the API stored them, the
server could already filter on them (`?tag=a&tag=b`, ANDed) — and no surface in
the web app ever showed one again. The one thing a tag is for, finding the rest
of its group, could not be done from the UI at all.
Now:
- A task's tags render as chips on its row and on its phone card, coloured by
the same hashed palette as the chat label chips so a tag reads the same
everywhere. Each chip is a control: clicking one adds that tag to the board's
filter, clicking a selected one removes it.
- The filter bar gains a **Tags** group — a select that ADDS a tag (it never
holds a value, because the board is filtered by every chip beside it, not by
the last one chosen) plus a removable chip per selected tag.
- Options come from `GET /tasks/tags`, the existing catalogue endpoint, fetched
once per activation rather than on the 30s refresh: it is a GROUP BY over
every task's tag array and it changes only when someone retags something. The
offered set is that catalogue unioned with the tags on the listed tasks, so a
tag created after the catalogue loaded is still selectable as soon as a task
carrying it appears.
- Tags count as an active filter, so Clear filters appears and clears them.
Two things had to change underneath. `passThroughQuery` read only the first
value of each parameter, which is right for every single-valued filter and
wrong for `tag`: dropping the second of `?tag=a&tag=b` widens the result
instead of narrowing it, the one direction a filter must never fail in. It now
forwards every value, unchanged for single-valued parameters. And
`/api/orchestrator/tasks/tags` did not exist; the static segment wins over the
sibling `[taskId]` route, so it does not shadow GET /tasks/{id}.
The Operations Center guide said to treat tags as a label for your own grouping
rather than a control on this screen. That was true and is not any more.
Verified: `make ci-web` (153 files / 1608 tests, audit clean, build clean),
`tsc --noEmit`, and the mocked Playwright suite (98 passed). The three tests
that guard real defects — every tag reaching the query, a chip not opening the
log viewer, tags counting as an active filter — were each confirmed to fail
against the bug they describe.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018qQJHUvRV2FM7Rj2GfUNqg
Codex is right, and the ARIA role I reached for was the tell. The phone card is a real <button>, so a tag chip inside it is a control nested in a button — invalid semantics however it is marked up. Rendering the chip as a span with role="button" dodges the HTML rule and keeps the actual problem: assistive technology can expose only the outer "View task" control, or make the tag action ambiguous. The chips are now a sibling of the card button inside the same <li>, and each is a real <button> with its own accessible name — so the insideButton variant, its hand-rolled Enter/Space handling and its event-stopping all go away rather than being fixed. That put them outside the card's border, since the border lived on the button. The card's box (border, radius, background) moves to the <li> and the button inside it goes transparent, so the card looks the same and now actually contains everything in it. Verified: tsc, oxlint, and 47 TasksTable tests. The new test was confirmed to fail against the nested layout it replaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018qQJHUvRV2FM7Rj2GfUNqg
Three Codex findings, all correct. **The catalogue never refreshed.** It was fetched once per activation, and `active` stays true for the whole signed-in session — so a tag created afterwards, on a task that is not on the page in front of you, never reached the dropdown until a full reload. Fetching it with the dashboard's 30s refresh is the wrong fix: it is a GROUP BY over every task's tag array and it changes only when somebody retags something. TAG_CATALOGUE_TTL_MS (5 minutes) bounds both, refreshed off the ordinary reload cadence. The stamp is taken before the request, so overlapping reloads start one fetch, and it stands on failure, so a failing endpoint is retried on the TTL rather than on every reload. The gap the TTL leaves is closed from the other side, as before: tagOptions unions the catalogue with the tags on the listed tasks, so a brand-new tag is selectable the moment a task carrying it appears. **docs/USER-GUIDES.md contradicted the shipped interface.** Its record of guide claims corrected during review still said tags do not filter the board and that TaskFilters offers no tag support. That was true when written — and writing it is what surfaced the gap — so the bullet keeps its history and is marked as since closed rather than deleted. **No design note.** AGENTS.md asks for one per feature; docs/TASK-TAGS.md records what shipped, the two decisions worth keeping (why a chip is a real button outside the card button, why the catalogue has a TTL), and the honest scope: the catalogue is deployment-wide while the board is not, which is why no tag counts are shown, and what was left out. Verified: `make ci-web` (153 files / 1610 tests, audit clean, build clean), the mocked Playwright suite (98 passed), and `go test ./scripts/` — which includes the docs-index check that would have caught the missing row. The TTL test was confirmed to fail against the once-per-activation effect it replaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018qQJHUvRV2FM7Rj2GfUNqg
…solidate/open-prs-2026-09-14
…to consolidate/open-prs-2026-09-14
…solidate/open-prs-2026-09-14 # Conflicts: # docs/README.md
Four findings from the pre-PR review of the merged tree, each verified against the source before fixing. The tag catalogue's TTL effect depended on refreshNonce, so a dashboard reload completing while the catalogue request was in flight ran the cleanup, dropped the valid response, and left catalogueFetchedAt stamped: no retry for five minutes and an empty dropdown. Request lifetime is now governed by a generation counter, bumped when a fetch starts and when the hook deactivates or unmounts (which also zeroes the stamp so reactivation fetches at once). A reload no longer cancels anything; a response whose generation is stale is discarded because a newer request owns the stamp or deactivation reset it. Three regression tests cover the reload-mid-flight case, an older fetch resolving after a newer one across deactivation, and reactivation before the TTL elapses. The desktop table row's Enter/Space handler ran for events bubbling from the tag chips inside it, opening the log viewer and, via preventDefault, blocking the chip's native activation. The handler now returns unless the row itself is the event target. Test covers Enter and Space on a focused chip. docs/MCP-CATALOG-STATUS.md overstated grantee isolation: AcquireTokenByID delegates to AcquireToken, so a grantee's run does refresh the owner's token row (and a 401 at mount sends the owner's row to needs_reauth), and revocation is checked at mount time, so a run already holding the mount finishes. The spacelift appendix row embedded the audit's probe tenant hostname; it is now a <tenant> placeholder, and the appendix legend says so. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b993f0ba0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Five review findings on #1501, each checked against the code. The status record still called #1488 and #1495 open; #1488 is merged and #1495 landed here. The runbook said scheduled runs mount every connected connection, but BuildRemoteMCPOverlay caps the mount at maxOverlayServers (8) for chat, scheduled runs and the broker alike, skipping the rest with a log line, and mcp_selection pins do not lift the cap; it also said any connection that cannot refresh is marked Reconnect needed, when network errors and 5xx stay transient and only a terminal OAuth error sets needs_reauth. Catalog corrections do not reach saved connections: the mount is built from the saved row's URL and the directory marks an entry added by name, so a Cartesia, Octagon or Globalping connection added before the fix keeps the old definition until it is removed and re-added. That is now stated in CONNECTOR-ONBOARDING.md and recorded as F13 with its deferral. Zerodha Kite's hint claimed the login session lives on Kite's side. fleet opens and closes a fresh MCP session per turn, so the login tool's session does not outlive the turn that called it and a scheduled run cannot sign in; the description, hint and status rows now say so. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review Your first pass reviewed
No runtime code changed in |
|
@codex security review Housekeeping: the security pass ran against |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61f9b9df0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 61f9b9df0a
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
GET /tasks/tags returned every tag in the deployment with per-task counts. That endpoint predates this change (#212) and nothing in the web app called it, so the exposure was theoretical; the board's new tag dropdown now fetches it through the web proxy for every signed-in user, which would have let a principal without the fleet-wide grant read other principals' tag names and per-tag activity. The catalogue is now scoped exactly as ListTasks is (#1082): a principal without admin or view_all_logs gets only tags on tasks it created, filtered in SQL on created_by or created_by_key_id, and the endpoint requires view_tasks like the list does. The JSON shape is unchanged. TestTagCatalogueScope covers the query (fleet-wide, user-scoped, key-scoped, unrelated key sees an empty catalogue); TestTagCatalogueAuthz covers the handler for a scoped user, a scoped API key, an intruder and an admin. The design note and both Operations Center guide copies said the dropdown lists every tag across the deployment; they now describe the scoped catalogue, and the note records why counts stay out of the UI. Runbook: three more honesty fixes from review. The Kubernetes callback sentence no longer implies an Ingress or prescribes where TLS terminates (the chart ships ingress.enabled false and ingress.tls empty); 128 tools is the default disclosure threshold, overridable by FLEET_TOOL_DISCLOSURE_THRESHOLD and the tool_disclosure_threshold admin setting; and the troubleshooting bullet for the token-unavailable log line distinguishes terminal OAuth failures, which mark the row Reconnect needed, from transient ones, which retry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 631e148cf5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…guide gaps The phone-card box moved to the list item, which clips overflow, so the card button's outward focus ring was invisible to keyboard users. The ring now lives on the list item, scoped with :has() to the card button so a focused tag chip shows only its own ring. The Operations Center guide promised every visible-task tag in the dropdown; a tag added to a task off the current page can take up to five minutes to appear by default (the catalogue's TTL), so the guide says so. The hosted connectors runbook told operators to keep the OAuth encryption key with the database backups, which puts the key beside the ciphertext it protects; it now asks for a separate access-controlled store and notes both are needed for recovery. It said scheduled runs mount every connected connection; only connections the user has enabled mount, and a disabled one is omitted with no notice to the model. And its catalog check expected every endpoint to answer initialize with 401, which is the OAuth shape only; open and api_key entries must complete initialize and tools/list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1dc7f082b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… keeps its cron-only SARIF category (#1503) ## What changed, and why Two leftovers from #1501 and #1502. **`GET /tasks/tags` in `docs/openapi.yaml`** still described a global catalogue. Since #1501 the endpoint requires `view_tasks` and is scoped by the same own-rows rule as `GET /tasks` (#1082): a non-admin user or scoped API key gets only the tags on tasks it created, with counts over those tasks, while the admin key, `PermissionAdmin` carriers and `view_all_logs` holders get the fleet-wide catalogue. The summary now says so in the same words the `/tasks` entry uses, so an API consumer does not mistake an omitted tag for a nonexistent one. **The govulncheck cron category is correct and now says why.** #1502 moved the weekly Grype upload onto the PR gate's category because GitHub expected the cron-only `grype-scheduled` configuration on every PR. `govulncheck-scheduled.yml` looks like the same shape, but is not: the per-PR govulncheck step in `ci.yml` runs the scan and uploads no SARIF, so this lane is the only govulncheck upload, there is no PR-side category to share, and GitHub shows no missing-configuration check for it (confirmed on #1501 and #1502, where only the Grype phantom appeared). The workflow comment and `docs/TESTING.md` record that so the category is not renamed by analogy. No scan command, schedule or alarm changed. ## How you verified it - `make lint` including actionlint over the edited workflow: clean. - `go test -tags fleet_host_executor -count=1 ./scripts/`: PASS. - Checked `gh pr checks` on #1501 and #1502: no "govulncheck" configuration-not-found check, only the Grype one that #1502 removed. - Reviewed once locally by Codex before opening. ## Scope and deviations Docs and one workflow comment only; no runtime, gate or schedule change. The alternative of making the PR gate emit SARIF under a shared category was not taken: it would change the per-PR gate for no visible defect. --- - [x] The title and "What changed, and why" are written for the release notes they become - [x] A design note (`docs/<FEATURE>.md`) added, if this ships a feature — not a feature - [x] An ADR added or superseded in `docs/adr/`, if this adds, weakens or reverses an invariant — none touched - [x] The diff is scoped to one change (no unrelated refactors) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
What changed, and why
One PR that supersedes the three open PRs, merged locally on top of current
mainand driven to green together. Supersedes #1495, #1496 and #1499; each of those is closed in favour of this one.Tags on the Operations Center board, and a tag filter (from #1499). Every task row and phone card shows its tags as chips; pressing a chip filters the board by that tag, and the filter bar offers a tag dropdown fed by a deployment-wide catalogue (
GET /api/orchestrator/tasks/tags, proxied to the backend) unioned with the tags on the listed tasks. The catalogue is refreshed at most every five minutes off the dashboard's ordinary reload cadence. Design note:docs/TASK-TAGS.md; both Operations Center guide copies updated anddocs/USER-GUIDES.mdcorrected so it no longer says tags do not filter the board.Seven built-in catalog entries corrected (from #1495). Data-only fixes the #1006 OAuth audit found by running fleet's own discovery against every official entry: expensify's trailing slash, cartesia's
/mcpendpoint, octagon's declared resource host, globalping now OAuth, zerodha-kite open with a sign-in hint, sage-intacct manual registration with a required client secret (pinned by test), openrouter's docs URL. Square and smartlead (SSE-only) are deliberately not changed.The #1006 OAuth pack recorded (from #1496).
docs/MCP-CATALOG-STATUS.md(live-run status table for twelve hosted connectors, the fleet changes the pack produced, and a per-entry appendix from re-probing all 231 official OAuth/tenant/open entries) anddocs/HOSTED-CONNECTORS-RUNBOOK.md(what an operator needs before and after Connect, per vendor). Both indexed indocs/README.md.Review fixes on the merged tree (new here, from a pre-PR review of the combined diff):
docs/MCP-CATALOG-STATUS.mdoverstated grantee isolation (a grantee's run does refresh the owner's token row; revocation is checked at mount time) and embedded the audit's probe tenant hostname in the spacelift row, now a<tenant>placeholder.Codex GitHub review of this PR (five P1 threads, all verified against the code and fixed in
61f9b9df): stale #1488/#1495 dispositions in the status record; the runbook now states the eight-server overlay cap and that transient refresh failures do not mark a connection for reconnect; saved connections keeping a pre-correction URL is documented inCONNECTOR-ONBOARDING.mdand recorded as finding F13 (reconciliation deferred, remove and re-add); the Zerodha Kite entry's hint no longer claims the login session persists across turns or works in scheduled runs.Codex re-review (three P2 runbook threads and one P3 security finding, fixed in the third follow-up commit):
GET /tasks/tagsreturned the deployment-wide catalogue with counts to any signed-in user; it is now scoped exactly likeListTasks(#1082 own-rows rule,view_tasksrequired, SQL filter oncreated_by/created_by_key_id), withTestTagCatalogueScopeandTestTagCatalogueAuthz. The design note and both Operations Center guide copies describe the scoped catalogue. The runbook no longer implies a Kubernetes deployment has an Ingress or prescribes where TLS terminates, names 128 tools as the default disclosure threshold with its two overrides, and distinguishes terminal from transient refresh failures in troubleshooting.Third Codex pass (five threads, fixed in the fourth follow-up commit): the phone card's keyboard focus ring was clipped by the list item's
overflow: hiddenand is now drawn on the list item via:has(); the guide discloses the catalogue's five-minute refresh bound; the runbook stops telling operators to keep the OAuth encryption key with the database backups, says only user-enabled connections mount in scheduled runs (a disabled one is omitted silently), and qualifies the 401 catalog check as the OAuth shape only.How you verified it
Local Postgres (podman
postgres:18) with the three DSN vars set, so the DB-backed packages ran rather than skipped:make build: clean.scripts/go-test.sh --count=1(themake testpath): 63 packagesok, 0 failures;internal/store12.7s,internal/httpapi22.0s,internal/runner7.7s confirm the DB suites executed.make lint: golangci-lint 0 issues; ruff check and format --check clean; migration lint 0 files.go test -tags fleet_host_executor ./scripts/ ./internal/clientconfig/...: PASS (docs-index check, guide sync check, catalog tests including the newsage-intacctpin).make sync-guides: no drift between the two guide copies.make ci-web: npm audit 0 vulnerabilities; oxlint 0 warnings/errors on 532 files; typecheck clean; vitest 153 files / 1614 tests passed (4 new);next build74/74 pages.make ci-e2e-mocked: 98 passed.gitleaks git --log-opts=origin/main..HEAD: 5 commits scanned, no leaks.61f9b9df):go test -tags fleet_host_executor -count=1 ./scripts/ ./internal/clientconfig/...PASS (docs index, catalog decode and pins);make lintclean.scripts/go-test.sh --count=1against Postgres again, 63 packagesok, 0 failures;TestTagCatalogueScopeandTestTagCatalogueAuthzprintPASS(notSKIP) under-v;make lintclean;make sync-guidesno drift;go test ./scripts/PASS;make ci-webgreen (1614 tests, build 74/74).make ci-webgreen again (1614 tests, 74/74),make sync-guidesno drift,go test ./scripts/PASS,make lintclean, oxlint clean.Reviewed twice by Codex before opening (the four findings above plus two residual ones on the first fix, all addressed).
Scope and deviations
The three PR branches are merged unchanged (merge commits, then one fix commit); the only conflict was two bullets added to the same spot in
docs/README.md, both kept. The Codex threads open on #1499 were addressed by its own follow-up commits (df1b7e2e,96ad1806) and verified here. Not done: square and smartlead catalog entries (SSE-only, a product/transport question per #1495); the catalogue is deployment-wide while the board is scoped, so no tag counts are shown (docs/TASK-TAGS.md).docs/TASK-TAGS.md) added, if this ships a featuredocs/adr/, if this adds, weakens or reverses an invariant — none touched🤖 Generated with Claude Code