feat(team): an agent can be opened, read, and edited (#264)

[M3gA-Mind]

Summary

Fixes #264.

An agent on the Team tab was a dead end. The card rendered a name and a role, the only action behind its menu was a destructive Remove, and none of what an agent is was reachable once it existed: not the description and instructions it was defined with, not which tools it may use, not which desks it belongs to, not its tier, and no edit at all. The "Define an agent" dialog is create-only, so the roster was write-once per member and iterating on an agent's instructions meant deleting it and starting over.

The tool grants were worse than unreachable from the console: they had no read surface anywhere. GET …/team carried no tools for any member, so what a company actually grants an agent could not be checked from outside the process. That is the verification gap the issue names, and it is closed here.

This adds GET/PATCH {scope}/team/{agentId} and the console screen that uses them.

The detail read

Identity (id, name, role), tier, isOrchestrator, the description as authored, desk membership with the lead flag, inbox state, the budget fields, and the tool grants.

isOrchestrator is not tier === "orchestrator", and the difference is load-bearing. The roster rule is "the first agent tagged tier = "orchestrator", else the first agent declared". A company that tags nobody still has an orchestrator. A console that read the tier string alone would label every teammate on such a roster a Worker and be wrong about all of them, including the one actually running the company. The host resolves it and sends the answer; the UI renders what it is told.

To send it from the default build, two rules moved out of #[cfg(feature = "openhuman")]:

• runtime::builder::agent_effective_grants was gated because build_roster was its only caller. It is now also the detail route's grant resolver, and that route ships in the default build.
• The orchestrator resolution moved to company::types::orchestrator_id, with harness::orchestrator::orchestrator_id delegating to it.

Both are single-source-of-truth moves rather than conveniences: a second copy would eventually disagree, and a tool-grant readout that disagrees with the harness is worse than none — it would show an operator a tool list the agent does not hold, or hide one it does.

The three tools lists, and which one the UI shows

tools carries requested, companyAllow and effective:

field	meaning
requested	the agent's own [[agent]].tools globs. Empty means the company's standard grant, not "no tools"
companyAllow	the [tools].allow ceiling the request is intersected with
effective	what the agent actually holds

The UI shows effective by default, as the primary list of badges. That is the only one of the three that answers the question an operator is asking, which is "what can this agent do". requested on its own reports the opposite of the truth for exactly the agents where it matters most: an agent that lists no tools inherits the whole allow-list, so rendering its empty request would tell the operator their agent is powerless when it holds everything the company allows. And a request the allow-list does not cover is not a grant, however plainly the manifest lists it.

The other two are shown as context around that list, not as peers of it: requested minus effective is rendered as a struck-through "asked for but not granted" line, which is the line an operator checking a tool-grant change is actually looking for and which no surface showed before, and companyAllow is a single footnote line naming the ceiling. When requested is empty the section subtitle says so in words ("this agent lists no tools of its own, so it holds everything the company allows") rather than leaving the operator to infer it from an absence.

What may be edited, and why that is the line

The issue leaves this open, so here is the rule, stated once and enforced in one place: the console edits what the console owns.

• An overlay teammate — defined through "Define an agent", or created by the orchestrator's add_agent — lives on the CompanyRecord this process writes. Its name, role and description are editable. That is the whole of the issue's "write-once per member".
• A manifest teammate is declared in the version-controlled company.toml and is 409 here, with a message naming where the edit belongs. This is the same line the existing remove refusal already draws (MANIFEST_TEAMMATE_DELETE), drawn in the same place for the same reason: the overlay model exists so the runtime never rewrites the blueprint, and a console PATCH that edited it would make the deployed company silently diverge from the file in git.
• The one field that is editable on a manifest teammate is its daily budget — and only because team: an operator must be able to set a teammate's daily budget from the Console, without a redeploy #343 modelled it as a BudgetOverride layered on top rather than as a rewrite. It keeps its own route and is untouched here.
• tier and tools are read-only for both kinds. There is no override layer for either, and inventing one is a policy decision (an operator raising an agent's own tool grants from a browser is a privilege question, not a form field) rather than something to smuggle into a detail view. Named as the follow-up below rather than half-shipped.

The host states the rule; the console does not re-derive it. Every detail response carries an editable list naming the fields PATCH will accept, and the console renders a field read-only exactly when that list omits it. A client-side copy of the rule would drift from what the host accepts and the operator would meet the disagreement as a failed save instead of as a disabled input.

PATCH is a real patch. An absent key leaves that field alone; "description": null clears it. The two must stay apart or every partial save would erase an agent's instructions, which is the single worst thing this route could do. A blank name/role is 400, an unknown teammate 404.

One axum detail worth naming

GET and PATCH are attached to team::router's existing /team/{agent_id} entry rather than merged in as their own router. axum panics when two routers claim the same path, even for disjoint methods — a Router::merge of get(...).patch(...) against the existing delete(remove_member) would take the process down at startup, not fail a request. So the handlers live in a sibling module (ops::team_agent) and are composed as a MethodRouter: team_agent::method_router().delete(remove_member).

Console

#/team/<agentId> is a sub-page, not a modal, through the sub-segment useHashView already supports: an agent is linkable, survives a refresh, and Back returns to the roster. The card's own name is the way in (a whole-card click would swallow the inbox switch and the actions menu, both of which live inside it), plus a "View agent" menu item. Only host-backed cards open — a starter-team card is a local placeholder with no record, so its id would 404 and the detail view would report a teammate that was never removed.

The create dialog and the edit form now render from one definition. AGENT_FIELDS in lib/agent.ts is the single description of the three authored fields, and views/team/AgentFields.tsx renders it for both surfaces, so "Define an agent" and "Edit agent" cannot drift into two sets of labels, placeholders and orderings for one set of values.

API Or Behavior Changes

New routes (both under the two existing scope forms, …/company/… and …/companies/{id}/…):

• GET {scope}/team/{agentId} — one agent in full. Any signed-in member.
• PATCH {scope}/team/{agentId} — edit an overlay teammate's name/role/description. Any signed-in member, matching POST …/team: defining a teammate was never admin-only, so correcting one is not either. Setting a budget still is, on its own route.

No existing route changed shape, and no behaviour changed for anything that does not call the new ones. Two functions were un-feature-gated (agent_effective_grants, and the orchestrator resolution moving to company::types); the harness now delegates to the shared copy and computes the same answers it did before.

The console gains a sub-route (#/team/<agentId>) and a menu item. A host that predates the route 404s, and the console renders "this host can't open an agent yet" rather than claiming the teammate was removed.

Tests

• cargo fmt --all -- --check — N/A - full local build matrix is prohibited on this machine; verified in CI.
• cargo clippy --all-targets -- -D warnings — N/A - full local build matrix is prohibited on this machine; verified in CI.
• cargo build --all-targets — N/A - full local build matrix is prohibited on this machine; verified in CI.
• cargo test — N/A - full local build matrix is prohibited on this machine; verified in CI.

cargo fmt itself cannot resolve a manifest in this checkout (no vendor submodules), so formatting was verified with rustfmt --edition 2024 --check run directly over every touched Rust file: clean.

Rust — 13 route tests in src/server/ops/team_agent.rs, on the wire through the real router:

• the read: tier, description, desks with the lead flag resolved from the effective member order, and an empty desk list rendered as a fact rather than an omission;
• effective_tools_are_the_intersection_not_the_request — an agent requesting a tool the company never allowed holds it not; an agent requesting nothing holds the whole allow-list;
• an_untagged_roster_still_names_an_orchestrator;
• an_operator_added_desk_membership_shows_on_the_agent — resolved through effective_desk_members, not the manifest list;
• the edit: persists and shows up on the roster row too, absent-vs-null, blank name/role refused, whitespace trimmed, manifest teammate 409 with nothing written, unknown id 404 on both verbs, and the editable contract itself.

Console — 12 vitest cases in frontend/test/unit/agent-detail.test.ts on the derivations that fail silently: the patch body (only-changed, never a read-only field, null vs absent for a cleared description), draft validity, the three tool-list readings, and the tier label.

Browser — a new Playwright spec, frontend/test/e2e/agent-detail.spec.ts, walking the exact path the issue describes against the live companies/e2e_harness host and pinned to that manifest: open the CEO card from the roster, read its instructions, assert Orchestrator, assert the effective grants (workspace.read, composio, mcp:*), assert it sits on no desk, assert there is no Edit and the reason is on screen; deep-link to #/team/engineer and assert the Engineering desk; then define an agent through the dialog, open it, edit it, and prove it stuck through a storage-cleared reload before removing it again.

The issue's testing requirements, answered item by item

• "Run the console against a live host and walk the exact path, by hand, in a browser." Not done by hand, and I would rather say so than write "tested". Building a host locally is prohibited on this machine, so the walk runs in the Console E2E CI job, which builds a real host and drives the suite against it. The spec above performs that exact walk, step for step. If a hand-driven pass is wanted before merge, it needs someone who can build the host.
• "Exercise the failure this issue describes, not only the happy path." The old behaviour is what the spec starts from: it begins on the Team tab at the card that used to be a dead end, and the assertions are that the instructions, the tier, the tools and the desks are now reachable from it. The refusal half is exercised too — a blueprint agent offers no Edit and says why, which is the correct form of "you cannot change this here" as distinct from the old "there is nothing here at all".
• "If a Playwright spec covers it, that spec must actually run in a lane that builds it (see ci: the integration suite runs nowhere — feature-gated out of one job, --lib'd out of the other #475)." It does: playwright.config.ts has testDir: "./test/e2e" and the Console E2E job runs npm run e2e over that directory, so a new spec file is picked up without registration. Nothing here is behind capabilities.ts — the routes ship in the default build, which is the feature set that lane's host is built with.
• "State what you ran it against and what you saw." Locally: npm run typecheck, npm run typecheck:unit and npm run typecheck:e2e all clean; the 12 new unit tests pass (vitest run test/unit/agent-detail.test.ts → 12 passed); rustfmt --check clean. Not run locally: the Rust suite and the browser suite, per the prohibition above. The numbers for both come from this PR's CI run.

One pre-existing failure, unrelated and untouched: frontend/test/unit/tour-resume.test.ts fails 10/10 locally with window.localStorage.clear is not a function. I confirmed it fails identically on a clean stash of main, and my local Node is v25.3.0 while CI pins Node 22, so it reads as a local-environment artifact rather than a repo defect. Flagged rather than fixed, since it is neither caused by nor related to this change.

Follow-ups this deliberately does not do

• Editing tier and tools. Read-only here for both kinds of teammate, for the reason given above: there is no override layer, and adding one is a privilege decision that deserves its own issue rather than a form field on a detail view.
• Editing a manifest teammate at all. The daily budget is the precedent for how that could work — an override layered on the record rather than a rewrite of company.toml — and the same shape would extend to instructions if the product wants it. That is a design decision about how far the overlay model should reach, not a gap in this screen.

Documentation

docs/spec/runtime/api.md — both routes added to the route table, with the three-list tools contract, the editable contract, and the manifest-vs-overlay edit rule written out.

While there: that file claimed overlay teammates are "roster-only… no harness Agent is built for them yet", which #71 falsified — build_roster promotes each one into a real CompanyAgent. Corrected, because this PR reports overlay teammates' effective grants and the stale sentence would have contradicted it. (Same species of expired claim as #475, one file over.)

Summary by CodeRabbit

• New Features
  • Added agent detail pages with tier, source, tools, desks, inbox, and budget information.
  • Added deep-link navigation and back navigation for individual agents.
  • Added editing for console-defined agent names, roles, and descriptions, including clearing descriptions.
  • Added API support for retrieving and updating agent details.
  • Added visibility into requested, company-allowed, and effective tool access.
• Bug Fixes
  • Improved orchestrator detection and effective tool-grant resolution.
  • Added clear validation and read-only handling for manifest-defined agents.

---

Update: the compile break, and what stops it recurring

The first push failed both Rust lanes. One root cause, presenting at three sites:

src/company/mod.rs declares mod types; privately and re-exports its items through pub use types::{…}. The new code reached crate::company::types::{ORCHESTRATOR_TIER, orchestrator_id} directly, which is error[E0603]: module 'types' is private, at harness/orchestrator.rs:75, harness/orchestrator.rs:122 and server/ops/team_agent.rs:387.

That is why the two lanes died differently rather than identically: the default Rust lane compiles only the feature-free site (team_agent.rs), while Rust (openhuman, tinycortex) additionally compiles src/harness/, so it hit all three.

The fix is to add both names to the existing pub use types::{…} list and reach them through crate::company, which is how every other consumer of that module already names its items — not to make mod types public. The module is private on purpose: its public surface is the curated re-export list, and widening the module to fix one call site would trade a compile error for an unbounded API.

Then the same mistake was audited for across the whole tree, since one instance of it usually means several:

$ grep -rn "company::types::" src/ | grep -v "^src/company/"
$ echo $?
1

No matches: every reference to that module from outside src/company/ now goes through the re-export. (Inside src/company/ the short types:: path is correct and untouched.)

Worth naming plainly: this is the class of error a local cargo check would have caught in seconds, and it was not run because a full local build matrix fills the shared disk on this machine. The trade is deliberate and the cost is a CI round-trip.

Update: the four review threads

All four CodeRabbit findings were accepted; none were declined.

1. The 404 branch showed the opposite message from the one its own comment promised. The sharpest of the four and a real defect: a host that predates this route 404s the path it does not serve, so mapping every 404 to missing rendered "This agent is no longer on the roster" — precisely the outcome the comment beside it said to avoid. And every non-404 (a dropped connection, a 500) fell into unsupported, telling an operator their host was too old when their network had simply gone away.

   Fixed with a documented classifyFailure helper and a distinct error state. The suggested fix was refined rather than taken as written. It treats "listTeam succeeded" as proof the teammate is gone, but GET …/team is the older route — an out-of-date host answers it perfectly, so its success proves nothing on its own. The question that actually separates the two facts is whether the roster still contains this agent:

   GET …/team	outcome
   lists this agent	the host has the roster but not the detail route → unsupported
   omits this agent	the host serves both and the teammate is gone → missing
   fails too	nothing is reachable, so do not guess → error

2. The empty-tools message was wrong for an agent that asked for nothing. Correct: under a company whose allow-list is empty, an agent that lists no tools also lands in that branch, and it was told "nothing it asked for is covered" when it had asked for nothing. summary.standardGrant now picks the wording.

3. disabled → readOnly on locked fields. Correct on accessibility: a disabled input leaves the tab order, so the very values this screen exists to show would be unreachable by keyboard and awkward to select or copy. readOnly refuses the edit and keeps the value reachable. Two stale doc comments that still said "disabled" were corrected with it. Safe because agentEdits already refuses to send a field the host marks read-only, so the form cannot smuggle one through.

4. Widen the e2e try above the creation step. Correct: the POST lands the moment the dialog is submitted, so a failure in the assertion right after it skipped finally and leaked the teammate onto a host this spec's own header promises to leave as it found it — where the leftover card could also change what card(page, role) matches.

Re-verified locally after these: npm run typecheck, npm run typecheck:e2e and npm run typecheck:unit clean, the 12 unit cases in agent-detail.test.ts still pass, and rustfmt --edition 2024 --check clean on every touched Rust file. Rust compilation and tests remain CI-only.

Update: a second lint behind the first, and why it was fixed by convention rather than by boxing

With the E0603 gone, Build (openhuman, tinycortex) passed — the tree compiles under both feature arms and the new test targets compile — and a single new error surfaced in Clippy, on both lanes:

error: the `Err`-variant returned from this function is very large
  --> src/server/ops/team_agent.rs:297:55
297 | fn trimmed_field(value: Option<&str>, field: &str) -> Result<Option<String>, Response> {
    | the `Err`-variant is at least 128 bytes
  (clippy::result_large_err, denied by -D warnings)

It was hidden behind the resolution error: E0603 aborts before lints run, so the first round could only ever surface one layer. It was also introduced by a review fix in the previous round — collapsing two match arms into ? is what made this helper return a Result at all.

Clippy's own suggestion is to box the Response. That was not taken. The crate already has a convention, and it is not a close call:

shape	occurrences in src/server/ops/
Result<_, ApiError>	34
Result<_, Response> in a non-handler helper	1 — team.rs::load_record, whose Ok is a whole CompanyRecord, so the lint exempts it
-> Option<Response> refusal helpers	team.rs::validate_cap, team.rs::require_roster_teammate
Box<Response> anywhere in src/	0

Boxing would have silenced the lint while leaving this one helper shaped unlike all 34 of its neighbours, and would have introduced the crate's first Box<Response>. The two Option<Response> refusal helpers are in team.rs — the very file this module extends — and exist for exactly this reason: to keep a large error out of a Result.

So trimmed_field returns Result<Option<String>, ApiError> and the caller converts at the boundary. The handler above it keeps Response, legitimately: its Ok is Json<AgentDetailDto>, which is larger still, which is why clippy exempted it and why the log named one function rather than three.

That ApiError clears the threshold is precedent, not hope: daily_spend_samples three functions up in team.rs pairs a 24-byte Ok (Option<Vec<UsageSample>>, the same size as Option<String>) with ApiError and passes this lint on main today. The reasoning is recorded in a doc comment on the function so the shape is not collapsed back later.

Review threads

All four CodeRabbit threads are resolved, each addressed by a code change plus a reply on the thread explaining what was done and why. None was declined. Where the suggested diff was not taken verbatim — the 404 classification, where roster membership rather than roster success is the discriminator — the divergence is written on the thread rather than left for a reader to spot in the diff.

[M3gA-Mind]

@coderabbitai review

[greptile-apps]

M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

[coderabbitai]

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b10d1869-bf80-44d4-85ff-392722eacccc

📥 Commits

Reviewing files that changed from the base of the PR and between f71f903 and 1d5cbb4.

📒 Files selected for processing (4)

• docs/spec/runtime/api.md
• frontend/src/components/app-shell.tsx
• src/company/mod.rs
• src/harness/orchestrator.rs

📝 Walkthrough

Walkthrough

The runtime API now supports agent detail retrieval and overlay-agent edits. The console adds agent detail navigation, resolved tool and desk displays, editable fields, validation, persistence, and backend, unit, and end-to-end coverage.

Changes

Agent detail and editing flow

Layer / File(s)	Summary
Runtime contracts and team route wiring src/company/..., src/harness/orchestrator.rs, src/runtime/builder.rs, src/server/ops/...	Shared orchestrator selection and effective grant resolution support agent details. The team route combines GET, PATCH, and DELETE handlers.
Agent detail and edit API src/server/ops/team_agent.rs, src/server/ops/language.rs, docs/spec/runtime/api.md	The API returns identity, source, tier, tools, desks, inbox, and budget data. Overlay agents support partial edits and description clearing. Manifest and unknown agents return distinct errors.
Frontend agent models and edit helpers frontend/src/api/types.ts, frontend/src/api/client.ts, frontend/src/lib/agent.ts, frontend/test/unit/agent-detail.test.ts	The frontend adds detail and edit DTOs, API methods, draft handling, validation, patch generation, grant summaries, tier labels, and unit tests.
Console detail view and validation frontend/src/components/app-shell.tsx, frontend/src/views/TeamView.tsx, frontend/src/views/team/*, frontend/test/e2e/agent-detail.spec.ts	Team cards open agent detail routes. The detail view displays agent metadata and supports editable overlay fields. End-to-end tests cover deep links, editing, persistence, roster updates, and cleanup.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TeamView
  participant OpenCompanyClient
  participant team_agent
  participant CompanyState
  TeamView->>OpenCompanyClient: Request agent details
  OpenCompanyClient->>team_agent: GET /team/{agent_id}
  team_agent->>CompanyState: Resolve agent, grants, desks, and budget
  CompanyState-->>team_agent: Agent detail data
  team_agent-->>OpenCompanyClient: AgentDetailDto
  OpenCompanyClient-->>TeamView: Display detail view

Loading

Suggested reviewers: oxoxdev

Poem

A rabbit opened the team card wide,
Agent details appeared inside.
Tools and desks came into view,
Names and roles could change there too.
The patch was saved with a hop.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The implementation satisfies issue [#264] with agent detail views, effective tools, desk membership, safe edits, routing, and supporting tests.
Out of Scope Changes check	✅ Passed	The changes support issue [#264] through API, UI, shared resolution logic, documentation, and tests without unrelated scope.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: opening, reading, and editing agents in the Team view.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (6)

frontend/src/lib/agent.ts (1)

27-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Freeze the shared field definition.

AGENT_FIELDS is exported as a mutable array. Both the create dialog and the edit form render from it. A const assertion prevents an accidental mutation from changing both forms at once.

♻️ Proposed change

-export const AGENT_FIELDS: AgentFieldSpec[] = [
+export const AGENT_FIELDS: readonly AgentFieldSpec[] = [
   { key: "name", label: "Name", placeholder: "e.g. Nova", kind: "line" },
   { key: "role", label: "Role", placeholder: "e.g. Growth Marketer", kind: "line" },
   {
     key: "description",
     label: "What they do",
     placeholder: "e.g. Runs paid acquisition and reports on ROAS.",
     kind: "prose",
   },
-];
+] as const;

🤖 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 `@frontend/src/lib/agent.ts` around lines 27 - 36, Update the exported
AGENT_FIELDS definition to use a const assertion, freezing the shared field
array and its entries against accidental mutation while preserving its current
values and usage by the create dialog and edit form.

frontend/src/views/team/AgentDetailView.tsx (2)

63-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse emptyDraft() for the initial draft.

This file already imports from @/lib/agent. The inline literal is a second definition of the blank draft. If AgentDraft gains a field, this literal stops compiling or silently diverges.

♻️ Proposed change

-  const [draft, setDraft] = useState<AgentDraft>({ name: "", role: "", description: "" });
+  const [draft, setDraft] = useState<AgentDraft>(emptyDraft);

Add the import:

 import {
   agentEdits,
   draftFrom,
   draftIsValid,
+  emptyDraft,
   isEditable,
   summarizeGrants,
   tierLabel,
   type AgentDraft,
   type AgentFieldKey,
 } from "`@/lib/agent`";

🤖 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 `@frontend/src/views/team/AgentDetailView.tsx` at line 63, Update the initial
state in the AgentDetailView component to use the existing emptyDraft() helper
from `@/lib/agent` instead of an inline AgentDraft literal, adding the named
import alongside the file’s current agent imports.

---

87-112: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Reset editing when the loaded agent changes.

boot re-runs when agentId, client, or company changes, and it replaces draft with the new agent's values. It does not clear editing. An operator who navigates from one agent to another while the edit form is open lands on the next agent already in edit mode.

The state self-corrects because draft is refreshed from the new response, so no wrong value can be saved. Add setEditing(false) in boot to keep the screen predictable.

♻️ Proposed change

   const boot = useCallback(async () => {
     setLoad("loading");
+    setEditing(false);
     try {

🤖 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 `@frontend/src/views/team/AgentDetailView.tsx` around lines 87 - 112, Update
the agent-loading function boot to call setEditing(false) whenever a newly
loaded agent replaces the current agent and refreshes draft, ensuring navigation
between agents exits edit mode while preserving the existing save behavior.

frontend/src/views/team/AgentFields.tsx (1)

36-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Namespace data-testid with idPrefix as well.

idPrefix exists so two AgentFields can be mounted at once, and it namespaces the DOM ids for exactly that reason. data-testid is not namespaced, so two mounted instances produce duplicate test ids and a Playwright locator can match the wrong field. No call site mounts two today, so this is preventive.

♻️ Proposed change

-                data-testid={`agent-field-${field.key}`}
+                data-testid={`${idPrefix}-field-${field.key}`}

Apply to both the Textarea and the Input branch, and update the matching locators in frontend/test/e2e/agent-detail.spec.ts.

Also applies to: 49-49, 58-58

🤖 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 `@frontend/src/views/team/AgentFields.tsx` at line 36, Namespace each field’s
data-testid with idPrefix in both the Textarea and Input branches of
AgentFields, matching the existing DOM id pattern built from idPrefix and
field.key. Update the corresponding Playwright locators in agent-detail.spec.ts
to use the namespaced test IDs.

frontend/test/unit/agent-detail.test.ts (1)

56-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the third arm of the description contract and a partially editable agent.

EditAgentInput documents three distinct description values: omitted, null, and a string. The suite covers omitted and null. It does not assert that new text is sent as a string.

The read-only test uses a manifest agent with editable: [], so every field is skipped by the same guard. An agent with one editable field would catch a regression in which isEditable is checked for one branch of the if/else if/else chain in agentEdits but not another.

💚 Proposed additional cases

   it("trims, so whitespace alone is not a change", () => {
     const detail = agent();
     expect(agentEdits(detail, { ...draftFrom(detail), name: "  Jamie  " })).toBeNull();
   });
 
+  it("sends new instructions as a string, not as null", () => {
+    const detail = agent();
+    expect(agentEdits(detail, { ...draftFrom(detail), description: "  Owns SEO.  " })).toEqual({
+      description: "Owns SEO.",
+    });
+  });
+
+  it("sends only the editable half when the host allows one field", () => {
+    // The all-locked case below cannot catch a guard that is missing from one
+    // branch of the field loop; this one can.
+    const detail = agent({ editable: ["role"] });
+    expect(
+      agentEdits(detail, { name: "Nope", role: "Head of Growth", description: "Nope" }),
+    ).toEqual({ role: "Head of Growth" });
+  });
+
   it("never sends a field the host says is read-only", () => {

🤖 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 `@frontend/test/unit/agent-detail.test.ts` around lines 56 - 83, Extend the
unit coverage around agentEdits to assert that non-empty trimmed description
text is emitted as a string. Replace or supplement the all-read-only manifest
case with a partially editable agent so one editable field is sent while
read-only fields remain excluded, exercising isEditable across each relevant
branch of agentEdits.

src/server/ops/team_agent.rs (1)

583-870: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a test for the budget and inbox fields on the detail response.

The tests cover identity, tools, desks, orchestrator resolution, editability, and every patch arm. They do not assert inboxEnabled, budgetUsdDaily, spentTodayUsd, budgetSetBy, or budgetSetAtMillis. Those five fields are new contract surface that the console renders. Add one test that sets a cap through PUT …/team/{id}/budget, toggles the inbox through PUT …/team/{id}/inbox, and then asserts the detail read reports both. This also pins the absent-means-uncapped rule, which is easy to break by removing a skip_serializing_if.

As per coding guidelines: "Add focused tests with every behavior change".

🤖 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 `@src/server/ops/team_agent.rs` around lines 583 - 870, Add a focused test near
the existing detail-read tests that creates an agent, sets its daily budget via
the team budget PUT endpoint, enables inbox via the team inbox PUT endpoint,
then fetches the agent detail and asserts inboxEnabled, budgetUsdDaily,
spentTodayUsd, budgetSetBy, and budgetSetAtMillis. Also verify an agent without
a configured budget reports the uncapped representation, preserving absent/null
budget fields rather than serializing a zero value.

Source: Coding guidelines

🤖 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 `@frontend/src/views/team/AgentDetailView.tsx`:
- Around line 281-284: Update the empty-tools rendering in AgentDetailView to
use summary.standardGrant when choosing the message: show the existing “nothing
it asked for is covered” text only when the agent requested tools but none were
granted, and use appropriate no-tools wording when it requested nothing. Keep
the effective-tools empty-state structure unchanged.
- Around line 73-80: Update the AgentDetailView load-state handling so 404
remains "missing", while transport and server failures map to a distinct "error"
state rather than "unsupported"; reserve "unsupported" for hosts that are
confirmed not to support the route, and add the corresponding empty-state
rendering alongside the existing states.

In `@frontend/src/views/team/AgentFields.tsx`:
- Around line 42-59: In AgentFields, replace disabled={locked} with
readOnly={locked} on both the Textarea and Input controls so locked values
remain keyboard-accessible while preventing edits.

In `@frontend/test/e2e/agent-detail.spec.ts`:
- Around line 110-122: Move the try block in the “an agent defined in the
console can be read back and edited” test to begin before the Add member dialog
interaction and creation POST. Keep the existing finally cleanup intact so
failures in creation or the roster assertion still remove the “Detail Spec”
teammate.

---

Nitpick comments:
In `@frontend/src/lib/agent.ts`:
- Around line 27-36: Update the exported AGENT_FIELDS definition to use a const
assertion, freezing the shared field array and its entries against accidental
mutation while preserving its current values and usage by the create dialog and
edit form.

In `@frontend/src/views/team/AgentDetailView.tsx`:
- Line 63: Update the initial state in the AgentDetailView component to use the
existing emptyDraft() helper from `@/lib/agent` instead of an inline AgentDraft
literal, adding the named import alongside the file’s current agent imports.
- Around line 87-112: Update the agent-loading function boot to call
setEditing(false) whenever a newly loaded agent replaces the current agent and
refreshes draft, ensuring navigation between agents exits edit mode while
preserving the existing save behavior.

In `@frontend/src/views/team/AgentFields.tsx`:
- Line 36: Namespace each field’s data-testid with idPrefix in both the Textarea
and Input branches of AgentFields, matching the existing DOM id pattern built
from idPrefix and field.key. Update the corresponding Playwright locators in
agent-detail.spec.ts to use the namespaced test IDs.

In `@frontend/test/unit/agent-detail.test.ts`:
- Around line 56-83: Extend the unit coverage around agentEdits to assert that
non-empty trimmed description text is emitted as a string. Replace or supplement
the all-read-only manifest case with a partially editable agent so one editable
field is sent while read-only fields remain excluded, exercising isEditable
across each relevant branch of agentEdits.

In `@src/server/ops/team_agent.rs`:
- Around line 583-870: Add a focused test near the existing detail-read tests
that creates an agent, sets its daily budget via the team budget PUT endpoint,
enables inbox via the team inbox PUT endpoint, then fetches the agent detail and
asserts inboxEnabled, budgetUsdDaily, spentTodayUsd, budgetSetBy, and
budgetSetAtMillis. Also verify an agent without a configured budget reports the
uncapped representation, preserving absent/null budget fields rather than
serializing a zero value.

🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d54af95b-cc06-40fd-958c-3660e22c4f43

📥 Commits

Reviewing files that changed from the base of the PR and between 25702fe and 8ba826a.

📒 Files selected for processing (17)

• docs/spec/runtime/api.md
• frontend/src/api/client.ts
• frontend/src/api/types.ts
• frontend/src/components/app-shell.tsx
• frontend/src/lib/agent.ts
• frontend/src/views/TeamView.tsx
• frontend/src/views/team/AgentDetailView.tsx
• frontend/src/views/team/AgentFields.tsx
• frontend/test/e2e/agent-detail.spec.ts
• frontend/test/unit/agent-detail.test.ts
• src/company/types.rs
• src/harness/orchestrator.rs
• src/runtime/builder.rs
• src/server/ops/language.rs
• src/server/ops/mod.rs
• src/server/ops/team.rs
• src/server/ops/team_agent.rs

[greptile-apps]

M3gA-Mind has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

[M3gA-Mind]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[M3gA-Mind]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[M3gA-Mind]

@coderabbitai review

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[M3gA-Mind]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.