Test Goose connections and fix Pi test false failures

[salman1993]

Why

Goose model browsing shows available IDs but does not confirm that the selected provider, model, and credentials can complete a request. The agent form already offers this check for Pi; the Pi test also needs to use model settings accepted by the running agent.

What changed

• Show Test connection for Goose once a provider and model are selected.
• Run one goose run --text turn with the effective unsaved draft, including write-only environment overrides, in the agent workspace. Use --no-session, --no-profile, and --max-turns 1 to avoid a saved chat or extension tools. Request a ten-token response with thinking effort off. Require a nonempty assistant text reply in Goose’s bounded JSON output, then report success, failure, or a 30-second timeout without saving the draft.
• Remove Pi Test connection’s forced --thinking off. The Databricks Opus model rejects that setting even though it responds normally with its configured thinking mode.
• Add native IPC and form coverage, including a fail-then-pass Pi regression. Stabilize an existing provider-change test by waiting for the model lookup and popup to finish.

Why goose info --check was replaced

On the configured Databricks provider, goose info --check returned HTTP 400: system: text content blocks must be non-empty. Its direct provider call passes an empty system prompt. A one-turn goose run check succeeds on that same local configuration, including with Buzz's minimal child environment and ten-token limit.

Why a fake Goose key falsely passed

The installed Goose CLI prints an assistant error content block for a 401 Anthropic response but exits 0 and marks JSON metadata.status as completed. Checking the process exit code or metadata alone therefore reports a false success. The native test now checks the assistant content and rejects the error while keeping response data private.

Why the Pi Databricks test was failing

The Pi test forced --thinking off; Databricks returned HTTP 400: "thinking.type.disabled" is not supported for this model. The normal agent does not force this override. The same Pi RPC prompt succeeds when the test respects Pi’s configured thinking setting.

Validation

• Focused Vitest, Rust controller/native tests, TypeScript, Biome, Cargo format, and git diff --check passed locally.
• Repository pre-push suite passed at this head: 620 JS tests across 35 files, plus design-system checks.
• Native IPC test covers the selected draft environment, Goose command arguments, a successful text reply, and an assistant authentication error even when Goose exits 0 and reports completed. The regression fails against the prior implementation and passes with this fix.
• Live CLI reproduction: goose info --check failed with the Databricks 400 above; the replacement one-turn goose run command exited 0 and returned a response using the configured provider. The same command succeeded with Buzz's minimal child environment.
• Live Goose JSON reproduction: a fake Anthropic key returned an assistant error block while the CLI exited 0 and reported completed; a configured Databricks run returned a nonempty assistant text block.
• Live Pi RPC reproduction: the selected Databricks model returned an assistant error with --thinking off and completed successfully without it. The production-context Pi integration test passed using the installed Pi and Databricks model.
• The human author reproduced the Pi false failure in the app before the fix. The updated native code has not yet been tried in the running app.

Try it in the app

Create a Goose agent, select a configured provider and model, enter its API key if needed, then click Test connection. A working setup should show “Connected. The model replied.” Try an invalid key or model to confirm an error appears and the draft remains editable. In particular, edit an existing Goose agent, enter a fake Anthropic key, and confirm the test no longer shows green. Then retry Test connection for the Pi Databricks model shown in the report; it should succeed after restarting the native app from this branch.

This is a draft until the app flow is exercised and the human author confirms it under the repository's PR readiness checklist.

Generated with Codex

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-29T02:56:40.974464Z	fc5324c	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[wesbillman]

Star Lord automated source review (via Wes’s account)

No actionable source-demonstrated findings at this revision. Minimalness 9/10, elegance 9/10, correctness 9/10 for the inspected source; this is not approval or runtime acceptance.

• Head: 928cad0d7b3ba9d19b5bb01c093b05e92e9be441
• Base: 2dd479666ca5bd40166b7e3e79cf3fb0872baa28

Reviewed

• The complete ten-file diff and supported picker → model-request owner → native ticket → controller → Goose process path. The change reuses existing request ownership and UI rather than introducing another lifecycle.
• Saved-revision checks, unsaved/default/write-only environment resolution, effective provider/model precedence, workspace validation, bounded native work, cancellation/disposal and sanitized errors. The independent native review returned no concrete defect and was reconciled against source.
• Success/cancellation and failure → retry focus separately: the Test connection control stays mounted, shared Button preserves focusability while loading, errors use the existing status surface, and retry does not discard the draft. Draft edits retire stale tests; browser lookup waits for cancelled native work to retire. This is source inference, not observed native focus behavior.
• Added IPC success/failure and Goose form coverage, plus the existing shared Pi pending/error/retry/edit-retirement coverage. The synthetic executable checks arguments/environment, not real provider inference.
• Public PR description (no attachments), changed source/docs/tests and both commits’ messages/attribution. No publication-privacy finding in that reviewed surface.

