Skip to content

contributor-relay: recognise and dismiss the agy onboarding wizard - #5661

Closed
hanthor wants to merge 1 commit into
hivecommons:v4from
tuna-os:fix/agy-onboarding
Closed

contributor-relay: recognise and dismiss the agy onboarding wizard#5661
hanthor wants to merge 1 commit into
hivecommons:v4from
tuna-os:fix/agy-onboarding

Conversation

@hanthor

@hanthor hanthor commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem

agy is a first-class backend throughout contributor-relay.sh — launch
flags, the --effort requirement, capability reporting — but getCLIState()
has no branch for it. An Antigravity pane always falls through to
'unknown', so the relay waits out CLI_READY_TIMEOUT_MS and hands the task
back, with the CLI sitting on a dismissible dialog the entire time.

Antigravity gates first run behind a three-step wizard:

  1. Choose your color scheme:[Next]
  2. Terms of Service & Data Use[Previous] [Done]
  3. Do you trust the contents of this project?Yes, I trust this folder

This is not a once-per-image cost. Agents that share a $HOME symlink a
single ~/.gemini, so the moment one agent writes
antigravity-cli/cache/onboarding.json mode 600, every other agent gets
EACCES and re-enters the wizard from step 1. On our fleet that recurred
constantly and had to be cleared by hand.

Changes

getCLIState() gains an agy branch — wizard → onboarding,
not signed in / Select login methodneeds-login, banner/prompt →
ready.

blockingPromptKey() learns the Terms of Service step. It is the one page
a bare Enter cannot leave: focus sits on the consent checkbox, where Enter
toggles rather than advances (the footer literally reads enter Toggle), so
the existing bare-Enter path dismisses in a loop until timeout. Down moves to
the button row and Right selects [Done]; the caller already appends Enter,
and tmux send-keys takes the sequence as-is.

The colour-scheme and folder-trust steps do advance on a bare Enter and
deliberately return null, matching the function's existing contract.

Verification

Both functions exercised against pane text captured from a live fleet:

blockingPromptKey        getCLIState
  theme  -> null           login  -> needs-login
  tos    -> "Down Right"   theme  -> onboarding
  trust  -> null           tos    -> onboarding
  codex  -> "1"            trust  -> onboarding
                           ready  -> ready

codex's existing prompts are unaffected. node --check clean.

Scope

Deliberately narrow. It does not attempt to fix the underlying shared-$HOME
permission race — that is an operator-side concern (we normalise the tree to
group-writable on a timer) and arguably wants umask 007 at launch, which is a
separate conversation.

`agy` is a first-class backend throughout this file (launch flags, --effort
handling, capability reporting) but getCLIState() has no branch for it, so an
Antigravity pane always fell through to 'unknown'. The relay then waited out
CLI_READY_TIMEOUT_MS and handed the task back — with the CLI sitting on a
dismissible dialog the whole time.

Antigravity gates first run behind a three-step wizard: colour scheme -> Terms
of Service -> folder trust. This is not a once-per-image cost: agents that
share a $HOME symlink one ~/.gemini, and whenever one writes
antigravity-cli/cache/onboarding.json mode 600 every OTHER agent gets EACCES
and re-enters the wizard from the start.

Two changes:

- getCLIState() gains an `agy` branch mapping the wizard to 'onboarding', the
  "not signed in / Select login method" screen to 'needs-login', and the
  banner/prompt to 'ready'.

- blockingPromptKey() learns the Terms of Service step. It is the one page a
  bare Enter cannot leave: focus sits on the consent CHECKBOX, where Enter
  toggles rather than advances ("enter Toggle"), so the relay would dismiss in
  a loop until timeout. Down moves to the button row and Right selects [Done];
  the caller appends Enter. The colour-scheme and folder-trust steps DO advance
  on a bare Enter and deliberately return null.

Verified against real captured panes from a live fleet: all five agy states
classify correctly, and codex's existing prompts are unaffected.

Signed-off-by: James Reilly <jreilly1821@gmail.com>
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Sep 2, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign clubanderson for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow kubestellar-prow Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 2, 2026

@clubanderson clubanderson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review — CONCERNS. The problem is real and the wizard analysis (Down Right for the ToS checkbox focus) is convincing, but the change as structured has one blocking issue and two gaps.

1. (blocking) getCLIState() already has an agy branch — this PR adds a second one that shadows it. At the bottom of the same else if chain there is an existing branch:

} else if (BACKEND === 'agy') {
  // agy shows "? for shortcuts" at the bottom when its interactive prompt
  // is ready. The generic />\s*$/ fires too early (during the splash).
  if (/\? for shortcuts/.test(text)) return 'ready';
}

The new branch sits earlier in the chain, so the existing one becomes unreachable dead code — and the new ready pattern /^>\s*$|❯|Antigravity CLI/m reintroduces exactly the failure that comment warns about: > fires during the splash, and "Antigravity CLI" is splash-banner text, so the relay can type the task prompt into a splash or a partially drawn wizard frame ( is also the wizard's selection cursor — any wizard variant whose heading isn't matched by the onboarding regex classifies ready). Suggested fix: fold the needs-login/onboarding checks into the existing branch and keep ? for shortcuts as the sole ready test. The login/onboarding-before-ready ordering inside the new branch is correct per the bob/codex precedent — just keep the tight ready gate.

