Skip to content

Antigravity UI/UX polish: review fixes and phone support - #373

Merged
sambitcreate merged 22 commits into
mainfrom
feature/pr-371-ui-ux-review-6e4d31
Oct 8, 2026
Merged

sambitcreate merged 22 commits into
mainfrom
feature/pr-371-ui-ux-review-6e4d31

Conversation

@sambitcreate

@sambitcreate sambitcreate commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

Follow-up to #371 (now merged into main).

# Finding Fix
1 The disabled Remove runtime button had a title tooltip that never appears (the shared Button sets pointer-events: none when disabled). The reason is visible text, linked with aria-describedby.
2 Install, Cancel and Remove unmount as the state changes, dropping focus to the page body. Escape in the remove confirmation closed the whole dialog. Focus moves to the control that replaced the one the user was on. Escape backs out of the confirmation only.
3 Antigravity activity rows didn't change tense ("Delete file x" while running and when done). Commands read just "Ran". Verb pairs added on the Mac, iOS and Android, with the same expected lines in each client's tests. Commands read "Ran a command"; the command line is still never persisted.
4 Approval cards read "read file needs approval" and "web fetch needs approval" in lowercase. Deletes and moves were shown as edits. Label map extended, sentence-case fallback added, and delete/move approvals get their own card name and summary.
5 A failed status read left "Checking the runtime…" on screen forever. The error is shown with a Try again button.
6 The shared harness UI hard-coded "Google", "dl.google.com" and "download from Google". The publisher comes from the harness definition and the host from the pinned asset; the renderer accepts only a bare hostname.
7 A static, half-opacity full-width bar showed during verify/extract (and the opacity was an inline style). The bar appears only with a real percentage, like the About and sidebar update bars.
8 "Install the runtime above" showed even on unsupported computers, while an update was needed, and mid-install. The hint depends on the runtime state.
9 The live region announced raw phase names ("validating"). It uses the same phase copy as the visible summary.
10 The download size appeared twice. Stated once, in the install disclosure.
11 Install used the neutral variant. accent, as the dialog's primary action.
12a Antigravity had no logo. Google's one-colour glyph as a themed mark on the Mac and iOS; brand-blue monogram on Android.
12b The picker's accessible description broke its "Deployment …" pattern. One shared helper for the tag and description.
12c The schedule warning said "no app default" when the default was Antigravity. It names the default and says why it can't run unattended.
12d The Assistant dock gate in #371 is on useAssistantChat, which no UI renders. Assistant chats still offered Antigravity, and the turn only failed after sending. Assistant and Bot chat pickers omit agent models. An already-selected one blocks sending, with a reason.
Phones A desktop-started Antigravity chat silently switched to the host default model on the phone. iOS and Android show a note above the composer: "This chat used Google Antigravity, which runs only on your Mac. Replies from here use …"

Assets: Google's official Antigravity one-colour glyph, with its viewBox fitted to the glyph so it sits at the same optical size as the other provider marks.

Contracts:

  • AcpHarnessStatus gains optional publisher and runtime.downloadHost. It is desktop IPC only; there is no Aiden Remote protocol change.
  • Phones get no contract change: they mirror the Mac's agent-provider list, as they already mirror the verb map and icon aliases.