Validation and readiness limits

No PR code, tests, builds or app workflows were executed. The one hosted snapshot of run 36502458977 had JavaScript, browser measurements, DCO and security checks passing; Rust/tool integration and all six browser journey shards were still running. Windows validation was skipped. This is not a current CI-green claim.

Read-only inspection of Goose v1.52.0 supports the --no-profile extension suppression and headless error-propagation assumptions (session builder, session result handling); it does not establish compatibility with every installed Goose version or provider. Actual timeout/process teardown, provider behavior and native keyboard/focus remain unexercised here.

The description explicitly says the changed app flow and human acceptance are outstanding and calls this a draft, whereas GitHub currently marks it non-draft. The reported local suite and live CLI success are author-supplied evidence, not execution by this review. The repository’s app/human readiness checklist therefore remains unverified; this review does not attest buzz-review-completed or merge readiness.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc5324ca0d

ℹ️ 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".

[wesbillman]

Star Lord automated source review (via Wes’s account)

No actionable source-demonstrated findings in this revision. Minimalness 9/10, elegance 9/10, correctness 9/10 for the inspected source. This is a non-blocking source review, not approval or runtime acceptance.

• Head: c317736aebbfaea934fd98a5a5174836a5ab0d86
• Base: ffe932228caee3ee43d9a766b04cd8f67a9a3cc7
• Reviewed immutable, hash-verified source snapshots, not a dirty working tree.

Revision review

This follow-up covers the substantive changes since my review of 928cad0d7b3ba9d19b5bb01c093b05e92e9be441, reconciled with the complete current twelve-file feature diff and its supported callers.

• Goose false-success fix: the bounded JSON reader and reply predicate require a successful process, completed metadata and nonblank assistant text, and reject assistant error content. Exit 0 plus completed metadata alone can no longer pass. The existing cancellation owner, timeout, process cleanup and sanitized UI errors remain in use. Added native IPC fixtures cover a text reply and an authentication error with exit 0/completed metadata.
• Pi false-failure fix: removing the forced --thinking off respects the existing configured arguments rather than overriding the running agent’s setting. The regression assertion checks removal of that override.
• Failure/retry focus, separately from success/cancel: the Test control remains mounted during a test, shared Button keeps it focusable while loading, and a failed test leaves it available for retry without discarding the draft. Exact-draft keys and existing cancellation ownership retire stale results. These are source observations, not observed native keyboard behavior.
• Public publication surface: inspected the description, changed code/docs/tests and all five commits’ messages/attribution; no publication-privacy finding in those materials. No author-provided images were found. The only image reference found in the discussion was the bot’s external P1 SVG badge, which could not be rendered here and is excluded from visual assessment. All five current commits contain Signed-off-by trailers; the bot’s missing-trailer claim is not supported by the current commit data.

Validation and readiness limits

No PR code, tests, builds, provider calls or app workflows were executed by this review. Read-only Goose v1.52.0 session-source inspection supports the JSON output assumptions, not compatibility with every installed version/provider.

The single hosted snapshot of run 36515125334, taken at approximately 03:08 UTC, showed JavaScript, Rust/tool integration, browser measurements, DCO and security checks passing; four browser journey shards passed and four were still running. Windows native validation was skipped. This is not an all-green or final CI claim.

The author reports local tests and live CLI reproductions, but explicitly says the changed native app flow and human acceptance remain outstanding. The description calls this a draft while GitHub marks it non-draft. Actual provider behavior, timeout/process teardown and native focus remain unexercised here; the app/human readiness checklist is unverified. This review does not attest buzz-review-completed or merge readiness.

[wesbillman]

Star Lord automated source review (via Wes’s account)

No new actionable source-demonstrated findings. Minimalness 9/10, elegance 9/10, correctness 9/10 for the inspected source. This is a non-blocking source review, not approval or runtime acceptance.

