fix(web): remove dead controls that teach the app is broken - #95
Conversation
A disabled control is a promise the product cannot keep. Every one of these taught an operator that Muster is half-built before they had done anything. - Drop the "New agent" button. There is no agent-creation API, and an empty affordance is worse than no affordance. - Turn the agent detail "Invoke" button into a link to the operations board, where work can genuinely be created, assigned, and dispatched to an agent. The readiness reason survives as the link title, so the signal stays without the dead end. - Render the organisation as plain text when the actor has one membership, keeping the accessible label. The real select now appears only when a second organisation exists, so the switcher is interactive exactly when it works. - The agent directory lists agents, not "human collaborators". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughUpdates agent directory and detail actions, and enables organisation switching when a session has multiple organisation memberships. ChangesAgent navigation updates
Organisation switching
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/components/agents-view.test.ts`:
- Around line 21-27: Extend the “routes work assignment to the operations board”
test to cover the readiness-dependent title contract: assert the ready-state
prompt and the not-ready title using agent.readiness.reason. Prefer rendering
the component and verifying the resulting title, or add assertions for both
branches while preserving the existing route and label checks.
- Around line 29-33: Update the disabled-attribute scan in the test “keeps every
remaining disabled control tied to live state” to parse JSX attributes
precisely: support whitespace around the assignment, reject boolean literals
such as disabled={true}, and avoid matching unrelated text. Prefer a JSX-aware
parser; otherwise tighten the regex and assertions to enforce these cases.
In `@apps/web/components/os/company-os-shell.tsx`:
- Around line 341-343: Update the organisation selector’s onChange flow to
persist and verify the chosen membership server-side, then refresh or invalidate
organisation-scoped session and data before committing the new UI selection.
Replace the source-only assertion with a behavior test covering selection,
server confirmation, data refresh, and the resulting organisation context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 09e06e24-da80-4cfd-9721-46775e2fa53e
📒 Files selected for processing (4)
apps/web/components/agents-view.test.tsapps/web/components/agents-view.tsxapps/web/components/os/company-os-shell.test.tsapps/web/components/os/company-os-shell.tsx
| describe("Agent detail", () => { | ||
| it("routes work assignment to the operations board", async () => { | ||
| const source = await readFile(viewUrl, "utf8"); | ||
| expect(source).toContain('href="/operations"'); | ||
| expect(source).toContain("Assign work"); | ||
| expect(source).not.toContain("Invoke"); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover the readiness-dependent title contract.
This test checks the route and label but not the new title behavior. A regression could remove either the ready prompt or agent.readiness.reason while the test still passes; assert both branches or render the component and verify the resulting title.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/components/agents-view.test.ts` around lines 21 - 27, Extend the
“routes work assignment to the operations board” test to cover the
readiness-dependent title contract: assert the ready-state prompt and the
not-ready title using agent.readiness.reason. Prefer rendering the component and
verifying the resulting title, or add assertions for both branches while
preserving the existing route and label checks.
| it("keeps every remaining disabled control tied to live state", async () => { | ||
| const source = await readFile(viewUrl, "utf8"); | ||
| for (const match of source.matchAll(/disabled(?:={([^}]*)})?/g)) { | ||
| expect(match[1], "permanently disabled control").toBeTruthy(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Parse disabled JSX attributes more precisely.
The regex accepts disabled={true} as valid, misses valid whitespace such as disabled = {state}, and can match unrelated text. Use a JSX-aware parser or at least reject boolean literals and support whitespace around the assignment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/components/agents-view.test.ts` around lines 29 - 33, Update the
disabled-attribute scan in the test “keeps every remaining disabled control tied
to live state” to parse JSX attributes precisely: support whitespace around the
assignment, reject boolean literals such as disabled={true}, and avoid matching
unrelated text. Prefer a JSX-aware parser; otherwise tighten the regex and
assertions to enforce these cases.
| onChange={(event) => | ||
| setChosenOrganisationId(event.target.value) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Wire organisation selection to an authoritative context switch.
This only updates local UI state. The supplied session builder currently always returns one organisation, so the selector is unreachable; if multi-memberships are later returned, selecting one will change the label while the session and loaded data remain scoped to the previous organisation. Persist and verify the membership change server-side, then refresh/invalidate organisation-scoped data before showing the new selection. Replace the source-only assertion with a behavior test for that flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/components/os/company-os-shell.tsx` around lines 341 - 343, Update
the organisation selector’s onChange flow to persist and verify the chosen
membership server-side, then refresh or invalidate organisation-scoped session
and data before committing the new UI selection. Replace the source-only
assertion with a behavior test covering selection, server confirmation, data
refresh, and the resulting organisation context.
Source: Coding guidelines
1 participant
Phase 3 of 6.
Permanently-disabled buttons train people to believe the product is broken. Three were reachable from the sidebar.
Changes
/agents) — deleted. There is no agent-creation API, so a disabled affordance was pure noise./agents/[id]) — now a working link to/operations, labelled "Assign work". Dispatch became real in9531f9a; this button was a dead end pointing at a live feature. Readiness detail survives as the link title. Navigation is deliberately never blocked: the composer and task drawer already refuse dispatch for a non-ready agent with their own operator-readable reason, so blocking here would duplicate a refusal.<select>only appears with more than one.Tests
apps/web/components/agents-view.test.tsincludes a guard rather than a fixed assertion: it walks everydisabledoccurrence in the file and fails if any lacks an expression, so the next permanently-disabled control is caught at CI rather than by inspection.company-os-shell.test.tspreviously asserted the switcher stays disabled — that assertion was pinning the defect, so it was rewritten.Reviewer note
The multi-org
<select>branch is forward-compat only: itsonChangesets local state and does not switch organisation. It cannot render today (SessionContext.organisationsreturns the current org alone). Worth wiring or removing when multi-org membership lands, so it never becomes the next control that looks live but isn't.Follow-up found, not fixed
AgentDetailViewrenders ten tabs but only branches onlearning— the other eight all fall through toAgentOverview, so eight distinct-looking links show identical content. Same class of defect, silent rather than visible. Fixing it means building those panels or trimming the strip; larger than a dead-control sweep.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes