docs: define Harnesses setup and global agent defaults - #272
Conversation
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49000442ca
ℹ️ 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".
| See [configuration parity](configuration.md) for development routing, release | ||
| flag exclusions and the supported deployment boundary. | ||
|
|
||
| ## Planned: Harnesses and agent defaults |
There was a problem hiding this comment.
Add the missing Signed-off-by trailer
Commit 07b867794b37755edcac0b84a7a7b7cfbb1c7b25 has no Signed-off-by trailer, so it violates the repository's per-commit DCO requirement and the hosted DCO Check will reject this head. Recreate the commit with a sign-off from the actual author and verify the check at the resulting head.
AGENTS.md reference: AGENTS.md:L153-L163
Useful? React with 👍 / 👎.
| creation; changing it later does not switch existing agents. Provider, model, | ||
| effort and environment defaults are **looked up at each start** only for fields | ||
| an agent leaves blank; per-agent values win. Per-agent effort is not yet an | ||
| editable field. Changing the default harness clears the default model and effort. |
There was a problem hiding this comment.
Clear the provider when changing the default harness
When a default provider is set and the user changes the default harness—for example, from Goose to Pi—this contract clears only model and effort. Provider choices are harness-specific elsewhere in this document, so the stale Goose provider remains eligible for inheritance by blank Pi agents and can make their next launch invalid or select the wrong backend. Clear or revalidate the default provider during this transition as well.
AGENTS.md reference: AGENTS.md:L83-L86
Useful? React with 👍 / 👎.
| Saving an agent or defaults restarts **running agents whose effective settings | ||
| changed** through the native supervisor and reports **“Saved. Restarted N | ||
| agents.”** Unchanged and stopped agents are not restarted. Effective settings | ||
| include inherited defaults, so today's `restartDiff` (raw saved configs) is not | ||
| enough on its own. This supersedes the current Save-without-restart rule. |
There was a problem hiding this comment.
Define recovery for failed save-triggered restarts
If saving defaults persists successfully but one of the affected agents fails to restart—for example because its CLI disappeared or its newly inherited configuration is invalid—the operation is now partially complete and the documented success message is inapplicable. The contract does not say whether other agents continue restarting, how the saved-but-stopped agent is reported, or how the user retries without re-saving; specify partial-success and recovery behavior before this becomes the implementation contract.
AGENTS.md reference: AGENTS.md:L83-L90
Useful? React with 👍 / 👎.
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord’s automated source review via Wes’s GitHub account.
One P3 documentation correction, detailed inline: the newly stated restart requirement for discovering installed harnesses does not match the existing native-snapshot/Agents-refresh path. No production code changes are requested by this review.
Reviewed head 49000442cab1c59e748cf0d463395e7042faa7a6 against base/merge-base 6cff43b6fb5fc0d100ed6215a4b587d63ab6c6d6: two Markdown files, 85 additions / 18 deletions. The proposed Harnesses and defaults behavior is explicitly labelled planned, not implemented. The native ownership, write-only environment boundary, copied creation-time harness, inherited launch settings and effective-change restart distinction otherwise read consistently with the stated plan and current owners. This review does not approve new product scope or certify the later implementation slices.
Validation: independently checked the changed documents’ 10 relative links/Markdown anchors and git diff --check against these pins; both passed. Relevant native snapshot, executable detection, controller Save/default resolution and editor callers were inspected as source. Rocket’s independent detection review is reconciled; the existing visible/ready five-second refresh is included in the finding. No PR code, install command, app, tests or builds were executed. The PR description’s local validation is reported at 615e96c, not the reviewed tip; it is not treated as a current-head rerun.
CI gap: one current-head snapshot of run 36170267440 shows JavaScript and CI required failed. The JavaScript log reports 3 failed / 3,933 passed tests: PluginImport.test.tsx (expected enabled-update text missing), ProfileAgentIdentity.test.tsx (expected 2 reads, received 3), and MessageRow.test.tsx (undefined messages.report). The six corresponding test/component files are unchanged from the pinned base, but this review did not run a matched base or establish complete failure causation; do not interpret the docs review as CI-green readiness. All four browser shards, measurements, Rust/tool integration, DCO, Semgrep and zizmor passed; Windows was skipped. CI checked a synthetic merge of this head into a7a572745a2a64cc96bb3d44afdbd3ff303eee46, not just the feature tip.
Native installation, credential/sign-in preservation, persistence/permissions, restart failure recovery and packaged/cross-platform acceptance remain future runtime checks. Non-blocking COMMENT only; no approval or merge authorization.
| This is the approved Settings → Agents contract for the next implementation | ||
| slices, not a description of controls already shipped. The current desktop still | ||
| requires a restart to discover newly installed CLIs, and Save currently leaves | ||
| running agents unchanged. Individual-agent configuration stays on the Agents |
There was a problem hiding this comment.
[P3] Describe the existing refresh-based harness detection accurately
The new sentence says the current desktop requires a restart to discover newly installed CLIs, but the existing call path already re-detects them: agent_control_snapshot calls Host::snapshot(), Snapshot::from() calls harness_options() on every snapshot, and installed() checks the filesystem rather than a startup cache (src-tauri/src/agents.rs:25–33,114–119,222–225,376–379; crates/agent-controller/src/runtime.rs:232–247). The mounted Agents UI refreshes every five seconds while visible/ready (src/features/agents/control-react.ts:11–20), and AgentSettingsFields.tsx:85–88 passes the refreshed options into the harness selector.
Thus installing Goose or the Pi executables in an already searched directory can update availability without an app restart today. A changed process PATH is a separate limitation, not a blanket restart requirement. Please distinguish the planned dedicated Check again/setup UI from the existing snapshot-based detection, and align the edited Pi fallback wording below. The scope can stay documentation-only; no new detection owner is needed to correct this claim.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No confirmed blocking defect in this documentation-only slice; one product decision remains for slice 4. Comment only, not approval. Reviewed 49000442cab1c59e748cf0d463395e7042faa7a6 against base/merge-base 6cff43b6fb5fc0d100ed6215a4b587d63ab6c6d6.
The Harnesses/defaults ownership matches the plan. The individual-agent Save rule needs an explicit decision: the plan says the old “Save never restarts” rule must go, but the requested old-Buzz parity is not immediate restart for ordinary runtime edits. Old Buzz saves those edits for a later spawn, with an opt-out/idle-gated restart policy; access-policy edits are a separate immediate-restart exception (save handler, idle policy). Please confirm immediate restart versus deferred idle-gated restart before implementing this sentence in slice 4. I am not treating the ambiguous approved wording as proof of an unauthorized product change.
Two useful slice-4 clarifications in the existing Codex threads: clear/revalidate an incompatible default provider when changing harness, and distinguish persisted defaults from best-effort restart outcomes, showing failures and manual recovery. Neither is an implemented runtime defect in this PR.
I independently verified the existing P3 discovery correction: native snapshots already re-run executable detection, and the mounted Agents UI refreshes them. Reopening is not required for executables installed in an already searched directory. This needs wording correction, not a new detection subsystem.
Validation: both changed Markdown files reviewed; 10 relative links/anchors and git diff --check passed at these pins. No builds, installers or app launches. All three commits in the reviewed range have sign-offs, contradicting the automated missing-trailer comment. Hosted CI 36170267440 remains red: its merge into a7a57274 reports the three stale test failures addressed in #276 (3 failed / 3,933 passed). Native/browser lanes pass. A docs verdict is not CI-green or runtime acceptance of slices 2–5.
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: OpenAI Codex <codex@openai.com>
Signed-off-by: OpenAI Codex <codex@openai.com>
…ad-on-send * origin/main: (58 commits) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) feat(identity): add native macOS import, creation and backup (#308) fix(status): reopen a Today status as Today near 16:00 (#275) test: use current navigation for GIF send roundtrip (#309) Fix composer focus when selecting channels and DMs (#307) fix: retire mention searches after chips and refuted prose (#303) ... # Conflicts: # src/features/messages/MessageComposer.test.tsx # src/features/messages/MessageComposer.tsx
* origin/main: (45 commits) Use Blue 11 links with Blue 3 hover and explicit contrast exceptions (#322) perf(messages): index the emoji catalog for reaction lookups (#333) Polish search palette and add conversation search (#340) Use step-ten avatar colors with contrasting outlines (#320) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) feat(identity): add native macOS import, creation and backup (#308) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentCard.tsx # src/bundled/agents/AgentsPage.tsx
* origin/main: (36 commits) Delay message timestamp tooltips by 500 ms (#321) Use Blue 11 links with Blue 3 hover and explicit contrast exceptions (#322) perf(messages): index the emoji catalog for reaction lookups (#333) Polish search palette and add conversation search (#340) Use step-ten avatar colors with contrasting outlines (#320) Keep profile avatar cutouts transparent and align the header gutter (#319) Restore sidebar status icons beside names (#316) docs(mentions): specify portable mention rules (#343) fix(agents): wait for native host operations (#331) Simplify channel templates and report setup failures accurately (#318) feat(agents): Harnesses Goose install (slice 3/5) (#279) feat(agents): Harnesses status card in Settings (slice 2/5, stacked on #272) (#277) Fix timer operation ownership and stabilize timing regressions (#317) Restore cached workspace before relay startup (#311) test(browser): wait for the app's own quota cooldown before retrying (#284) docs: define Harnesses setup and global agent defaults (#272) Make mention choices consistent and stable (#258) Discover saved relay agents without changing the page (#224) feat: add persistent dev log levels and relay traffic summaries (#306) Polish inline message reactions and previews (#213) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentEditor.tsx # src/bundled/profiles/ProfileAgentIdentity.test.tsx
Stack position
Slice 1/5 of the approved Harnesses stack. Docs only; this PR describes planned behavior, not controls already implemented.
Next slices:
2. Harnesses card: status labels, Check again, ACP tooltip, Pi copy-commands, and Add/Edit setup links to Settings.
3. Goose one-click install: upstream
download_cli.shviacurl | bashwithCONFIGURE=false, serialized installs, re-detection, install log, and restarting waiting agents.4. Global defaults: native store and resolution above
BuildDefaults::resolve(), defaults card, copied creation-time harness, inherited provider/model/effort/env, and restart-on-save for affected running agents.5. Follow-up Pi one-click: managed Node v24, Buzz-owned npm folder, launch PATH.
Design changes
Validation at 623da63
bin/pnpm format:checkpassed (Biome checks source; Markdown is not formatted by this command).git diff --check origin/main...HEADpassed.check-stagedandcheck-pushchecks passed. Global organization push hook passed. The PR diff remains docs only; no source builds or runtime checks were run locally.Integration note
#230 and #241 are merged in the current base; the Goose paragraph incorporates their changes.