Test plan

  • npm run test:acp: 127/127 (after npm run build:worktree-file-io; a stale native helper from before feat: Google Antigravity provider over ACP, with reusable ACP harness primitives #371 fails client-files.test.ts)
  • npx tsc --noEmit, npm run type-check:e2e, eslint on every changed TS file, CI registry test
  • Renderer lane, node scripts/run-ci-tests.mjs --lane renderer: all jobs pass. That covers renderer-other 1956/1956 and the preserved CLI-package, generative-UI Chromium and iOS release-policy suites.
  • runtime-subagents: 2711/2711. core-git: 2519/2520 (see the flake below).
  • Preserved core/runtime suites (browser, terminal coverage, native helpers) were not run locally, because the runner stops at the first failing job. This branch doesn't touch them; hosted CI runs them.
  • CLI package: npm ci && npm run build && npm test, 79/79 (the bundle now reaches renderer/shared/acp-harness.ts through provider-deployment.ts)
  • Android ./gradlew :app:testDebugUnitTest :app:lintDebug :app:compileDebugAndroidTestKotlin: 422/422 unit tests, lint clean
  • iOS AidenChatTests on the iPhone 18 Pro simulator, Xcode 27 beta: 261/261. This is simulator evidence, not device acceptance.
  • Electron e2e tests/e2e/providers-settings.spec.ts, 4/4. The new test drives the real setup dialog with stubbed harness IPC through async progress, failure and completion, and through Escape in the remove confirmation. It fails against the earlier passive-effect focus recovery.
  • After merging main: Android provider logos come from main's VectorDrawable generator (--check reports all 43 up to date), and AidenProviderIconTest, AidenModelPreferenceTest and AidenChatTest pass
  • Visual check of the setup dialog in light and dark themes on a live build

Flake, existing on main and unrelated to this branch: main/services/external-editors.test.ts, "Linux Zed aliases preserve PATH precedence, executable guards and literal workspace argv".

  • Symptom: SyntaxError: Unexpected end of JSON input.
  • Frequency: failed in 2 of 3 local core-git lane runs; passes on its own (16/16).
  • Cause: the test polls until its marker file exists, then parses it, but the fake launcher writes it with a non-atomic writeFileSync.
  • Follow-up: a fix is proposed separately; no timeout or retry change here.

🤖 Generated with Claude Code

CI failures on 8a69115c (run 37821395118)

Each failed job was rerun once, as the repo's rules allow.

  • Electron E2E (2/3): tests/e2e/custom-model-options.spec.ts, "custom GLM effort selector persists and sends every supported thinking mode" (from Add per-model effort settings for custom connections #384).
    • Symptom: locator.click timed out and the radiogroup intercepted pointer events. It failed on both attempts and both retries.
    • Not from this PR: locally it failed 2/10 on the main merge base and 1/10 on this branch.
    • Cause: a test race. The click's scroll-into-view moves the hover-revealed control out from under the pointer, so it collapses.
    • Fix (4044f405): the test focuses the level first. Focus keeps the control open and never changes the level, so the click is still what's tested. 20/20 locally after the fix.
  • iOS build and simulator tests: AidenChatTests.testChatOpenFetchesTheProgressSnapshotOnceAndReloadsDoNotDuplicateIt() failed on the hosted iPhone 17 Pro simulator, with app-launch failures in the same job.

CI failure on 4044f405 (run 37834947647)

  • Android build and APK, "Run Android Compose UI tests": two specs failed with RootViewPicker$RootViewWithoutFocusException, because the emulator window never gained focus:
    • AidenChatProgressUiTest.nestedAgentBackRestoresParentThenRosterAfterRecreation
    • AidenChatProgressUiTest.chatInspectorOwnerRestoresAfterDelayedNegotiationAndRosterHydration
  • Not from this PR: these are the same two specs and symptom feat: Google Antigravity provider over ACP, with reusable ACP harness primitives #371 recorded on its own CI. This PR doesn't touch chat-progress UI. Locally the Android unit suite passes 422/422 and the instrumented tests compile.
  • iOS build and simulator tests (same run): AidenChatTests.testForeignRunResponseStaysVisibleUntilAFailedTranscriptReadRecovers() failed after 63.8 s, with "Failed to launch app" from the hosted simulator in the same job.
    • The failing test differs from the earlier run, but the launch failure is the same.
    • The test is in the class that passed 262/262 locally.
  • Action: after the workflow finished, the failed jobs (Android and iOS) were rerun once.

sambitcreate and others added 17 commits October 7, 2026 12:12
The shared Button sets pointer-events: none when disabled, so the
title tooltip on the disabled "Remove runtime" button never appeared,
and keyboard users could not focus it to find out. The reason is now
visible text next to the button, linked with aria-describedby.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Install, Cancel and Remove each unmount (or are disabled) as soon as
the runtime state changes, which dropped keyboard focus to the page
body in the middle of the setup dialog. Focus now moves to whichever
control replaced the one the user was on: Cancel while installing,
Install or Retry after a cancel or removal, Remove runtime after an
install, or the section itself when nothing is left to act on.

Escape inside the inline remove confirmation now backs out of the
confirmation instead of closing the whole dialog.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…client

Activity that Google Antigravity runs itself (delete_file, move_file,
web_fetch, agent_subagents, agent_tool and the fresh-session notice)
had no verb pair, so rows read "Delete file x" both while running and
when done, unlike "Writing" → "Wrote". The Mac, iOS and Android verb
maps gain the same pairs, and each client's test expects the same lines.

Agent commands carried no detail, so their rows read only "Running" or
"Ran". They now read "Ran a command"; the raw command line is still
never persisted.

The persisted fresh-session label is unchanged so older phone builds,
which show the label as-is, still read naturally.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Approval cards for Antigravity reads, fetches and other tools fell back
to the raw tool name in lowercase ("read file needs approval", "web
fetch needs approval") next to "Edit file needs approval". Deletes and
moves were presented as edits.

- The card label map covers read_file, delete_file, move_file,
  web_fetch and agent_tool, and any unknown tool name now falls back
  to sentence case.
- Permission requests carry whether a file change deletes or moves, so
  the card and its summary say "Delete file" / "wants to delete files".

Phone approval cards use generic copy and the summary line, so they
need no change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
useHarnessStatus swallowed a failed status read, which left the setup
dialog on "Checking the Google Antigravity runtime…" forever. The hook
now reports the error and a retry, and the section shows what went
wrong with a "Try again" button. Any later status broadcast clears the
error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The runtime section is shared by every ACP harness, but its copy
hard-coded "Google's own agent runtime", "dl.google.com" and "download
from Google", and the setup dialog said "runs Google's agent". A second
harness would have shown Google's name and host.

- Harness definitions declare a publisher; Antigravity's is "Google".
- The installer reports the host of the pinned archive for this
  computer, and the status projection carries both to the renderer.
- The renderer accepts only a bare host name and a short publisher, and
  the copy says "its own agent runtime" when either is missing rather
  than guessing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
While verifying, extracting or validating, the runtime section drew a
full-width bar at half opacity (an inline style outside the token
system), which read as a finished download. Like the update bars in
About and the sidebar, the bar now appears only with a real
percentage; the summary line names the current phase.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The setup dialog said "Install the runtime above before signing in"
whenever sign-in was unavailable, including on computers the runtime
does not support (no Install button exists), while an update was needed
(the button says Update), mid-install, and while the status was still
loading. The hint now says what is actually missing in each state, and
nothing while loading.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The polite live region read the raw phase names ("validating",
"activating") while the visible summary said "Starting it once to
confirm it works" and "Finishing". Both now use the same phase copy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Before installing, the summary line ("Not installed. 111 MB download
from Google.") and the install disclosure right below it both gave the
size. The summary now just says "Not installed."; the disclosure keeps
the size, source and free space next to the Install button.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Install (and Update or Retry) is the only step forward in the setup
dialog until the runtime exists, but it used the neutral filled
variant. It now uses the accent variant, as primary actions do
elsewhere (for example "Update and restart" in About).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Antigravity fell back to a letter tile next to real logos in the model
picker, provider settings and onboarding, and to a "G" monogram on the
phones.

- Mac and iOS bundle Google's one-colour Antigravity glyph as a themed
  mark (viewBox fitted to the glyph so it sits at the same optical
  size as the other marks).
- Android, which draws monograms for every provider, uses Antigravity's
  brand blue.
- The iOS test now also checks that every supported slug has a bundled
  logo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The model picker's accessible description reads "Provider X, Deployment
local, Inputs …", but Antigravity entries said "Agent running on this
computer", breaking the pattern. The visible tag and spoken phrase now
come from one shared helper: "· Agent" and "Deployment agent on this
computer".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…edule

When the app default model was Google Antigravity, an unpinned
scheduled task showed "If no app default is available, this task
cannot run." But a default is available and works in chats; it just
can't run unattended. The warning now names the default and says why:
it runs only in chats open on this computer, so pin a provider.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kers

The Assistant dock gate added earlier is on useAssistantChat, which no
UI renders, so Assistant chats still offered Google Antigravity in
their model picker. Picking it failed only after sending ("works only
in ordinary desktop chats, not here").

Assistant and Bot chats now leave agent-backed providers out of the
picker. If a chat already has one selected, the composer says why it
can't send ("Google Antigravity runs only in ordinary desktop chats,
not in Assistant chats. Choose another model.") instead of failing
the turn.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Mac leaves agent-backed providers such as Google Antigravity out of
the phone's model catalog, so opening a chat Antigravity answered on
the Mac silently resolved to the host's default model. A reply from the
phone then went to a different model with no explanation.

iOS and Android now show a note above the composer: "This chat used
Google Antigravity, which runs only on your Mac. Replies from here use
Gemini Flash." The note disappears once the chat is on a model the
phone can use. Bot chats, whose model is fixed, are unaffected. The
agent-provider list mirrors the Mac's ACP_HARNESS_PROVIDER_IDS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@very-hermes-bot very-hermes-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.

Comment thread renderer/components/scheduled-task-editor.tsx Outdated
@very-hermes-bot

very-hermes-bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Hermes Review Bot

Confidence: 5

Engine: agy/gemini-3.8-flash-high
Review mode: full
Head: 4044f40592112446019a89c5ae90b0a9c8eccf08
Generated: 2026-10-08T19:55:24+00:00
Reviews: 1

Summary

Antigravity UI/UX polish resolves accessibility, focus management, and surface-scoping feedback from PR #371, while introducing cross-client mobile parity for desktop-started agent chats on iOS and Android. The setup dialog replaces the disabled button's hidden tooltip with accessible reason text, adds layout-effect focus recovery across dynamic action button unmounts, and traps Escape to dismiss the inline removal confirmation without closing the outer dialog. Agent approvals and timeline activity rows now support verb pairs across desktop and mobile, mask raw command execution strings to protect secrets, and enforce sentence casing for unknown tools. Model selection explicitly excludes agent harnesses from Assistant and Bot chats, informs mobile users when a desktop-only agent model cannot answer from a phone, and guards scheduled task configurations against unattended agent defaults. Maintainers should double-check that the capture-phase Escape keydown listener on window in renderer/components/settings/harness-runtime-section.tsx:160 cleanly tears down when the dialog unmounts or confirmation toggles.

Confidence Score: 5/5

Fully traced across backend handlers, renderer components, focus/keyboard lifecycle, IPC contracts, and native iOS/Android implementations with verified test coverage.

📁 Important Files Changed
  • renderer/components/settings/harness-runtime-section.tsx: Implements layout-effect focus recovery across button unmounts, window-level capture-phase Escape interception for inline removal confirmation, sanitized setup disclosures, and status error retries.
  • renderer/main/chat-pane.tsx: Filters agent-backed providers out of Assistant and Bot chat pickers and blocks sending if an agent model is selected on an unsupported surface.
  • main/services/acp/generation-host.ts & main/services/acp/permissions.ts: Distinguishes file deletion and move operations from edits in approval requests and card summaries.
  • main/services/acp/activity.ts: Masks raw command lines with generic descriptor text to prevent secret exfiltration in persisted activity logs.
  • renderer/shared/acp-harness.ts: Provides validation helpers for harness status parsing, hostname sanitization, surface blocking reasons, and setup hints.
  • renderer/components/chat-approval-card.tsx: Adds explicit tool labels for agent actions and implements sentence-casing fallback for unmapped tools.
  • renderer/lib/agent-steps.ts, ios/AidenOnTheGo/Models/AidenChat.swift, & android/app/src/main/java/sbtbiswas/AidenOnTheGo/models/AidenChat.kt: Defines shared active/completed verb pairs for ACP agent activities.
  • ios/AidenOnTheGo/Features/Remote/AidenChatFeature.swift & android/app/src/main/java/sbtbiswas/AidenOnTheGo/features/chat/AidenChatDetailScreen.kt: Renders informational banner above composer when a desktop-started agent chat is opened on mobile.
  • tests/e2e/providers-settings.spec.ts: E2E test driving agent runtime install, background progress/failure, focus recovery, and inline Escape handling.

Findings

No findings.

Sequence Diagram

sequenceDiagram
    autonumber
    actor User
    participant Dialog as HarnessRuntimeSection
    participant IPC as Main Process (IPC)
    participant Installer as AcpRuntimeInstaller

    User->>Dialog: Press Enter on "Install Google Antigravity"
    Dialog->>IPC: providers:harness:install
    Dialog->>Dialog: Set acting=true; button disables
    IPC->>Installer: Start download
    Installer-->>IPC: Status change: installing (downloading)
    IPC-->>Dialog: providers:harness:changed
    Dialog->>Dialog: useLayoutEffect recovers focus -> "Cancel installation"
    Installer-->>IPC: Download complete (installed)
    IPC-->>Dialog: providers:harness:changed
    Dialog->>Dialog: Cancel unmounts -> useLayoutEffect shifts focus to "Remove runtime"
    User->>Dialog: Press Enter on "Remove runtime"
    Dialog->>Dialog: confirmRemove=true -> focus shifts to "Keep"
    User->>Dialog: Press Escape
    Dialog->>Dialog: Window capture listener intercepts Escape, calls setConfirmRemove(false)
    Dialog->>Dialog: Keep unmounts -> useLayoutEffect restores focus to "Remove runtime"
Loading
[]

Last reviewed commit: 4044f4059211
Reviews (1) · Comment /hermes review to trigger a new review · /hermes review full for full re-review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Two verified P2 issues need addressing before merging.

Reviewed changes Reviewed all 50 changed files and 17 commits at 5a99b01d against stacked base feature/antigravity-aiden-integration-0631d1 (31eb003e): desktop runtime setup, activity/approvals, provider presentation, Assistant/Bot gates, schedule warnings, and native phone notices.

  1. [P2] Runtime focus recovery misses asynchronous transitions at harness-runtime-section.tsx:128-150. With the production section and shared Dialog mounted in Chromium, asynchronous completion or failure while Cancel installation has focus leaves focus on the outer dialog rather than Remove runtime or Retry installation. Radix's MutationObserver redirects focus before the passive effect executes, and its lost predicate then excludes the dialog container. Perform replacement-control focus synchronization before the observer, such as in a layout effect, and add mounted async completion/failure coverage rather than only testing the target mapping.
  2. [P2] Extracted warning copy breaks a registered test at scheduled-task-editor.tsx:615. npx tsx --test renderer/components/scheduled-task-editor.test.tsx reports 2 passed and 1 failed because line 32 still requires No provider pinned. in the component source, although the text now comes from the helper. Replace the obsolete source assertion with rendered warning coverage for absent and Antigravity defaults, not another production-source grep.

Verification: 107 focused TypeScript tests, 91 Android JVM tests, iOS release-policy/asset checks, and 10 CI-registry tests passed. The focus issue was reproduced in Chromium. Native iOS XCTest was not run locally on Linux. Hosted CI was still in progress at the last check.

Inline review publication encountered GitHub API errors, including an internal GraphQL failure with request ID 7808:1E1A50:13C63F1:3EE219B:6AC67AB6; findings are included in the review body to preserve the full feedback.

Pullfrog  | Fix it ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Base automatically changed from feature/antigravity-aiden-integration-0631d1 to main October 7, 2026 21:06
@sambitcreate
sambitcreate enabled auto-merge (squash) October 8, 2026 17:48
sambitcreate and others added 3 commits October 8, 2026 13:49
…review-6e4d31

# Conflicts:
#	android/app/src/main/java/sbtbiswas/AidenOnTheGo/features/shared/AidenProviderIcon.kt
#	android/app/src/test/java/sbtbiswas/AidenOnTheGo/features/shared/AidenProviderIconTest.kt
When an install failed or finished in the background while Cancel had
focus, Radix's focus scope moved focus from the removed button to the
dialog container in a MutationObserver callback. That ran before the
passive effect, which then saw focus on the dialog (not the page body)
and left it there instead of moving to Retry or Remove runtime.

Focus recovery now runs in a layout effect, in the same task as the
DOM commit, and treats focus on any container around the section as
lost. A new Electron e2e drives the real renderer and dialog with
stubbed harness IPC: async progress, async failure and async completion
each land focus on the replacement control, and Escape backs out of
the remove confirmation. It fails against the previous passive effect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…copy

The provider warning's text now comes from scheduledTaskProviderGuardrail,
so the source-grep assertion for "No provider pinned." in
scheduled-task-editor.test.tsx failed. The warning is a small exported
component, and the test renders it for no default, an Antigravity
default, a usable default and a pinned provider. The test also checks
that the warning has no border, ring or outline. This replaces the
guardrail source greps rather than adding more.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@very-hermes-bot very-hermes-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.

Comment thread renderer/main/chat-pane.tsx

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes Reviewed the changes since 5a99b01d, including both prior review fixes and the adaptations after merging main.

  • Moved runtime focus recovery into a layout effect. Verified asynchronous transitions focus Cancel, Retry, and Remove before the dialog's focus scope intervenes, while preserving focus outside the runtime section.
  • Added mounted Electron interaction coverage. The new test exercises install progress, failure, completion, and Escape from the removal confirmation using controllable harness IPC.
  • Replaced the obsolete schedule-warning assertion. The test now renders the production warning for missing and agent-backed defaults, and checks that usable defaults and pinned providers show no warning.
  • Adapted Android Antigravity artwork to the shared logo renderer. Added the generated VectorDrawable, registered its provider mapping, and updated native attribution notices.

Verification passed: 33 focused renderer tests, focused Android JVM suites, all 43 generated Android logo checks, iOS release-policy/artwork checks, desktop and e2e TypeScript checks, and focused lint. A Chromium probe with the production runtime section and shared Radix dialog confirmed the focus fix and Escape behavior. Native iOS XCTest and full Electron e2e were not run on this Linux runner; hosted CI on this head is still incomplete.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

… copy

In an Assistant or Bot chat, a signed-out Google Antigravity selection
showed "needs an API key. Add one in Settings → Providers." The
missing-credential check ran before the agent-surface reason, and
Antigravity signs in to an account rather than taking a key.

The composer now asks acpHarnessReadinessMessage first. For an agent
model it says the chat type can't host it (signing in wouldn't help),
or otherwise asks the user to sign in. Other providers keep the
existing API-key copy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes Reviewed the composer-readiness fix in 8a69115c since the prior review at 69f181ce, with the full PR diff as context.

  • Prioritized agent-surface restrictions. A signed-out Antigravity selection in an Assistant or Bot chat now explains that the chat type cannot host it before offering authentication guidance.
  • Corrected sign-in guidance. Ordinary desktop chats now request Antigravity account sign-in rather than an API key, while other providers retain their existing authentication copy and send-readiness checks.
  • Added behavioral coverage. The shared-helper test checks restriction precedence, signed-out and configured agent states, and preservation of non-agent behavior.

Verification passed: 22 focused harness/runtime/schedule tests, desktop TypeScript checking, and lint on all three incrementally changed files. Electron e2e and native mobile suites were not rerun for this desktop-only delta. Hosted CI on this head is still incomplete; this review does not establish merge readiness.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

"custom GLM effort selector persists and sends every supported thinking
mode" (#384) clicks levels that are revealed only while the control is
hovered or focused. Playwright's click scrolls the target into view,
which can move the group out from under the stationary pointer. The
group then collapses, the radio jumps back, and the click times out
with the radiogroup intercepting pointer events.

The race is on main too: locally it failed 2/10 at the merge base and
1/10 on this branch, and it failed every attempt on CI shard 2 for this
PR. The test now focuses the level first. Focus-within keeps the group
expanded and never changes the level, so the click is still the action
under test. With the fix: 20/20 locally.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes Reviewed 4044f405 since the prior Pullfrog review at 8a69115c, with the complete PR diff as context.

  • Stabilized thinking-level clicks. Added focus and visibility checks before clicking each option, keeping the hover-revealed selector open during scrolling without forcing clicks or changing timeouts or retries.
  • Preserved behavioral coverage. Confirmed that focus alone does not select a level and that the test retains checked-state assertions, request-payload checks for all five configured modes, relaunch persistence, and capability-reset coverage.

Verification passed: e2e TypeScript checking, lint on the changed spec, collection of its three Playwright tests, and seven focused thinking-control/provider-thinking tests. Full Electron execution requires macOS and was not run on this Linux runner. Hosted CI on 4044f405 is still incomplete; this review does not establish merge readiness.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

@sambitcreate
sambitcreate merged commit ca3448b into main Oct 8, 2026
48 of 51 checks passed
@sambitcreate
sambitcreate deleted the feature/pr-371-ui-ux-review-6e4d31 branch October 8, 2026 21:20
sambitcreate added a commit that referenced this pull request Oct 8, 2026
…-upgrades-compare-42da8d

Main claimed Aiden Remote contract revision 25 for the Bots rework (#377), so
the phone simulator viewer moves to revision 26: the constant, fixture,
desktop/iOS/Android revision assertions, the mobile fixture gate, and the
simulator-scoped docs and OpenAPI text. Also fix main's chat-pane, which
still read the removed bot query after #377 and #373 crossed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sambitcreate added a commit that referenced this pull request Oct 8, 2026
#373 made approval tool labels sentence case after #377's Bot chat test was
written, so main's renderer-other lane fails on 'share image'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants