Skip to content

Commit fc2d930

Browse files
committed
Revert "feat(web): surface AMR sign-in failure reason + recovery (UI + CLI) (#5301)"
This reverts commit e22f9d3.
1 parent e22f9d3 commit fc2d930

34 files changed

Lines changed: 59 additions & 765 deletions

apps/daemon/src/cli.ts

Lines changed: 1 addition & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { splitResearchSubcommand } from './research/cli-args.js';
1616
import { resolveDaemonUrl } from './daemon-url.js';
1717
import { requestJsonIpc } from '@open-design/sidecar';
1818
import { SIDECAR_ENV, SIDECAR_MESSAGES } from '@open-design/sidecar-proto';
19-
import { EXPORT_FORMATS, EXPORT_IMAGE_FORMATS, AMR_LOGIN_FAILURE_SUMMARY } from '@open-design/contracts';
19+
import { EXPORT_FORMATS, EXPORT_IMAGE_FORMATS } from '@open-design/contracts';
2020
import { buildExportCliRequestBody, buildExportCliResultEnvelope, resolveExportCliDeckMode } from './export-cli-request.js';
2121
import { exportRoutePath } from './export-cli-routing.js';
2222
import {
@@ -627,7 +627,6 @@ async function runAmr(args) {
627627
if (!sub || sub === 'help' || args.includes('--help') || args.includes('-h')) {
628628
console.log(`Usage:
629629
od amr status [--refresh] [--json]
630-
od amr login [--json]
631630
632631
Options:
633632
--daemon-url <url> Open Design daemon HTTP base.
@@ -680,109 +679,12 @@ Options:
680679
if (wallet?.error?.message) console.log(`Reason\t${wallet.error.message}`);
681680
return;
682681
}
683-
case 'login':
684-
return runAmrLogin(base, flags);
685682
default:
686683
console.error(`unknown subcommand: od amr ${sub}`);
687684
process.exit(2);
688685
}
689686
}
690687

691-
// Localized reason lives in the web UI; the CLI mirrors the neutral English
692-
// summary from contracts so the same failure vocabulary drives both surfaces.
693-
function amrLoginReasonSummary(failure) {
694-
if (!failure || typeof failure.code !== 'string') return '';
695-
return AMR_LOGIN_FAILURE_SUMMARY[failure.code] ?? '';
696-
}
697-
698-
// Poll the vela status until the sign-in completes, fails, or times out. Mirrors
699-
// the web poll contract (2s interval, 3s startup-settle, 5-min ceiling) so the
700-
// CLI reports the same classified failure the UI would show (issue #426).
701-
const AMR_CLI_LOGIN_POLL_INTERVAL_MS = 2000;
702-
const AMR_CLI_LOGIN_STARTUP_SETTLE_MS = 3000;
703-
const AMR_CLI_LOGIN_TIMEOUT_MS = 5 * 60 * 1000;
704-
705-
async function runAmrLogin(base, flags) {
706-
const loginResp = await fetch(`${base}/api/integrations/vela/login`, {
707-
method: 'POST',
708-
});
709-
// 202 = started, 409 = a sign-in is already in flight on another surface.
710-
// Both are valid: attach to the existing login and poll it (CLI/web parity).
711-
// Only a real start failure (anything else) exits non-zero here.
712-
const alreadyRunning = loginResp.status === 409;
713-
if (!loginResp.ok && !alreadyRunning) {
714-
const body = await loginResp.json().catch(() => null);
715-
const failure = body?.failure ?? null;
716-
if (flags.json) {
717-
process.stdout.write(
718-
JSON.stringify(
719-
{ ok: false, error: body?.error ?? `HTTP ${loginResp.status}`, failure },
720-
null,
721-
2,
722-
) + '\n',
723-
);
724-
} else {
725-
console.error(`Sign-in failed\t${failure?.code ?? 'AMR_LOGIN_UNKNOWN'}`);
726-
const reason = amrLoginReasonSummary(failure) || body?.error;
727-
if (reason) console.error(`Reason\t${reason}`);
728-
}
729-
process.exit(1);
730-
}
731-
732-
if (!flags.json) {
733-
console.log(
734-
alreadyRunning
735-
? 'Sign-in already in progress. Waiting for it to complete…'
736-
: 'Sign-in started. Complete it in your browser…',
737-
);
738-
}
739-
const startedAt = Date.now();
740-
for (;;) {
741-
await new Promise((resolve) => setTimeout(resolve, AMR_CLI_LOGIN_POLL_INTERVAL_MS));
742-
const statusResp = await fetch(`${base}/api/integrations/vela/status`);
743-
if (!statusResp.ok) return structuredHttpFailure(statusResp);
744-
const status = await statusResp.json();
745-
if (status?.loggedIn) {
746-
if (flags.json) {
747-
process.stdout.write(JSON.stringify({ ok: true, status }, null, 2) + '\n');
748-
} else {
749-
const account = status?.user?.email ?? status?.user?.id ?? 'signed in';
750-
console.log(`Signed in\t${account}`);
751-
}
752-
return;
753-
}
754-
// Mirror amrLoginPollOutcome: after the startup-settle grace a login that is
755-
// no longer in flight has terminated ("stopped"); the 5-min ceiling is a
756-
// timeout. Stopped takes priority — waiting for lastLoginFailure alone would
757-
// spin the full ceiling on a canceled exit (which surfaces no failure).
758-
const elapsed = Date.now() - startedAt;
759-
const stopped =
760-
status?.loginInFlight === false && elapsed >= AMR_CLI_LOGIN_STARTUP_SETTLE_MS;
761-
const timedOut = elapsed >= AMR_CLI_LOGIN_TIMEOUT_MS;
762-
if (!stopped && !timedOut) continue;
763-
let failure;
764-
if (stopped) {
765-
failure = status?.lastLoginFailure ?? {
766-
code: 'AMR_LOGIN_INTERRUPTED',
767-
recovery: 'reauth',
768-
};
769-
} else {
770-
failure = { code: 'AMR_LOGIN_TIMEOUT', recovery: 'reauth' };
771-
await fetch(`${base}/api/integrations/vela/login/cancel`, {
772-
method: 'POST',
773-
}).catch(() => {});
774-
}
775-
if (flags.json) {
776-
process.stdout.write(JSON.stringify({ ok: false, failure }, null, 2) + '\n');
777-
} else {
778-
console.error(`Sign-in failed\t${failure.code}`);
779-
const reason = amrLoginReasonSummary(failure);
780-
if (reason) console.error(`Reason\t${reason}`);
781-
}
782-
process.exit(1);
783-
}
784-
}
785-
786688
// ---------------------------------------------------------------------------
787689
// Subcommand: od research …
788690
// ---------------------------------------------------------------------------

apps/web/src/components/AmrLoginPill.tsx

Lines changed: 4 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,6 @@ import {
2929
} from './amrLoginPolling';
3030
import { Icon } from './Icon';
3131
import { amrConsoleUrlForProfile, amrProfileBadgeLabel } from '../runtime/amr-guidance';
32-
import {
33-
amrLoginFailureForOutcome,
34-
amrLoginFailureForSpawn,
35-
amrLoginReasonText,
36-
} from '../runtime/amr-login-failure';
3732

3833
interface AmrLoginPillProps {
3934
className?: string;
@@ -49,12 +44,6 @@ interface AmrLoginPillProps {
4944
revealPendingCancelAction?: boolean;
5045
showConsoleAction?: boolean;
5146
iconOnlySignOut?: boolean;
52-
// Suppress the inline error text inside the compact control. The host lifts
53-
// the reason (via `onErrorChange`) and renders it somewhere with room to
54-
// breathe — e.g. a full-width row under the Settings agent card — so the
55-
// classified reason no longer wraps awkwardly next to the Authorize button.
56-
hideInlineError?: boolean;
57-
onErrorChange?: (message: string | null) => void;
5847
onStatusChange?: (status: VelaLoginStatus | null) => void;
5948
}
6049

@@ -86,9 +75,6 @@ export interface AmrAccountControlProps {
8675
showConsoleAction?: boolean;
8776
consoleUrl?: string;
8877
iconOnlySignOut?: boolean;
89-
// When true, the error status still styles the control (Authorize button) but
90-
// the inline reason text is not rendered; the host shows it elsewhere.
91-
hideInlineError?: boolean;
9278
showCancelSignInAction?: boolean;
9379
// Activation URL surfaced while signing in, so the user can re-open the
9480
// sign-in page when the browser did not auto-open. The URL already carries
@@ -137,7 +123,6 @@ export function AmrAccountControl({
137123
showConsoleAction = false,
138124
consoleUrl,
139125
iconOnlySignOut = false,
140-
hideInlineError = false,
141126
showCancelSignInAction = false,
142127
activationUrl,
143128
browserOpenFailed = false,
@@ -248,7 +233,7 @@ export function AmrAccountControl({
248233
{signInLabel ?? t('settings.amrSignIn')}
249234
</button>
250235
) : null}
251-
{hasError && !hideInlineError ? (
236+
{hasError ? (
252237
<span className="amr-account-control__error" role="alert">
253238
{loginErrorText}
254239
</span>
@@ -293,8 +278,6 @@ export function AmrLoginPill({
293278
revealPendingCancelAction = false,
294279
showConsoleAction = false,
295280
iconOnlySignOut = false,
296-
hideInlineError = false,
297-
onErrorChange,
298281
onStatusChange,
299282
}: AmrLoginPillProps) {
300283
const { t } = useI18n();
@@ -319,23 +302,9 @@ export function AmrLoginPill({
319302
if (next) {
320303
setStatus(next);
321304
onStatusChange?.(next);
322-
// Persist the daemon's classified failure across ordinary reads
323-
// (mount/focus), so a reload after a failed sign-in keeps the specific
324-
// reason instead of resetting to a plain signed-out pill (issue #426).
325-
// Assign unconditionally in the terminal signed-out/not-signing-in state
326-
// so a later clean read (daemon restart drops the in-memory
327-
// lastVelaLoginExit) also CLEARS a previously shown reason. The poll-stop
328-
// branch overrides with the definitive outcome text on the same tick.
329-
if (!next.loggedIn && !next.loginInFlight) {
330-
setErrorMessage(
331-
next.lastLoginFailure
332-
? amrLoginReasonText(t, next.lastLoginFailure)
333-
: null,
334-
);
335-
}
336305
}
337306
return next;
338-
}, [onStatusChange, t]);
307+
}, [onStatusChange]);
339308

340309
useEffect(() => {
341310
if (!skipInitialRefresh) void refresh();
@@ -361,32 +330,8 @@ export function AmrLoginPill({
361330
setPending(null);
362331
setCanceledVisible(false);
363332
}
364-
// NOTE: keep this effect's deps to `[initialStatus, stopPolling]` — no `t`.
365-
// A per-render `t` identity would re-run `setStatus(initialStatus)` on every
366-
// render and clobber a local post-cancel status (loginInFlight:false) back
367-
// to the host's still-in-flight snapshot, bouncing the pill to "Signing in…"
368-
// (#3158). The classified-reason mapping that needs `t` lives in its own
369-
// effect below.
370333
}, [initialStatus, stopPolling]);
371334

372-
// The Settings card mounts this pill with `initialStatus` + `skipInitialRefresh`
373-
// and refetches on window focus, so a host-pushed signed-out snapshot never
374-
// flows through `refresh()`. Mirror refresh()'s terminal mapping here (unless
375-
// the pill's own login is mid-flight) so the classified reason surfaces on
376-
// that surface after reload/focus too — and clears when the daemon no longer
377-
// reports `lastLoginFailure` (restart drops the in-memory exit) (issue #426).
378-
// Split off the status-sync effect (and thus off `setStatus`) so re-running on
379-
// a fresh `t` only ever calls the idempotent setErrorMessage (#3158).
380-
useEffect(() => {
381-
if (!initialStatus || initialStatus.loggedIn) return;
382-
if (initialStatus.loginInFlight || loginPendingRef.current) return;
383-
setErrorMessage(
384-
initialStatus.lastLoginFailure
385-
? amrLoginReasonText(t, initialStatus.lastLoginFailure)
386-
: null,
387-
);
388-
}, [initialStatus, t]);
389-
390335
useEffect(() => {
391336
if (!canceledVisible) return;
392337
const timeout = window.setTimeout(() => {
@@ -399,12 +344,6 @@ export function AmrLoginPill({
399344
onStatusChange?.(status);
400345
}, [onStatusChange, status]);
401346

402-
// Lift the resolved reason so a host that hides the inline error (e.g. the
403-
// Settings agent card) can render it with room to breathe.
404-
useEffect(() => {
405-
onErrorChange?.(errorMessage);
406-
}, [onErrorChange, errorMessage]);
407-
408347
const startPolling = useCallback((startedAt = Date.now()) => {
409348
stopPolling();
410349
loginStartedAtRef.current = startedAt;
@@ -437,9 +376,7 @@ export function AmrLoginPill({
437376
loginStartedAtRef.current = null;
438377
loginPendingRef.current = false;
439378
setPending(null);
440-
setErrorMessage(
441-
amrLoginReasonText(t, amrLoginFailureForOutcome(outcome, next)),
442-
);
379+
setErrorMessage(t('settings.amrLoginErrorCompact'));
443380
}
444381
};
445382
pollRef.current = window.setInterval(() => {
@@ -539,7 +476,7 @@ export function AmrLoginPill({
539476
loginStartedAtRef.current = null;
540477
loginPendingRef.current = false;
541478
setPending(null);
542-
setErrorMessage(amrLoginReasonText(t, amrLoginFailureForSpawn(result)));
479+
setErrorMessage(result.error || t('settings.amrLoginErrorCompact'));
543480
return;
544481
}
545482
notifyAmrLoginStatusChanged('login-started');
@@ -666,7 +603,6 @@ export function AmrLoginPill({
666603
signInLabel={signInLabel}
667604
showConsoleAction={showConsoleAction}
668605
iconOnlySignOut={iconOnlySignOut}
669-
hideInlineError={hideInlineError}
670606
signInDisabled={loginInFlight}
671607
signOutDisabled={logoutInFlight}
672608
showCancelSignInAction={revealPendingCancelAction && loginInFlight}

apps/web/src/components/EntryShell.tsx

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,6 @@ import {
150150
notifyAmrLoginStatusChanged,
151151
} from './amrLoginPolling';
152152
import { closeAmrActivationWindowBestEffort } from './AmrLoginPill';
153-
import {
154-
amrLoginFailureForOutcome,
155-
amrLoginFailureForSpawn,
156-
amrLoginReasonText,
157-
} from '../runtime/amr-login-failure';
158153
import { smoothScrollToTop } from '../utils/smoothScrollToTop';
159154
import { summarizeProjectNameFromPrompt } from '../utils/projectName';
160155
import { LIBRARY_UI_VISIBLE } from '../features/libraryUi';
@@ -1341,29 +1336,15 @@ function OnboardingView({
13411336
let cancelled = false;
13421337
void fetchVelaLoginStatus()
13431338
.then((next) => {
1344-
if (cancelled || !next) return;
1345-
setAmrStatus(next);
1346-
// Surface the daemon's persisted classified failure on the onboarding
1347-
// mount fetch so a reload after a failed sign-in keeps the specific
1348-
// reason instead of resetting to the plain sign-in CTA (issue #426).
1349-
// Assign unconditionally in the terminal signed-out/not-signing-in
1350-
// state so a clean read (daemon restart drops the in-memory
1351-
// lastVelaLoginExit) also CLEARS a previously shown reason.
1352-
if (!next.loggedIn && !next.loginInFlight) {
1353-
setAmrLoginError(
1354-
next.lastLoginFailure
1355-
? amrLoginReasonText(t, next.lastLoginFailure)
1356-
: null,
1357-
);
1358-
}
1339+
if (!cancelled && next) setAmrStatus(next);
13591340
})
13601341
.finally(() => {
13611342
if (!cancelled) setAmrStatusResolved(true);
13621343
});
13631344
return () => {
13641345
cancelled = true;
13651346
};
1366-
}, [t]);
1347+
}, []);
13671348

13681349
useEffect(() => {
13691350
if (runtime === 'amr') return;
@@ -1924,9 +1905,7 @@ function OnboardingView({
19241905
}
19251906
if (!loginResult.ok && !loginResult.alreadyRunning) {
19261907
resolveAmrAuthTracking(analytics.track, 'failed', 'spawn_failed');
1927-
setAmrLoginError(
1928-
amrLoginReasonText(t, amrLoginFailureForSpawn(loginResult)),
1929-
);
1908+
setAmrLoginError(loginResult.error || t('settings.amrLoginErrorCompact'));
19301909
return;
19311910
}
19321911
if (await pollAmrLoginCompletion()) {
@@ -1983,9 +1962,7 @@ function OnboardingView({
19831962
} else {
19841963
resolveAmrAuthTracking(analytics.track, 'failed', 'login_stopped');
19851964
}
1986-
setAmrLoginError(
1987-
amrLoginReasonText(t, amrLoginFailureForOutcome(outcome, nextStatus)),
1988-
);
1965+
setAmrLoginError(t('settings.amrLoginErrorCompact'));
19891966
return false;
19901967
}
19911968
}

0 commit comments

Comments
 (0)