2. No tests. bin/contributor-relay.test.js already pins getCLIState/blockingPromptKey in both directions for codex (onboarding panes classify as onboarding, trust → '1', non-matching text → null). This change wants the same: pane fixtures for the three wizard steps, login, and the real ready chrome, plus blockingPromptKey returning 'Down Right' for the ToS page and null for theme/trust — and a negative case showing ordinary pane text containing "Terms of Service" without the [Done] button row stays null. The verification table in the description is exactly the fixture set; committing it prevents regression.

3. (minor) The ToS check in blockingPromptKey is not backend-gated. It is only reached when the pane already classified onboarding (called from waitForCLI), which bounds the blast radius, but the Down Right sequence is wrong for any non-agy onboarding pane that happens to contain both strings. Gating on BACKEND === 'agy' (as the codex-specific checks are effectively gated by their exact prompt text) would remove the collision class entirely.

Happy to re-review quickly once the branch is consolidated — the shared-$HOME EACCES scoping note is appreciated and the narrow scope is right.

@clubanderson

Copy link
Copy Markdown
Member

Re-review — CONCERNS stand unchanged. No commits since the earlier review (head is still 4332a0a), so briefly restating rather than re-litigating:

  1. (blocking) getCLIState() still gains a second BACKEND === 'agy' branch earlier in the chain, making the existing agy branch — the deliberately tight ? for shortcuts ready gate — unreachable dead code. The new ready pattern (/^>\s*$|❯|Antigravity CLI/m) reintroduces the premature-ready failure that gate exists to prevent: > and the banner text match the splash, and is also the wizard's selection cursor. Fold the needs-login/onboarding checks into the existing branch and keep ? for shortcuts as the sole ready test.
  2. Still no tests. bin/contributor-relay.test.js already pins getCLIState/blockingPromptKey for codex; the verification table in this PR's description is exactly the fixture set — committing it (three wizard panes, login, real ready chrome, Down Right for the ToS page, null negatives) is what prevents regression.
  3. (minor) The ToS Down Right check in blockingPromptKey remains ungated on BACKEND === 'agy'.

The underlying problem and the shared-$HOME EACCES analysis remain solid — this flips to OK as soon as the branch is consolidated and the fixtures land.

@clubanderson clubanderson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks — the agy backend is documented in src/docs/contributor-relay.md, and the new strings line up with the Antigravity onboarding surfaces already modeled elsewhere (Do you trust the contents of this project, I trust this folder, Terms/Data Use, color scheme). I can't approve this version yet because I found one fixable false-positive path.

Finding: getCLIState() scans the whole captured pane for agy onboarding strings before readiness. If a previous legitimate task output quotes Terms of Service & Data Use, [Done], or I trust this folder and remains visible after a relaunch, the relay can classify a ready agy prompt as onboarding and send Enter/Down Right to the live session. The ready match on Antigravity CLI is also too broad for splash/banner text. I tested a local fix that scopes agy login/onboarding/ready checks to the visible tail, uses the documented ready footer (? for shortcuts), includes both project/directory trust wording, and adds JS regression coverage for stale onboarding prose.

Validation of the local fix: node bin/contributor-relay.test.js and cd src && go build ./... both pass. I attempted to push the fix to tuna-os:hive but do not have permission (403), so posting this as a comment rather than requesting changes.

@kubestellar-hive

Copy link
Copy Markdown
Contributor

[quality] regression-risk note: this PR changes bin/contributor-relay.sh (agy onboarding-wizard detection/dismissal) but doesn't update bin/contributor-relay.test.js. Sibling relay PRs (#5700, #5702, #5660) each pair relay changes with test updates. Suggest adding a test case covering the wizard-detection branch (both the match and the no-match passthrough) so the dismissal heuristic doesn't regress silently.

Advisory only — filed by quality agent (hold-gated mode).

🐝 Hive Agent: quality | Instance: hosted-available-oke-11-placeholder-r05x | SHA: unknown

— hive: agent=quality backend=copilot model=claude-fable-5

@clubanderson clubanderson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-review — CONCERNS still stand. I checked the current head (4332a0a) in a fresh worktree and ran cd src && go build ./... plus node bin/contributor-relay.test.js (267/267 passed). The branch still contains only the original bin/contributor-relay.sh change and has not applied the validated fix from the earlier review.

The blocking structure remains: getCLIState() adds an earlier BACKEND === 'agy' branch, making the existing agy branch with the tight ? for shortcuts ready gate unreachable. The new ready regex (/^>\s*$|❯|Antigravity CLI/m) can classify splash/banner/wizard cursor states as ready. Please fold the login/onboarding detection into the existing agy branch, keep ? for shortcuts as the ready signal, scope onboarding detection to the visible tail, gate the ToS Down Right key on agy, and add relay regression coverage.

@clubanderson

Copy link
Copy Markdown
Member

Adopted and merged via #5753 because maintainer pushes to the fork were not available. The replacement PR preserved hanthor's authored commit, added the validated agy readiness/onboarding fix plus regression tests and changelog, passed CI, and merged to v4.

@clubanderson

Copy link
Copy Markdown
Member

Closed after adoption and merge via #5753.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has signed the DCO. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants