Skip to content

Commit e9409fe

Browse files
badcubanbadcuban
authored
Unify provider sign-in on the hidden flow and match actions to errors (#120)
* Unify provider sign-in on the hidden flow and match actions to errors Every Sign in button (setup card row, composer notices) now drives the same server-side login flow the settings panel uses, through a shared useProviderConnectFlow hook: inline "Signing in" status with the live output line where the user clicked, no thread terminal takeover, and a hand-off to the settings panel (deep-linked to the instance) when a run stalls past the auto-expand threshold or fails while interactive. On the held-send notice, a successful sign-in triggers the existing recheck, so the held message releases itself through the normal preflight gate with no second click. A replay guard keeps an attached surface from treating a finished session from minutes ago as the run the user just started. The provider status notice now earns its actions from the structured snapshot instead of always offering Refresh and Diagnostics: signed out gets Sign in alone, a missing CLI gets Open Settings plus Refresh, disabled gets Open Settings, and probe trouble keeps Refresh plus Diagnostics. URLs in provider status details render as real links via a shared linkifier, so the Claude install address stops being dead text. * Assert the linkifier round-trip on segments instead of stripping tags CodeQL flagged the test's tag-stripping regex as incomplete multi character sanitization. It was never sanitizing, just recovering text content from static markup, but the pure segment splitter's documented invariant (concatenated segments reproduce the input) states the same property directly without the alert-bait pattern. * Open the hand-off terminal as soon as the flow is active Screenshot verification caught the settings hand-off restarting the stall timer: a user who already waited out the threshold in chat arrived at settings and waited it out again before the terminal appeared. The ?instance= deep link now threads through the provider card to the login flow, which opens its terminal immediately while the session is active.
1 parent 6d80a0c commit e9409fe

22 files changed

Lines changed: 1375 additions & 219 deletions

‎apps/web/src/components/ChatView.browser.tsx‎

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3761,6 +3761,23 @@ describe("ChatView timeline estimator parity (full app)", () => {
37613761
}
37623762
});
37633763

