Uh oh!
There was an error while loading. Please reload this page.
Add direct Goose Agent Mode - #607
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds Agent Mode across the Tauri backend and frontend, including streaming sessions, permissions, proxy coordination, feature-flagged navigation, account cleanup, and desktop lifecycle handling. It also updates Rust toolchains, CI configuration, environment variables, and pre-commit toolchain checks. ChangesAgent Mode runtime and desktop integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant AgentMode
participant ProxyService
participant TauriAgentRuntime
User->>Sidebar: Select Agent Mode
Sidebar->>AgentMode: Navigate to /agent
AgentMode->>ProxyService: Ensure Agent proxy is ready
ProxyService->>TauriAgentRuntime: Start or reconcile proxy
AgentMode->>TauriAgentRuntime: Start runtime and send message
TauriAgentRuntime->>AgentMode: Stream timeline and permission events
AgentMode->>User: Render session timeline
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying maple with |
| Latest commit: | 4a133bd |
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7ea83396.maple-ca8.pages.dev |
| Branch Preview URL: | https://codex-goose-sdk-maple.maple-ca8.pages.dev |
e1a638b to
fe73a02Compareb8b52f2 to
0537f99Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
frontend/src/routeTree.gen.ts (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConfigure route generation to preserve import conventions.
This generated import uses a relative path and single quotes. Configure the route generator to emit
@/routes/agentwith double quotes; a manual edit will be overwritten on regeneration. As per coding guidelines, “Use path aliases (@/*maps to./src/*) for imports in TypeScript/React code” and “Use 2-space indentation, double quotes”.🤖 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/routeTree.gen.ts` at line 24, Configure the route generator responsible for routeTree.gen.ts to emit aliased imports using "`@/routes/agent`" and double quotes, rather than manually editing the generated file; update its formatting/import settings and regenerate the route tree to verify the convention persists.Source: Coding guidelines
frontend/src/services/agentModels.test.ts (1)
3-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the configured path alias for this service import.
Replace the relative import with
@/services/agentModels. As per coding guidelines, “Use path aliases (@/*maps to./src/*) for imports in TypeScript/React.”Proposed fix
-} from "./agentModels";+} from "`@/services/agentModels`";🤖 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/services/agentModels.test.ts` around lines 3 - 8, Update the import in the agentModels test to use the configured "`@/services/agentModels`" path alias instead of the relative "./agentModels" path, preserving the existing imported symbols.Source: Coding guidelines
frontend/src/services/agentModels.ts (1)
3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse camelCase for these exported constants.
DEFAULT_AGENT_MODELandPRIMARY_AGENT_MODEL_IDSviolate the TypeScript naming rule here; rename them and update the references infrontend/src/components/AgentMode.tsxandfrontend/src/services/agentModels.test.ts.🤖 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/services/agentModels.ts` around lines 3 - 4, Rename the exported constants DEFAULT_AGENT_MODEL and PRIMARY_AGENT_MODEL_IDS in agentModels.ts to camelCase names, then update all references in AgentMode.tsx and agentModels.test.ts to use the new identifiers consistently.Source: Coding guidelines
frontend/src/services/flags.test.ts (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the
@alias for the internal import.Proposed fix
-import { FlagsClient, type FlagsFetch } from "./flags";+import { FlagsClient, type FlagsFetch } from "`@/services/flags`";As per coding guidelines, “Use path aliases (
@/*maps to./src/*) for imports in TypeScript/React.”🤖 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/services/flags.test.ts` at line 2, Update the import in the flags test to use the configured @ path alias instead of the relative "./flags" path, while preserving the existing FlagsClient and FlagsFetch imports.Source: Coding guidelines
frontend/src/services/agentTimeline.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the configured TypeScript path alias.
Proposed fix
-import type { AgentTimelineItem } from "./agentRuntimeService";+import type { AgentTimelineItem } from "`@/services/agentRuntimeService`";As per coding guidelines,
frontend/src/**/*.{ts,tsx}must use path aliases (@/*maps to./src/*).🤖 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/services/agentTimeline.ts` at line 1, Replace the relative import in the agentTimeline module with the configured @ path alias, preserving the existing AgentTimelineItem type import from agentRuntimeService.Source: Coding guidelines
frontend/src/services/agentTimeline.test.ts (1)
2-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the configured TypeScript path alias.
Replace the relative service imports with
@/services/....Proposed fix
-import type { AgentTimelineItem } from "./agentRuntimeService";-import { coalesceAdjacentThinkingItems, hasRenderableThinkingText } from "./agentTimeline";+import type { AgentTimelineItem } from "`@/services/agentRuntimeService`";+import { coalesceAdjacentThinkingItems, hasRenderableThinkingText } from "`@/services/agentTimeline`";As per coding guidelines,
frontend/src/**/*.{ts,tsx}must use path aliases (@/*maps to./src/*).🤖 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/services/agentTimeline.test.ts` around lines 2 - 3, Update the imports in the agentTimeline test to use the configured `@/services/`... path aliases instead of relative ./ paths, including both agentRuntimeService and agentTimeline imports.Source: Coding guidelines
frontend/src-tauri/src/agent.rs (1)
391-395: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the required Rust logging macros.
Replace these new
log::warn!calls withlog::info!orlog::error!according to severity.As per coding guidelines,
frontend/src-tauri/src/**/*.rsrequireslog::info!andlog::error!for Rust logging.Also applies to: 974-979, 2537-2540
🤖 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-tauri/src/agent.rs` around lines 391 - 395, Replace the `log::warn!` calls in the error-handling branches around the pending permission cancellation and the additionally referenced locations with the required logging macros: use `log::error!` for failed operations and `log::info!` where the condition is informational, preserving each existing message and error context.Source: Coding guidelines
frontend/src/services/proxyService.ts (1)
364-389: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider bounding the backend auth probe with a timeout.
checkProxyBackendAuthawaitsfetch(...)with noAbortController/timeout. If the local proxy accepts the connection but stalls,ensureProxyReadyInner(which is on the serializedenqueueProxyOperationtail) can hang indefinitely, blocking every subsequent proxy operation. A short timeout would fail fast into the existingunknown_error/retry path.Separately, when
!status.runningthis returns"auth_error", which pushes the caller into the key-refresh/revoke branch even though the real cause is a proxy that failed to start. That's a benign misclassification today, but worth confirming it doesn't burn an unnecessary API key rotation.♻️ Example: add an abort timeout
- try {- const response = await fetch(`http://${host}:${status.config.port}/v1/models`);+ try {+ const response = await fetch(`http://${host}:${status.config.port}/v1/models`, {+ signal: AbortSignal.timeout(5000)+ });🤖 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/services/proxyService.ts` around lines 364 - 389, Add a short AbortController-based timeout to the fetch in checkProxyBackendAuth, passing its signal and ensuring the controller is cleaned up so stalled probes fail into the existing unknown_error path. Also verify the !status.running return value does not trigger unnecessary API-key refresh or revocation in ensureProxyReadyInner; adjust the status handling or caller branch if needed to distinguish a stopped backend from an authentication failure.
🤖 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-tauri/src/agent.rs`:
- Around line 1125-1159: Move the autogenerated-title persistence in the setup
flow so it occurs only after configure_session_agent succeeds, or restore the
original session snapshot whenever setup fails. Update the logic around
should_restore_autogenerated_title and configure_session_agent to ensure
provider, mode, or extension setup errors do not leave an empty session renamed.
- Around line 274-281: Replace renderer-controlled account scoping in
account_scope, agent_list_sessions, and agent_load_session with a Rust-tracked
authenticated-account identity; validate the caller’s requested user_id against
that identity or ignore it, and derive the session-manager scope exclusively
from the authenticated account before reopening account_session_manager.
In `@frontend/src/billing/billingService.ts`:
- Around line 89-91: Scope the cached Bearer token to the authenticated account
in BillingService: update updateOpenSecret to compare the verified account
identity on the incoming and current OpenSecretContextType, and clear or replace
the cached token when that identity changes; preserve it only for the same
account. Ensure BillingServiceProvider’s context refresh cannot reuse a token
across account switches, and apply the same identity handling to the token
access logic around the referenced additional lines.
---
Nitpick comments:
In `@frontend/src-tauri/src/agent.rs`:
- Around line 391-395: Replace the `log::warn!` calls in the error-handling
branches around the pending permission cancellation and the additionally
referenced locations with the required logging macros: use `log::error!` for
failed operations and `log::info!` where the condition is informational,
preserving each existing message and error context.
In `@frontend/src/routeTree.gen.ts`:
- Line 24: Configure the route generator responsible for routeTree.gen.ts to
emit aliased imports using "`@/routes/agent`" and double quotes, rather than
manually editing the generated file; update its formatting/import settings and
regenerate the route tree to verify the convention persists.
In `@frontend/src/services/agentModels.test.ts`:
- Around line 3-8: Update the import in the agentModels test to use the
configured "`@/services/agentModels`" path alias instead of the relative
"./agentModels" path, preserving the existing imported symbols.
In `@frontend/src/services/agentModels.ts`:
- Around line 3-4: Rename the exported constants DEFAULT_AGENT_MODEL and
PRIMARY_AGENT_MODEL_IDS in agentModels.ts to camelCase names, then update all
references in AgentMode.tsx and agentModels.test.ts to use the new identifiers
consistently.
In `@frontend/src/services/agentTimeline.test.ts`:
- Around line 2-3: Update the imports in the agentTimeline test to use the
configured `@/services/`... path aliases instead of relative ./ paths, including
both agentRuntimeService and agentTimeline imports.
In `@frontend/src/services/agentTimeline.ts`:
- Line 1: Replace the relative import in the agentTimeline module with the
configured @ path alias, preserving the existing AgentTimelineItem type import
from agentRuntimeService.
In `@frontend/src/services/flags.test.ts`:
- Line 2: Update the import in the flags test to use the configured @ path alias
instead of the relative "./flags" path, while preserving the existing
FlagsClient and FlagsFetch imports.
In `@frontend/src/services/proxyService.ts`:
- Around line 364-389: Add a short AbortController-based timeout to the fetch in
checkProxyBackendAuth, passing its signal and ensuring the controller is cleaned
up so stalled probes fail into the existing unknown_error path. Also verify the
!status.running return value does not trigger unnecessary API-key refresh or
revocation in ensureProxyReadyInner; adjust the status handling or caller branch
if needed to distinguish a stopped backend from an authentication failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19d7ab74-abe1-40c8-88d5-9da5f7b82e89
⛔ Files ignored due to path filters (2)
flake.lockis excluded by!**/*.lockfrontend/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
.githooks/pre-commit.github/workflows/desktop-build.yml.github/workflows/desktop-pr-build.yml.github/workflows/release.yml.github/workflows/rust-tests.ymlREADME.mdflake.nixfrontend/.env.examplefrontend/src-tauri/Cargo.tomlfrontend/src-tauri/capabilities/default.jsonfrontend/src-tauri/src/agent.rsfrontend/src-tauri/src/lib.rsfrontend/src-tauri/src/proxy.rsfrontend/src/billing/billingService.tsfrontend/src/components/AccountMenu.tsxfrontend/src/components/AgentMode.tsxfrontend/src/components/BillingServiceProvider.tsxfrontend/src/components/DeleteAccountDialog.tsxfrontend/src/components/DeleteChatDialog.tsxfrontend/src/components/GuestPaymentWarningDialog.tsxfrontend/src/components/Sidebar.tsxfrontend/src/components/VerificationModal.tsxfrontend/src/components/apikeys/ProxyConfigSection.tsxfrontend/src/routeTree.gen.tsfrontend/src/routes/__root.tsxfrontend/src/routes/agent.tsxfrontend/src/services/agentAuthLifecycle.test.tsfrontend/src/services/agentAuthLifecycle.tsfrontend/src/services/agentModels.test.tsfrontend/src/services/agentModels.tsfrontend/src/services/agentOperationFence.test.tsfrontend/src/services/agentOperationFence.tsfrontend/src/services/agentRuntimeService.tsfrontend/src/services/agentTimeline.test.tsfrontend/src/services/agentTimeline.tsfrontend/src/services/flags.test.tsfrontend/src/services/flags.tsfrontend/src/services/proxyService.test.tsfrontend/src/services/proxyService.tsfrontend/src/vite-env.d.tsscripts/ci/_common.sh
| fn account_scope(user_id: &str) -> Result<String, String> { | ||
| let user_id = user_id.trim(); | ||
| if user_id.is_empty() { | ||
| return Err("Agent Mode requires a signed-in account".to_string()); | ||
| } | ||
| let digest = Sha256::digest(user_id.as_bytes()); | ||
| Ok(format!("{digest:x}")) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not trust the renderer-supplied user_id for account scoping.account_scope only hashes the input, and agent_list_sessions / agent_load_session reopen account_session_manager from that scope when no runtime is active, so a Tauri caller can point at another locally stored account by passing a different ID. Derive the scope from a Rust-tracked authenticated account instead.
🤖 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-tauri/src/agent.rs` around lines 274 - 281, Replace
renderer-controlled account scoping in account_scope, agent_list_sessions, and
agent_load_session with a Rust-tracked authenticated-account identity; validate
the caller’s requested user_id against that identity or ignore it, and derive
the session-manager scope exclusively from the authenticated account before
reopening account_session_manager.
| if should_restore_autogenerated_title { | ||
| session_manager | ||
| .update(&session.id) | ||
| .system_generated_name(prompt_title) | ||
| .apply() | ||
| .await | ||
| .map_err(|e| format!("Failed to name Agent session: {e}"))?; | ||
| session = session_manager | ||
| .get_session(&session.id, false) | ||
| .await | ||
| .map_err(|e| format!("Failed to load named Goose session: {e}"))?; | ||
| emit_agent_event( | ||
| &app_handle, | ||
| AgentEventEnvelope { | ||
| event_type: "sessionUpdated".to_string(), | ||
| session_id: Some(session.id.clone()), | ||
| run_id: Some(run_id.clone()), | ||
| item: None, | ||
| status: None, | ||
| session: Some(session_summary(&session)), | ||
| message: None, | ||
| }, | ||
| ); | ||
| } | ||
| let agent = configure_session_agent(&agent_manager, &session, &model, &mode).await?; | ||
| Ok((agent, turn_snapshot)) | ||
| } | ||
| .await; | ||
| let (agent, task_turn_snapshot) = match setup_result { | ||
| Ok(setup) => setup, | ||
| Err(error) => { | ||
| agent_manager | ||
| .unregister_cancel_token(&request.session_id) | ||
| .await; | ||
| return Err(error); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not rename the session before fallible agent setup.
Line 1127 persists the generated title before configure_session_agent. If provider, mode, or extension setup fails, the error path only unregisters the token, leaving an empty session renamed for a prompt that never ran. Configure first, or restore the snapshot on setup failure.
🤖 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-tauri/src/agent.rs` around lines 1125 - 1159, Move the
autogenerated-title persistence in the setup flow so it occurs only after
configure_session_agent succeeds, or restore the original session snapshot
whenever setup fails. Update the logic around should_restore_autogenerated_title
and configure_session_agent to ensure provider, mode, or extension setup errors
do not leave an empty session renamed.
| updateOpenSecret(os: OpenSecretContextType): void { | ||
| this.os = os; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Scope the cached billing token to the authenticated account.
updateOpenSecret swaps contexts but preserves the global token. After an account switch, the next request can successfully use the previous account’s Bearer token, exposing or operating on its billing state. Key the token by verified account identity, or clear it only when that identity changes.
Based on supplied frontend/src/billing/billingApi.ts:83-109 and frontend/src/components/BillingServiceProvider.tsx:4-13, the token is a Bearer credential and the singleton is refreshed during render.
Also applies to: 256-257
🤖 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/billing/billingService.ts` around lines 89 - 91, Scope the
cached Bearer token to the authenticated account in BillingService: update
updateOpenSecret to compare the verified account identity on the incoming and
current OpenSecretContextType, and clear or replace the cached token when that
identity changes; preserve it only for the same account. Ensure
BillingServiceProvider’s context refresh cannot reuse a token across account
switches, and apply the same identity handling to the token access logic around
the referenced additional lines.
Uh oh!
There was an error while loading. Please reload this page.
Summary
maple-proxywith account-scoped credential handling and the released image request-body limit needed byread_imagemaple-proxyresponses through unchanged at the application wrapper boundaryagent_modeWhy
Tinfoil's former GLM serving path could emit multiple tool-call fragments with the same index in one SSE event. Maple temporarily merged those fragments in
proxy.rs; the Tinfoil GLM update now emits a Goose-compatible stream, so retaining an application-specific stream parser would add latency and maintenance risk without providing value. Kimi remains available on a best-effort basis until its provider path receives the same compatibility work.The image-loading fix is independent and remains at the proxy layer: Goose's base64 image requests can exceed Axum's old default, so
maple-proxy0.1.10 and Maple's proxy invariant test are preserved without repeating a large-image GUI test in every launch cycle.The
agent_modelookup uses OS Flags' public user endpoint without credentials. PR/local builds useflags-dev.opensecret.cloud, release builds useflags.opensecret.cloud, missing keys are off, and failed requests are not cached. The rest of Maple renders immediately; enabled desktop users see the Agent Mode entry after the asynchronous lookup. The route/runtime remains unchanged because this is a rollout gate, not an authorization boundary.Validation
nix develop .. -c bun run test— 51 passed, including 7 flags-client testsnix develop .. -c bun run typechecknix develop .. -c bun run format:checknix develop .. -c bun run lint— 0 errors; only existing unrelated warningsnix develop .. -c bun run buildnix develop ../.. -c cargo fmt --checknix develop ../.. -c cargo clippy --all-targets --all-features --locked -- -D warningsnix develop ../.. -c cargo test --all-targets --all-features— 60 passednix flake checkNotes
b7eb1e9735833a7bf12ab92994a788fbc770f218(Goose 1.42.0).0537f99fis the squashed, validated Agent Mode baseline.550c5ceadds generic flags infrastructure;2dd2833adds only theagent_modeentrypoint gate.agent_mode, so the shipped behavior remains safely off until the flag is created and rolled out.Summary by CodeRabbit