• Head: d45c6347128473638de91bc60134b4be600db1e5
• Base: c37e83bcba0910af3a67f133aae59d72a0ad3779
• Immutable source snapshot; all 1,711 recorded file hashes rechecked, no dirty working-tree inputs.

Follow-up scope

This revision merges the target branch into previously reviewed c317736aebbfaea934fd98a5a5174836a5ab0d86. I reconciled the 233 changed paths against the target base and the twelve-file feature diff, focusing on merge interactions rather than reopening the earlier review. The Goose test/JSON validation and Pi thinking fix retain their previously reviewed behavior. The overlapping controller/Pi test changes supply the new session-policy fields; the provider-change test now observes the upstream combobox aria-busy completion signal before closing the popup.

The existing request owner still retires stale draft tests and serializes native cancellation. Failure/retry focus was assessed separately from success/cancel: Test remains mounted during a test, the shared Button explicitly stays focusable while loading, and completion re-enables retry without discarding the draft. This is source inference, not observed native focus behavior.

Public-material check: no new privacy finding in the description, changed source/docs/tests or six commit messages/attribution. There are no author-provided image attachments in the fetched description/discussion; the bot’s external P1 SVG badge was not visually assessed. All six PR commits contain Signed-off-by trailers, and hosted DCO passed; the old missing-trailer comment is not an outstanding finding.

Validation limits and unresolved CI

No PR code, tests, builds, provider calls or app workflows were executed. The author’s reported CLI/local-test evidence is not execution by this review; native app/provider compatibility and human acceptance remain unverified.

The 16:56 UTC snapshot of run 36600661178 is not green:

• Rust/tool integration, DCO, Semgrep and zizmor passed. JavaScript and several browser shards were still running; Windows native validation was skipped.
• Browser measurements failed: WebKit scroll.spec.mjs:155 reported a 650-pixel anchor difference against a <4-pixel expectation.
• Chromium and WebKit journey shard 3 failed: both channel-activity-corners cases timed out hovering an invisible channel button.

Those details come from the three completed job logs, not local reproduction. These failing test files are outside this PR’s feature diff; their root causes and attribution remain unresolved. No CI reruns or polling were performed. This review does not attest the app/human readiness checklist or merge readiness.

[wesbillman]

Star Lord automated source review (via Wes’s account)

No new actionable source-demonstrated findings. Minimalness 9/10, elegance 9/10, correctness 9/10 for the inspected source. COMMENT only—not approval or merge readiness.

• Head: 470156261748d60c5424a38cb7a496f1dc83aa4f
• Base: c37e83bcba0910af3a67f133aae59d72a0ad3779
• Pinned source blobs verified against the head tree; no dirty working-tree inputs.

Follow-up

The only source delta since reviewed d45c6347128473638de91bc60134b4be600db1e5 is the six-line responsive-navigation setup.

At the test’s 640px viewport, the shell deliberately hides navigation until its disclosure is opened (docs/shell-design.md:43–48, globals.css:238–260). Opening “Show navigation” when present and awaiting the channel’s visibility establishes the missing precondition before hover. Repeating that setup on each popup opening also accommodates drawer state after Escape. The existing row-count, corner geometry, scrolling, focus and thread-opening assertions remain intact; there are no added sleeps, retries, relaxed assertions or browser cases. Browser geometry remains the appropriate test layer.

The Goose/Pi production code and its success/cancel/error/retry-focus behavior are byte-identical to the prior reviewed head, so this follow-up does not reopen them or claim additional runtime evidence.

Public-material check: no new privacy finding in the current description, feature diff or seven commit messages/attribution. No author-provided image attachments were present in the fetched description/discussion; the bot’s external P1 SVG badge remains outside visual assessment. All seven commits carry Signed-off-by; current hosted DCO passed.

Validation limits

No PR code, tests, builds or app workflows were executed locally. The 17:07 UTC hosted snapshot showed:

• Chromium shard 3 passed: its completed log reports both changed-file cases passing (8.9s and 8.2s), with 75 cases passing across the shard. This is hosted evidence, not a local run or comparative performance claim.
• Browser measurements, Rust/tool integration, DCO and security checks passed.
• Chromium shard 6 failed during browser installation (exit 100), before its tests ran.
• JavaScript and several browser shards, including WebKit shard 3, were still running; Windows native validation was skipped.

Thus the earlier Chromium hover failure is addressed in this hosted run, but both-engine validation is not established by this snapshot and CI is not all green. The native app/provider flow and human acceptance remain unverified. No CI reruns or polling were performed.