3764+
/**
3765+
* Push one provider-auth event down the same subscription the composer
3766+
* notice attaches to when it starts a sign-in.
3767+
*/
3768+
function emitProviderAuthEvent(
3769+
event:
3770+
| { type: "command"; flow: string; command: string }
3771+
| { type: "output"; data: string }
3772+
| { type: "status"; status: string; exitCode: number | null; detail: string | null },
3773+
) {
3774+
rpcHarness.emitStreamValue(WS_METHODS.providerAuthSubscribe, {
3775+
instanceId: "codex",
3776+
createdAt: new Date().toISOString(),
3777+
...event,
3778+
});
3779+
}
3780+
37643781
async function mountSignedOutProviderSend(options: {
37653782
/** Providers the recheck behind "I've signed in" resolves with. */
37663783
refreshedProviders: (signedOut: ServerProvider) => ReadonlyArray<ServerProvider>;
@@ -3785,6 +3802,11 @@ describe("ChatView timeline estimator parity (full app)", () => {
37853802
if (body._tag === ORCHESTRATION_WS_METHODS.dispatchCommand) {
37863803
return { sequence: fixture.snapshot.snapshotSequence + 1 };
37873804
}
3805+
// `providerAuth.start` succeeds with void; the harness's default `{}`
3806+
// would fail the response decode and surface as a start error.
3807+
if (body._tag === WS_METHODS.providerAuthStart) {
3808+
return null;
3809+
}
37883810
if (body._tag === WS_METHODS.serverRefreshProviders) {
37893811
return {
37903812
providers: encodeServerConfig({
@@ -3817,9 +3839,106 @@ describe("ChatView timeline estimator parity (full app)", () => {
38173839
"Explain this repo",
38183840
);
38193841

3820-
return { confirmSignedIn, mounted, turnStartRequests };
3842+
const providerAuthStartRequests = () =>
3843+
wsRequests.filter((request) => request._tag === WS_METHODS.providerAuthStart);
3844+
3845+
return { confirmSignedIn, mounted, providerAuthStartRequests, turnStartRequests };
38213846
}
38223847

3848+
it("signs in from the held-send notice and releases the message without a second click", async () => {
3849+
const { mounted, providerAuthStartRequests, turnStartRequests } =
3850+
await mountSignedOutProviderSend({
3851+
refreshedProviders: (signedOut) => [
3852+
{ ...signedOut, status: "ready", auth: { status: "authenticated" } },
3853+
],
3854+
});
3855+
3856+
try {
3857+
(await waitForButtonByText("Sign in")).click();
3858+
3859+
// The login runs in the server's own PTY, never in the thread's terminal.
3860+
await vi.waitFor(
3861+
() => {
3862+
expect(providerAuthStartRequests()).toHaveLength(1);
3863+
},
3864+
{ timeout: 8_000, interval: 16 },
3865+
);
3866+
expect(providerAuthStartRequests()[0]).toMatchObject({ instanceId: "codex", flow: "login" });
3867+
3868+
emitProviderAuthEvent({ type: "command", flow: "login", command: "codex login" });
3869+
emitProviderAuthEvent({ type: "status", status: "running", exitCode: null, detail: null });
3870+
emitProviderAuthEvent({ type: "output", data: "Opening browser to complete sign-in\r\n" });
3871+
3872+
await vi.waitFor(
3873+
() => {
3874+
expect(document.body.textContent).toContain(
3875+
"Signing in… Opening browser to complete sign-in",
3876+
);
3877+
},
3878+
{ timeout: 8_000, interval: 16 },
3879+
);
3880+
3881+
emitProviderAuthEvent({ type: "status", status: "succeeded", exitCode: 0, detail: null });
3882+
3883+
await vi.waitFor(
3884+
() => {
3885+
expect(turnStartRequests()).toHaveLength(1);
3886+
},
3887+
{ timeout: 8_000, interval: 16 },
3888+
);
3889+
// The success recheck is not a bypass, and it fires exactly once.
3890+
await waitForLayout();
3891+
expect(turnStartRequests()).toHaveLength(1);
3892+
} finally {
3893+
await mounted.cleanup();
3894+
}
3895+
});
3896+
3897+
it("keeps the held message and shows the last line when the sign-in fails", async () => {
3898+
const { mounted, providerAuthStartRequests, turnStartRequests } =
3899+
await mountSignedOutProviderSend({
3900+
refreshedProviders: (signedOut) => [signedOut],
3901+
});
3902+
3903+
try {
3904+
(await waitForButtonByText("Sign in")).click();
3905+
3906+
await vi.waitFor(
3907+
() => {
3908+
expect(providerAuthStartRequests()).toHaveLength(1);
3909+
},
3910+
{ timeout: 8_000, interval: 16 },
3911+
);
3912+
3913+
emitProviderAuthEvent({ type: "command", flow: "login", command: "codex login" });
3914+
emitProviderAuthEvent({ type: "status", status: "running", exitCode: null, detail: null });
3915+
emitProviderAuthEvent({ type: "output", data: "error: could not reach auth.openai.com\r\n" });
3916+
emitProviderAuthEvent({
3917+
type: "status",
3918+
status: "failed",
3919+
exitCode: 1,
3920+
detail: "codex login exited with code 1.",
3921+
});
3922+
3923+
await vi.waitFor(
3924+
() => {
3925+
expect(document.body.textContent).toContain(
3926+
"Sign-in failed. codex login exited with code 1.",
3927+
);
3928+
},
3929+
{ timeout: 8_000, interval: 16 },
3930+
);
3931+
expect(turnStartRequests()).toHaveLength(0);
3932+
// The action comes back so the user can try again, and the draft is intact.
3933+
await expect.element(page.getByRole("button", { name: "Sign in" })).toBeVisible();
3934+
expect(useComposerDraftStore.getState().draftsByThreadKey[THREAD_KEY]?.prompt).toBe(
3935+
"Explain this repo",
3936+
);
3937+
} finally {
3938+
await mounted.cleanup();
3939+
}
3940+
});
3941+
38233942
it("sends the held message once the recheck behind I've signed in comes back clean", async () => {
38243943
const { confirmSignedIn, mounted, turnStartRequests } = await mountSignedOutProviderSend({
38253944
refreshedProviders: (signedOut) => [
@@ -3859,7 +3978,7 @@ describe("ChatView timeline estimator parity (full app)", () => {
38593978
{ timeout: 8_000, interval: 16 },
38603979
);
38613980
expect(turnStartRequests()).toHaveLength(0);
3862-
expect(document.body.textContent).toContain("The terminal shows where the sign-in stopped.");
3981+
expect(document.body.textContent).toContain("The last sign-in did not complete.");
38633982
expect(useComposerDraftStore.getState().draftsByThreadKey[THREAD_KEY]?.prompt).toBe(
38643983
"Explain this repo",
38653984
);

‎apps/web/src/components/ChatView.tsx‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,8 @@ import {
217217
} from "./chat/providerStatusNotice";
218218
import { useSessionStartupNotice } from "./chat/sessionStartupNotice";
219219
import { buildProviderSendPreflightNotice } from "./chat/providerReadinessNotice";
220+
import { toProviderSignInFlowView } from "./chat/providerSignIn";
221+
import { useProviderConnectFlow } from "./settings/useProviderConnectFlow";
220222
import { buildThreadErrorNotice } from "./chat/threadErrorNotice";
221223
import { type ComposerNotice, selectComposerNotices } from "./chat/composerNotices";
222224
import {
@@ -2494,6 +2496,33 @@ export default function ChatView(props: ChatViewProps) {
24942496
// The recheck runs the ordinary send path, which is rebuilt every render.
24952497
// Holding it behind a ref keeps the notice itself stable.
24962498
const confirmProviderSignedInRef = useRef<() => void>(() => {});
2499+
// One sign-in flow serves every composer notice: the held-send row and the
2500+
// provider-status row are mutually suppressed, and both speak about the
2501+
// instance the composer would send to.
2502+
const composerSignInInstanceId =
2503+
providerSendPreflight?.instanceId ?? activeProviderStatus?.instanceId ?? null;
2504+
const hasHeldSendRef = useRef(false);
2505+
hasHeldSendRef.current = providerSendPreflight !== null;
2506+
const composerSignInController = useProviderConnectFlow({
2507+
instanceId: composerSignInInstanceId,
2508+
flow: "login",
2509+
// A held message is waiting on exactly this: re-probe and, if the provider
2510+
// agrees, send it. Without this the user would sign in and then still have
2511+
// to click "I've signed in".
2512+
onSucceeded: () => {
2513+
if (hasHeldSendRef.current) {
2514+
confirmProviderSignedInRef.current();
2515+
}
2516+
},
2517+
});
2518+
const composerSignInView = useMemo(
2519+
() =>
2520+
toProviderSignInFlowView({
2521+
instanceId: composerSignInInstanceId,
2522+
controller: composerSignInController,
2523+
}),
2524+
[composerSignInController, composerSignInInstanceId],
2525+
);
24972526
useEffect(() => {
24982527
// The notice belongs to the send it interrupted, so it must not follow the
24992528
// user into another thread.
@@ -3410,14 +3439,6 @@ export default function ChatView(props: ChatViewProps) {
34103439
projectCwd={firstRunProject?.cwd ?? null}
34113440
projectEnvironmentId={firstRunProject?.environmentId ?? environmentId}
34123441
isOnlyWorkspaceProject={firstRunWorkspaceProjects.length === 1}
3413-
onSignIn={(row) => {
3414-
if (!row.signInCommand) return;
3415-
void runProviderAuthReconnect({
3416-
provider: row.driverKind,
3417-
command: row.signInCommand,
3418-
message: `${row.name} is not signed in.`,
3419-
});
3420-
}}
34213442
onChooseProject={() => useCommandPaletteStore.getState().openAddProject()}
34223443
onSkip={dismissFirstRunSetupForEnvironment}
34233444
onStart={() => {
@@ -3432,7 +3453,6 @@ export default function ChatView(props: ChatViewProps) {
34323453
firstRunProject,
34333454
firstRunWorkspaceProjects.length,
34343455
providerInstanceEntries,
3435-
runProviderAuthReconnect,
34363456
scheduleComposerFocus,
34373457
showFirstRunSetupCard,
34383458
]);
@@ -5240,6 +5260,7 @@ export default function ChatView(props: ChatViewProps) {
52405260
const providerStatusNotice = useProviderStatusNotice({
52415261
status: activeProviderStatus,
52425262
activeTurnInProgress,
5263+
signIn: composerSignInView,
52435264
// The held-send notice and the setup card each already state this
52445265
// provider's problem with the actions that fix it; a second ambient row
52455266
// saying it again is the stacking noise the dock exists to end.
@@ -5262,15 +5283,15 @@ export default function ChatView(props: ChatViewProps) {
52625283
usageReset: threadErrorUsageResetAction,
52635284
retry: threadErrorRetryAction,
52645285
providerLabel: activeProviderLabel,
5265-
onRunAuthReconnect: runProviderAuthReconnect,
5286+
signIn: composerSignInView,
52665287
onDismiss: () => setThreadError(activeThread?.id ?? null, null),
52675288
}),
52685289
[
52695290
activeProviderLabel,
52705291
activeThread?.error,
52715292
activeThread?.id,
5293+
composerSignInView,
52725294
providerAuthReconnectPrompt,
5273-
runProviderAuthReconnect,
52745295
setThreadError,
52755296
threadErrorNoticeVisible,
52765297
threadErrorRetryAction,
@@ -5284,13 +5305,7 @@ export default function ChatView(props: ChatViewProps) {
52845305
prompt: providerSendPreflight,
52855306
recheckFailed: providerSendPreflightRecheckFailed,
52865307
isRechecking: isRecheckingProviderSendPreflight,
5287-
onRunSignIn: (prompt) => {
5288-
void runProviderAuthReconnect({
5289-
provider: prompt.provider,
5290-
command: prompt.command ?? "",
5291-
message: `${prompt.providerLabel} is not signed in.`,
5292-
});
5293-
},
5308+
signIn: composerSignInView,
52945309
onConfirmSignedIn: () => confirmProviderSignedInRef.current(),
52955310
onDismiss: () => {
52965311
setProviderSendPreflight(null);
@@ -5299,10 +5314,10 @@ export default function ChatView(props: ChatViewProps) {
52995314
})
53005315
: null,
53015316
[
5317+
composerSignInView,
53025318
isRecheckingProviderSendPreflight,
53035319
providerSendPreflight,
53045320
providerSendPreflightRecheckFailed,
5305-
runProviderAuthReconnect,
53065321
],
53075322
);
53085323
const composerNotices = useMemo(

0 commit comments

Comments
 (0)