Show tags on the board and filter by them - #1499
Closed
bradflaugher wants to merge 3 commits into
Closed
bradflaugher wants to merge 3 commits into
bradflaugher wants to merge 3 commits into
Conversation
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 Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ac8828319
ℹ️ 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".
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
4 tasks
bradflaugher
added a commit
that referenced
this pull request
Sep 14, 2026
…Auth pack record (supersedes #1495, #1496, #1499) (#1501) ## What changed, and why One PR that supersedes the three open PRs, merged locally on top of current `main` and 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 and `docs/USER-GUIDES.md` corrected 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 `/mcp` endpoint, 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) and `docs/HOSTED-CONNECTORS-RUNBOOK.md` (what an operator needs before and after Connect, per vendor). Both indexed in `docs/README.md`. **Review fixes on the merged tree** (new here, from a pre-PR review of the combined diff): - The tag catalogue's TTL effect was cancelled by any dashboard reload finishing mid-flight, dropping a valid response and leaving the stamp set, so the dropdown could stay empty for five minutes. Request lifetime is now a generation counter, bumped on fetch start and on deactivation/unmount (which also zeroes the stamp). Three regression tests. - Enter/Space on a focused desktop tag chip bubbled to the row handler, which opened the log viewer and blocked the chip's activation. The row handler now acts only when the row itself is the target. Regression test. - `docs/MCP-CATALOG-STATUS.md` overstated 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 in `CONNECTOR-ONBOARDING.md` and 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/tags` returned the deployment-wide catalogue with counts to any signed-in user; it is now scoped exactly like `ListTasks` (#1082 own-rows rule, `view_tasks` required, SQL filter on `created_by` / `created_by_key_id`), with `TestTagCatalogueScope` and `TestTagCatalogueAuthz`. 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: hidden` and 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` (the `make test` path): 63 packages `ok`, 0 failures; `internal/store` 12.7s, `internal/httpapi` 22.0s, `internal/runner` 7.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 new `sage-intacct` pin). - `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 build` 74/74 pages. - `make ci-e2e-mocked`: 98 passed. - `gitleaks git --log-opts=origin/main..HEAD`: 5 commits scanned, no leaks. - Each of the four new regression tests was confirmed to fail against the code it replaces. - After the docs/catalog follow-up (`61f9b9df`): `go test -tags fleet_host_executor -count=1 ./scripts/ ./internal/clientconfig/...` PASS (docs index, catalog decode and pins); `make lint` clean. - After the tag-catalogue scoping fix: `scripts/go-test.sh --count=1` against Postgres again, 63 packages `ok`, 0 failures; `TestTagCatalogueScope` and `TestTagCatalogueAuthz` print `PASS` (not `SKIP`) under `-v`; `make lint` clean; `make sync-guides` no drift; `go test ./scripts/` PASS; `make ci-web` green (1614 tests, build 74/74). - After the focus-ring and runbook fixes: `make ci-web` green again (1614 tests, 74/74), `make sync-guides` no drift, `go test ./scripts/` PASS, `make lint` clean, oxlint clean. - Each fix was reviewed locally by Codex before pushing. 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`). --- - [x] The title and "What changed, and why" are written for the release notes they become - [x] A design note (`docs/TASK-TAGS.md`) added, if this ships 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) — three related changes consolidated deliberately, see above 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Kristian Yendrek <yendrek.kristian@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
Superseded by #1501, which merges this branch at |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed, and why
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 server-side) — 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.opswhileops + urgentwere applied would be lying about the state of the board.GET /tasks/tags, the catalogue endpoint that already existed. It is refreshed off the ordinary reload cadence but at most once perTAG_CATALOGUE_TTL_MS(5 minutes): it is aGROUP BYover every task's tag array and changes only when somebody retags something, so paying for it every 30s buys nothing — while fetching it once left a tag created later, on a task off the current page, unreachable until a full reload. The window the TTL leaves is closed from the other side: the offered set is the catalogue unioned with the tags on the listed tasks, so a brand-new tag is selectable the moment a task carrying it appears.Three things had to change underneath:
passThroughQueryread only the first value of each parameter. That is right for every single-valued filter and wrong fortag: dropping the second of?tag=a&tag=bwidens the result instead of narrowing it — the one direction a filter must never fail in. It now forwards every value, with single-valued parameters unaffected./api/orchestrator/tasks/tagsdid not exist. The statictagssegment wins over the sibling[taskId]route, so it does not shadowGET /tasks/{id}— the same orderingcmd/fleet/main.gospells out for the Go router.<button>to the enclosing<li>. A chip cannot live inside the card button (see below), so it renders as its sibling; moving the border, radius and background one level out is what keeps the chips inside the visible card.docs/TASK-TAGS.mdis the design note. The Operations Center guide previously 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; the guide now describes the chips, the dropdown, and that tags stack.How you verified it
make ci-web— 153 test files / 1610 tests passed,npm audit0 vulnerabilities, oxlint clean, build compiled.npx tsc --noEmit— clean.npx playwright test --project=mocked— 98 passed.go test -tags fleet_host_executor ./scripts/—ok, including the docs-index and guide-sync checks.p.setforp.append→expected [ 'urgent' ] to deeply equal [ 'ops', 'urgent' ]),.get()/.set()→ 2 failures),stopPropagation→ fails),anyFilterclause → fails),make lintcould not complete locally: the installedgolangci-lintis built against Go 1.25 and refuses this repo'sgo 1.27target. This change touches no Go, and CI runs the real linter.Scope and deviations
Scoped to display and filtering. No schema change and no new Go code —
TaskFilter.Tags, the?tag=parameter andGET /tasks/tagswere all already there and untouched; this PR is the UI path to them plus the plumbing fixes that path needed.The catalogue is deployment-wide while the board shows only your own tasks for a non-admin, so a tag a colleague uses can filter down to nothing. That is the same pre-existing property as the dashboard counters, it is not newly introduced here, and rather than hide it the guide states it. Tag counts are deliberately not shown for the same reason — "ops (12)" above a board showing two of them would be a number that is wrong for most readers.
One pre-existing issue was deliberately not fixed: the desktop
<tr>isrole="button"and already nests real buttons (run now, delete). The phone card's nesting was this PR's to fix because this PR created it; the row's predates it, and chasing it here would widen the diff.docs/TASK-TAGS.mdrecords it as worth revisiting.Deliberately deferred: no tag management from the board (renaming or deleting a tag across tasks), and no tag filter on the Upcoming or Sleeping panels.
🤖 Generated with Claude Code
https://claude.ai/code/session_018qQJHUvRV2FM7Rj2GfUNqg