From e06aff0d1ad0cc189de6c58806f01cae416fff87 Mon Sep 17 00:00:00 2001 From: Kenny Daniel Date: Mon, 3 Aug 2026 18:18:41 -0700 Subject: [PATCH 1/3] Login lane returns its outcome; the wizard stops reading its prose (LLP 0179) The wizard classified a failed join by substring-matching three exported English sentences out of the login lane's captured stderr, which made user-facing text load-bearing. remoteLogin() now returns { exitCode, reason }, runRemoteLogin is the exit-code adapter over it, and classifyLoginFailure and printJoinFailure switch on the reason code. The three message constants are module-local again. Output and exit codes are unchanged. --- ...0135-install-experience-overhaul.design.md | 6 + ...login-lane-returns-its-outcome.decision.md | 114 +++++++++++++++ src/core/cli/remote_commands.js | 131 +++++++++++++----- src/core/cli/wizard/index.js | 4 +- src/core/cli/wizard/join.js | 65 +++++---- src/core/cli/wizard/types.d.ts | 23 ++- src/core/remote/types.d.ts | 37 +++++ test/core/cli/wizard/index.test.js | 5 +- test/core/cli/wizard/join.test.js | 45 +++--- test/core/remote-login-command.test.js | 63 ++++++++- 10 files changed, 393 insertions(+), 100 deletions(-) create mode 100644 llp/0179-login-lane-returns-its-outcome.decision.md diff --git a/llp/0135-install-experience-overhaul.design.md b/llp/0135-install-experience-overhaul.design.md index b73c361a..127b5ef7 100644 --- a/llp/0135-install-experience-overhaul.design.md +++ b/llp/0135-install-experience-overhaul.design.md @@ -324,6 +324,12 @@ transient network error) to `'failed' | 'abandoned'` for `runInitWizard`'s retry-or-local-or-quit prompt (`@ref LLP 0129#failed-join-returns-to-fork`). +Extended-by: [LLP 0179](./0179-login-lane-returns-its-outcome.decision.md). +The taxonomy and the classification it feeds are unchanged; how the wizard +*learns* them is not. The implementation read the login lane's captured +stderr and substring-matched the messages it printed; the lane now returns +`{ exitCode, reason }` and `classifyLoginFailure` switches on the code. + ## Pick phase {#pick} `runWizardPick` keeps `walkthrough.js`'s existing shape (prompt, write, guard, diff --git a/llp/0179-login-lane-returns-its-outcome.decision.md b/llp/0179-login-lane-returns-its-outcome.decision.md new file mode 100644 index 00000000..b72c4e04 --- /dev/null +++ b/llp/0179-login-lane-returns-its-outcome.decision.md @@ -0,0 +1,114 @@ +# LLP 0179: The login lane returns its outcome; the wizard stops reading its prose + +**Type:** Decision +**Status:** Accepted +**Systems:** CLI, Onboarding +**Author:** Phil / Claude +**Date:** 2026-08-03 +**Related:** LLP 0058 (the D7 taxonomy), LLP 0134 (the wizard wraps the login lane), LLP 0135 (`classifyLoginFailure`), LLP 0129 (a failed join returns to the fork) + +> The wizard classifies a failed join by substring-matching three +> exported English sentences out of the login lane's captured stderr. +> The login lane knows the refusal code exactly; it just throws it away +> at the command boundary. Return it instead. + +## Context + +[LLP 0134](./0134-wizard-wraps-remote-login.decision.md#login-lane) made +the wizard's team path *be* the login lane rather than a second +enrollment mechanism, and [LLP 0135](./0135-install-experience-overhaul.design.md) +gave it `classifyLoginFailure`, mapping the +[LLP 0058 D7](./0058-oidc-login-client.decision.md#d7) taxonomy to +`'failed' | 'abandoned'` for the fork prompt +([LLP 0129](./0129-init-wizard-fork.decision.md#failed-join-returns-to-fork)). + +Both decisions were about *which mechanism enrolls*. Neither said how +the wizard should learn the outcome, and the implementation took the only +channel the mechanism offered. `runRemoteLogin` is written as a CLI +command: it returns a number and says everything else in prose. So the +wizard tees the login lane's stderr, and `classifyLoginFailure` +substring-matches three exported constants +(`LOGIN_NO_MEMBERSHIP_MESSAGE` and friends) against the captured text. + +That makes user-facing English load-bearing. The sentences cannot be +reworded, localized, or wrapped without silently changing which fork the +wizard offers, and nothing in the type system says so. The information +being reconstructed is not lost or ambiguous: the loopback receiver +attaches the server's exact refusal code to the error as `callbackError`, +`explainLoginError` switches on it one frame below, and then the frame +above collapses it to an exit code. + +## Decision + +**The login lane returns a structured outcome.** +`remoteLogin(argv, ctx, deps)` in `remote_commands.js` returns +`{ exitCode, reason }`, where `reason` is a `LoginOutcomeReason` code, not +a message: `'ok'`, the three D7 refusals (`'no_membership'`, +`'org_not_permitted'`, `'org_selection_required'`), `'denied'` for a +provider denial, `'login_failed'` for a transient or local failure +(timeout, network, an abandoned browser flow), `'usage'` for an argument +or unconfigured-target error, `'connected_elsewhere'` for the LLP 0063 D4 +exclusivity gate, and `'store_failed'` / `'seed_failed'` / +`'enroll_failed'` / `'daemon_incomplete'` for the post-auth steps. Every +`return` in the browser and static paths names one. + +**`runRemoteLogin` stays a `Promise`.** +It is now a one-line adapter over `remoteLogin`. The CLI dispatch table +is uniform over commands that return an exit code, and a command surface +is exactly the place where an outcome *should* collapse to a number; the +seam moves inward rather than up. + +**Prose is for humans, codes are for +control flow.** `classifyLoginFailure` switches on `reason`. The three +message constants stop being exported API and go back to being strings +`explainLoginError` prints. The wizard keeps teeing stderr, because +`WizardJoinResult.detail` echoes the lane's own explanation to the user +([LLP 0135](./0135-install-experience-overhaul.design.md#join)) - that +use is narration, which is what captured prose is good for. + +The classification is unchanged: exactly the three refusals are +`'failed'`; everything else non-zero stays `'abandoned'`. A provider +denial and a timeout remain retriable on purpose - the user can just try +again - so `'denied'` does not join the definitive set. + +## Alternatives considered + +**Extract a pure `enrollWithLogin()` core and reduce the command to +printing.** The tempting shape, and rejected for now: the login lane's +output is interleaved with its work by design, not by accident. The +consent notice must print *before* the browser opens +([LLP 0063 D3](./0063-login-auto-provision-forward-sink.decision.md)), +the first-sync hold message must print after the marker is on disk and +ahead of three different exits ([LLP 0101](./0101-first-sync-review-window.decision.md)), +and the forwarding line's wording is pinned by +[LLP 0100 R1a](./0100-enrollment-privacy-review.spec.md#requirements). +Hoisting all of it into a caller means re-deriving that ordering from a +result object, and the regression it risks is silent. The outcome return +solves the actual problem (a typed answer for the one caller that needs +one) without touching a single message. + +**Have the wizard call `loginWithBrowser` directly.** It returns a +session and `callbackError` already, so no scraping - but it performs +none of the enrollment (session write, gateway seed, sink provisioning, +daemon install, attach wait). Using it would rebuild that in the wizard, +which is the second enrollment mechanism LLP 0134 forbids. + +## Consequences + +- Rewording a login message can no longer change wizard control flow. + The three constants are internal again, so a reviewer no longer has to + know that user-facing English is API. +- `LoginLaneResult` carries `reason` alongside `exitCode` and `stderr`. + Test doubles that fabricate a lane result now name a reason instead of + embedding a sentence in a fake stderr. +- `reason` is a wider vocabulary than the wizard consumes today + (`'seed_failed'`, `'daemon_incomplete'`, ...). That is deliberate: the + distinctions already exist as separate exits and separate messages, and + naming them costs nothing where they are already branched. +- The command's own contract is unchanged: same exit codes, same output, + byte for byte. This refactor is invisible from the terminal. + +## References + +- LLP 0058#d7 (the refusal taxonomy this stops re-deriving) +- LLP 0134#login-lane, LLP 0135#join (what the wizard is allowed to be) diff --git a/src/core/cli/remote_commands.js b/src/core/cli/remote_commands.js index 2846f0b0..8285ee91 100644 --- a/src/core/cli/remote_commands.js +++ b/src/core/cli/remote_commands.js @@ -30,7 +30,7 @@ import { loadClientDescriptors, probeAttachedClients, resolveLiveGatewayEndpoint /** * @import { CommandRunContext } from '../../../hypaware-plugin-kernel-types.js' - * @import { OidcSession } from '../../../src/core/remote/types.js' + * @import { LoginOutcome, OidcSession } from '../../../src/core/remote/types.js' */ /** @@ -361,26 +361,49 @@ export async function runRemoteAdd(argv, ctx) { * @param {string[]} argv * @param {CommandRunContext} ctx * @param {{ login?: typeof loginWithBrowser, seed?: typeof seedLoginGateway, enroll?: typeof enrollCentralSink, waitForAttach?: typeof waitForClientAttach }} [deps] test seam for the browser flow, gateway seeding, central-sink enrollment, and the post-enroll attach wait + * @returns {Promise} * @ref LLP 0058#d1 [implements]: browser mode of `hyp remote login`; one command, one store, one more way to populate it */ export async function runRemoteLogin(argv, ctx, deps = {}) { + return (await remoteLogin(argv, ctx, deps)).exitCode +} + +/** + * The login lane proper: everything `hyp remote login` does, reported as an + * outcome rather than an exit code. The command surface above collapses it to + * a number, which is where a number belongs; the wizard's join phase branches + * on `reason` instead of matching the D7 sentences out of captured stderr, + * which is what it used to have to do. + * + * Output is unchanged and still happens here, interleaved with the work: the + * consent notice has to precede the browser (LLP 0063 D3) and the first-sync + * hold message has to follow its marker write (LLP 0101), so the printing is + * ordered by the work, not appended to it. + * + * @param {string[]} argv + * @param {CommandRunContext} ctx + * @param {{ login?: typeof loginWithBrowser, seed?: typeof seedLoginGateway, enroll?: typeof enrollCentralSink, waitForAttach?: typeof waitForClientAttach }} [deps] + * @returns {Promise} + * @ref LLP 0179#outcome [implements]: the login lane returns { exitCode, reason }; runRemoteLogin is the adapter that keeps the CLI contract a number + */ +export async function remoteLogin(argv, ctx, deps = {}) { const tokenFileArg = valueFlag(argv, '--token-file') const tokenFile = tokenFileArg.value if (tokenFileArg.present && !tokenFile) { ctx.stderr.write('hyp remote login: --token-file expects a path\n') - return 2 + return { exitCode: 2, reason: 'usage' } } const orgArg = valueFlag(argv, '--org') const org = orgArg.value if (orgArg.present && !org) { ctx.stderr.write('hyp remote login: --org expects an org name\n') - return 2 + return { exitCode: 2, reason: 'usage' } } const hostArg = valueFlag(argv, '--host') const host = hostArg.value if (hostArg.present && !host) { ctx.stderr.write('hyp remote login: --host expects a host label\n') - return 2 + return { exitCode: 2, reason: 'usage' } } // The target name is the first positional. Skip the VALUE slot of a // value-taking flag so e.g. `login --org acme` (name omitted) is not misread @@ -485,7 +508,7 @@ export function valueFlag(argv, flag) { * @param {string | undefined} tokenFile * @param {any} stdin * @param {CommandRunContext} ctx - * @returns {Promise} + * @returns {Promise} * @ref LLP 0058#d8 [implements]: static token stays the documented headless fallback */ async function runStaticLogin(name, tokenFile, stdin, ctx) { @@ -497,7 +520,7 @@ async function runStaticLogin(name, tokenFile, stdin, ctx) { : (await readAllStdin(stdin)).trim() } catch (err) { ctx.stderr.write(`hyp remote login: ${err instanceof Error ? err.message : String(err)}\n`) - return 1 + return { exitCode: 1, reason: 'login_failed' } } if (!token) { ctx.stderr.write('hyp remote login: empty token\n') @@ -506,7 +529,7 @@ async function runStaticLogin(name, tokenFile, stdin, ctx) { if (!tokenFile) { ctx.stderr.write(' (to sign in with a browser instead, re-run with --browser)\n') } - return 2 + return { exitCode: 2, reason: 'usage' } } return persistStaticToken(name, token, ctx) @@ -521,7 +544,7 @@ async function runStaticLogin(name, tokenFile, stdin, ctx) { * @param {string} name * @param {string} token a non-empty, trimmed token * @param {CommandRunContext} ctx - * @returns {Promise} + * @returns {Promise} */ async function persistStaticToken(name, token, ctx) { const stateDir = readObservabilityEnv(ctx.env).stateDir @@ -531,7 +554,7 @@ async function persistStaticToken(name, token, ctx) { // writeToken now contends for the cross-process credentials lock and can // throw a lock timeout; keep the friendly `hyp remote login:` contract. ctx.stderr.write(`hyp remote login: ${err instanceof Error ? err.message : String(err)}\n`) - return 1 + return { exitCode: 1, reason: 'store_failed' } } ctx.stdout.write(`stored query-scoped token for '${name}'\n`) @@ -541,7 +564,7 @@ async function persistStaticToken(name, token, ctx) { if (!remotes[name]) { ctx.stderr.write(`note: '${name}' is not a configured target - add it with 'hyp remote add ${name} '\n`) } - return 0 + return { exitCode: 0, reason: 'ok' } } /** @@ -557,7 +580,7 @@ async function persistStaticToken(name, token, ctx) { * @param {{ org?: string, host?: string, noBrowser: boolean, noForward: boolean, noDaemon: boolean }} opts * @param {CommandRunContext} ctx * @param {{ login: typeof loginWithBrowser, seed: typeof seedLoginGateway, enroll: typeof enrollCentralSink, waitForAttach: typeof waitForClientAttach }} deps - * @returns {Promise} + * @returns {Promise} */ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon }, ctx, { login, seed, enroll, waitForAttach }) { const remotes = await readConfiguredRemotes(ctx) @@ -565,12 +588,12 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon if (!entry) { ctx.stderr.write(`hyp remote login: '${name}' is not a configured target - add it first with 'hyp remote add ${name} '\n`) ctx.stderr.write(" (or pass a static token with --token-file )\n") - return 2 + return { exitCode: 2, reason: 'usage' } } const identityBase = deriveIdentityBase(entry.url) if (!identityBase) { ctx.stderr.write(`hyp remote login: target '${name}' has an invalid url (${entry.url})\n`) - return 2 + return { exitCode: 2, reason: 'usage' } } const stateDir = readObservabilityEnv(ctx.env).stateDir @@ -587,7 +610,7 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon if (!alreadyEnrolled && connectedOrigins.length > 0) { ctx.stderr.write(`hyp remote login: this machine is connected to ${connectedOrigins[0]}\n`) ctx.stderr.write(" disconnect first ('hyp leave'), then log in to the new server\n") - return 2 + return { exitCode: 2, reason: 'connected_elsewhere' } } // Consent is a pre-auth warning, not a prompt (LLP 0063 D3): completing the @@ -623,7 +646,10 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon if (!callbackError) { ctx.stderr.write(" (on a machine with no browser, pass a static token with --token-file or pipe it on stdin; --no-browser prints the URL to open elsewhere)\n") } - return 1 + // The server's own refusal code, carried out as the reason: this is the + // fact the wizard used to reconstruct by matching explainLoginError's + // English back out of stderr (LLP 0179#no-prose-control-flow). + return { exitCode: 1, reason: loginFailureReason(callbackError) } } // The single-use code is already spent by here, so a write failure (most // likely a lock timeout under a concurrent hyp process) is not a login @@ -634,14 +660,14 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon } catch (err) { ctx.stderr.write(`hyp remote login: signed in but could not store the session: ${err instanceof Error ? err.message : String(err)}\n`) ctx.stderr.write(" (re-run 'hyp remote login' once any other hyp process releases the credentials lock)\n") - return 1 + return { exitCode: 1, reason: 'store_failed' } } ctx.stdout.write(`logged in to '${name}' as org '${session.org}'\n`) // No gateway credential (server didn't mint one, or --no-forward): query-only // login, nothing to forward. --no-forward with a minted gateway discards it // unseeded (LLP 0063 D3) - declining enrollment, not just forwarding. - if (!session.gateway) return 0 + if (!session.gateway) return { exitCode: 0, reason: 'ok' } if (noForward) { // --no-forward declines *new* enrollment; it cannot un-enroll a machine // that already forwards (that is `hyp leave`). Tell the truth for each case @@ -651,7 +677,7 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon } else { ctx.stdout.write('note: --no-forward - signed in for queries only; this machine is not enrolled and will not forward logs\n') } - return 0 + return { exitCode: 0, reason: 'ok' } } // One login, two credentials (LLP 0061 D1): the gateway credential seeds the @@ -665,7 +691,7 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon seeded = await seed({ stateDir, configPath: localConfigPath(ctx), targetUrl: entry.url, gateway: session.gateway }) } catch (err) { ctx.stderr.write(`hyp remote login: signed in, but could not seed the forwarding credential: ${err instanceof Error ? err.message : String(err)}\n`) - return 1 + return { exitCode: 1, reason: 'seed_failed' } } // The in-login local-only picker is retired (LLP 0102): enrollment-time @@ -703,11 +729,11 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon result = await enroll({ ctx, url: centralUrl, gateway: session.gateway, noDaemon }) } catch (err) { ctx.stderr.write(`hyp remote login: signed in, but enrollment failed: ${err instanceof Error ? err.message : String(err)}\n`) - return 1 + return { exitCode: 1, reason: 'enroll_failed' } } if (result.connectedElsewhere) { ctx.stderr.write(`hyp remote login: this machine connected to ${result.connectedElsewhere} during sign-in - not enrolling\n`) - return 1 + return { exitCode: 1, reason: 'connected_elsewhere' } } // Name the server, don't print its URL: every modern terminal autolinks a // bare `https://` run (there is no escape that suppresses it), and this @@ -738,12 +764,12 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon if (noDaemon) { ctx.stdout.write("daemon install skipped (--no-daemon); run 'hyp daemon install' to finish enrolling\n") ctx.stderr.write(DURABLE_HINT) - return 0 + return { exitCode: 0, reason: 'ok' } } if (result.daemonCode !== 0) { ctx.stderr.write("note: enrolled, but the daemon install did not finish - run 'hyp daemon install'\n") ctx.stderr.write(DURABLE_HINT) - return result.daemonCode + return { exitCode: result.daemonCode, reason: 'daemon_incomplete' } } // The daemon is installed; it now pulls the org config and auto-attaches any // clients it enables (LLP 0044). Wait for that first reconcile so we report @@ -764,7 +790,7 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon ctx.stdout.write("no clients attached yet - check 'hyp status', or run 'hyp attach ' to capture\n") } ctx.stderr.write(DURABLE_HINT) - return 0 + return { exitCode: 0, reason: 'ok' } } // A matching sink already existed: this was a re-seed (already enrolled). @@ -786,23 +812,54 @@ async function runBrowserLogin(name, { org, host, noBrowser, noForward, noDaemon // already forwarding, so there is no "first" sync to defer - this fork writes // no hold (LLP 0101 #which). The durable CLI floor stays discoverable. ctx.stderr.write(DURABLE_HINT) - return 0 + return { exitCode: 0, reason: 'ok' } } /** - * The D7 messages that describe a *definitive* login rejection: retrying - * the same bare login cannot fix any of them. For `no_membership` and - * `org_not_permitted` an admin has to act; for `org_selection_required` - * (a multi-org account with no selector) the user has to pick an org via - * `hyp remote login --org `, which the wizard's bare login cannot - * supply. Exported as the single source of truth so the wizard join - * phase's `classifyLoginFailure` recognizes them from the captured - * login-lane stderr without re-encoding the taxonomy (LLP 0058 D7). A - * transient/other error carries none of these phrases. + * The D7 messages that describe a *definitive* login rejection: retrying the + * same bare login cannot fix any of them. For `no_membership` and + * `org_not_permitted` an admin has to act; for `org_selection_required` (a + * multi-org account with no selector) the user has to pick an org via + * `hyp remote login --org `, which the wizard's bare login cannot supply. + * + * Prose, and nothing else. These were exported so the wizard could recognize + * a refusal by substring-matching them out of captured stderr, which made + * English load-bearing; `loginFailureReason` reports the same distinctions as + * codes now, so these can be reworded freely. + * @ref LLP 0179#no-prose-control-flow [implements]: the messages stop being API */ -export const LOGIN_NO_MEMBERSHIP_MESSAGE = 'this account is not a member of any org on this server - ask an admin to invite you' -export const LOGIN_ORG_NOT_PERMITTED_MESSAGE = 'the selected org is not permitted for this account - check the --org name' -export const LOGIN_ORG_SELECTION_MESSAGE = 'this account has more than one org - re-run with --org to choose one' +const LOGIN_NO_MEMBERSHIP_MESSAGE = 'this account is not a member of any org on this server - ask an admin to invite you' +const LOGIN_ORG_NOT_PERMITTED_MESSAGE = 'the selected org is not permitted for this account - check the --org name' +const LOGIN_ORG_SELECTION_MESSAGE = 'this account has more than one org - re-run with --org to choose one' + +/** + * The reason code behind a failed sign-in: the server-surfaced callback error + * (LLP 0058 D7) when there was one, otherwise a local failure - a loopback + * timeout, a network error, an abandoned browser flow - which is retriable and + * so is not one of the definitive refusals. + * + * The `default` covers a code we do not model (a new server refusal, or a raw + * OAuth error): retriable is the safe reading, since the alternative tells a + * user to stop trying over a code we do not understand. + * + * @param {string | undefined} callbackError + * @returns {LoginOutcome['reason']} + * @ref LLP 0179#outcome [implements]: the D7 code the terminal explains, reported rather than re-derived + */ +function loginFailureReason(callbackError) { + switch (callbackError) { + case 'no_membership': + return 'no_membership' + case 'org_not_permitted': + return 'org_not_permitted' + case 'org_selection_required': + return 'org_selection_required' + case 'access_denied': + return 'denied' + default: + return 'login_failed' + } +} /** * Translate a server-surfaced callback `error` (D7) into a clear message. The diff --git a/src/core/cli/wizard/index.js b/src/core/cli/wizard/index.js index 690923d1..0a7ac6e1 100644 --- a/src/core/cli/wizard/index.js +++ b/src/core/cli/wizard/index.js @@ -20,7 +20,6 @@ import { buildPluginCatalog } from '../../plugin_catalog.js' import { collectHypAwareStatus } from '../../daemon/status.js' import { formatFirstSyncDeadline, readFirstSyncDeadline } from '../../usage-policy/first_sync_hold.js' import { LOCAL_INSTALL_RETENTION_DAYS, runPickerFinale, writeWalkthroughRunSummary } from '../walkthrough.js' -import { LOGIN_ORG_SELECTION_MESSAGE } from '../remote_commands.js' import { useColor } from '../stdio.js' import { evaluateReturningGate, runWizardFork } from './fork.js' import { firstLookNoticeSink, firstLookRunnerFromCtx, runWizardFirstLook } from './first_look.js' @@ -252,6 +251,7 @@ export async function runInitWizard(opts) { * login it wraps cannot pass `--org` - so the fix is the manual login, * after which re-running `hyp init` re-enters as an enrolled machine. * + * @ref LLP 0179#no-prose-control-flow [implements]: the multi-org branch reads the lane's reason code, not its sentence * @param {RunInitWizardOptions} opts * @param {WizardJoinResult} join */ @@ -260,7 +260,7 @@ function printJoinFailure(opts, join) { opts.stderr.write('Sign-in did not complete. You can try again, or set up locally for now.\n') return } - if (join.detail?.includes(LOGIN_ORG_SELECTION_MESSAGE)) { + if (join.reason === 'org_selection_required') { opts.stderr.write('Joining failed: this account belongs to more than one org. Run `hyp remote login --org ` first, then re-run `hyp init`.\n') return } diff --git a/src/core/cli/wizard/join.js b/src/core/cli/wizard/join.js index ea5b1a0b..e7bd14a0 100644 --- a/src/core/cli/wizard/join.js +++ b/src/core/cli/wizard/join.js @@ -9,10 +9,7 @@ import { Attr, getLogger, withSpan } from '../../observability/index.js' import { readObservabilityEnv } from '../../observability/env.js' import { resolveConfigPath, resolveLayeredConfigFromDisk } from '../../runtime/boot.js' import { - LOGIN_NO_MEMBERSHIP_MESSAGE, - LOGIN_ORG_NOT_PERMITTED_MESSAGE, - LOGIN_ORG_SELECTION_MESSAGE, - runRemoteLogin, + remoteLogin, waitForCentralConverge, } from '../remote_commands.js' import { classifyClientProvenance } from './provenance.js' @@ -28,7 +25,7 @@ export const ORG_CONFIG_WAIT_MS = 60000 /** * The wizard's join phase (LLP 0135 #join). A thin narration wrapper around * the existing `hyp remote login` machinery, never a second enrollment - * mechanism (`@ref LLP 0134#login-lane`): it runs `runRemoteLogin`, waits + * mechanism (`@ref LLP 0134#login-lane`): it runs the login lane, waits * (bounded) for the daemon to converge on the org config, and computes the * set of picker source ids the central layer owns so the pick phase can * lock them (`@ref LLP 0129#join-before-picker`). @@ -43,7 +40,7 @@ export const ORG_CONFIG_WAIT_MS = 60000 * empty lock set rather than blocking - nothing is pinned, so the picker * composes freely (LLP 0129). * - * @ref LLP 0134#login-lane [implements]: "Join a team" wraps `runRemoteLogin`; the wizard adds narration and the locked-row computation, not a second enrollment path. + * @ref LLP 0134#login-lane [implements]: "Join a team" wraps the `hyp remote login` lane; the wizard adds narration and the locked-row computation, not a second enrollment path. * * @param {RunWizardJoinOptions} opts * @returns {Promise} @@ -93,7 +90,7 @@ async function runJoinFlow(opts, span) { const status = classifyLoginFailure(login) setSpanAttr(span, 'status', 'error') setSpanAttr(span, Attr.ERROR_KIND, status === 'failed' ? 'login_rejected' : 'login_abandoned') - return { status, detail: login.stderr } + return { status, detail: login.stderr, ...(login.reason ? { reason: login.reason } : {}) } } opts.stdout.write("Applying your org's configuration...\n") @@ -138,39 +135,41 @@ export async function computeCentralLockedSources(opts) { /** * Map an incomplete login to the fork-returning outcome (LLP 0129 - * #failed-join-returns-to-fork). The login lane already wrote the human - * explanation to stderr via `explainLoginError`; we only classify by - * matching the *definitive* D7 rejection phrases it emits: - * `no_membership` and `org_not_permitted` mean an admin has to act, and - * `org_selection_required` (a multi-org account) needs an explicit - * `hyp remote login --org ` the wizard's bare login cannot supply. - * Retrying the same login is futile for all three -> `'failed'`. - * Anything else - a transient network error, a login timeout, an - * abandoned browser flow, a store/seed failure - is retriable, so -> - * `'abandoned'`. + * #failed-join-returns-to-fork), reading the login lane's reason code. + * The *definitive* D7 rejections are `no_membership` and + * `org_not_permitted` (an admin has to act) and `org_selection_required` + * (a multi-org account needs an explicit `hyp remote login --org ` + * the wizard's bare login cannot supply). Retrying the same login is + * futile for all three -> `'failed'`. Anything else - a provider denial, + * a transient network error, a login timeout, an abandoned browser flow, + * a store/seed failure - is retriable, so -> `'abandoned'`. * - * @ref LLP 0058#d7 [constrained-by]: reuses the login lane's existing membership/permission taxonomy rather than re-encoding it - * @param {Pick} login + * A lane result with no reason (an old-shaped test double) classifies as + * retriable, which is the same answer the stderr matcher gave when it + * recognized nothing. + * + * @ref LLP 0179#no-prose-control-flow [implements]: classify on the reason code, never on the lane's English + * @param {Pick} login * @returns {'failed' | 'abandoned'} */ export function classifyLoginFailure(login) { - const stderr = login?.stderr ?? '' - if ( - stderr.includes(LOGIN_NO_MEMBERSHIP_MESSAGE) || - stderr.includes(LOGIN_ORG_NOT_PERMITTED_MESSAGE) || - stderr.includes(LOGIN_ORG_SELECTION_MESSAGE) - ) { - return 'failed' + switch (login?.reason) { + case 'no_membership': + case 'org_not_permitted': + case 'org_selection_required': + return 'failed' + default: + return 'abandoned' } - return 'abandoned' } /** * The production login lane: run `hyp remote login` (bare, so it resolves * the default target and the browser flow, `@ref LLP 0134#no-token-join` - * - the wizard never passes a token) against the wizard's command context, - * teeing its stderr so `classifyLoginFailure` can read the D7 phrase while - * the user still sees the login lane's own output. + * - the wizard never passes a token) against the wizard's command context. + * `remoteLogin` reports the outcome, so classification reads a code; the + * stderr tee stays for `detail`, which echoes the lane's own explanation + * back to the user (LLP 0179#no-prose-control-flow). * * @param {RunWizardJoinOptions} opts * @returns {Promise} @@ -182,8 +181,8 @@ async function defaultRunLogin(opts) { } const capture = teeWriter(ctx.stderr) const teed = /** @type {CommandRunContext} */ ({ ...ctx, stderr: capture.stream }) - const exitCode = await runRemoteLogin([], teed, {}) - return { exitCode, stderr: capture.text() } + const { exitCode, reason } = await remoteLogin([], teed, {}) + return { exitCode, reason, stderr: capture.text() } } /** @@ -208,7 +207,7 @@ async function defaultResolveLayered(opts) { /** * A write-through capture over a stream's `write`: every chunk is recorded * and forwarded to the underlying stream, so the login lane's stderr is both - * shown to the user and available to `classifyLoginFailure`. + * shown to the user and available as the failure `detail`. * * @param {{ write(chunk: string): unknown }} target * @returns {{ stream: { write(chunk: string): unknown }, text(): string }} diff --git a/src/core/cli/wizard/types.d.ts b/src/core/cli/wizard/types.d.ts index 32215a64..e02079f4 100644 --- a/src/core/cli/wizard/types.d.ts +++ b/src/core/cli/wizard/types.d.ts @@ -1,6 +1,7 @@ import type { CapabilityRegistry, CommandRunContext, HypAwareV2Config } from '../../../../hypaware-plugin-kernel-types.d.ts' import type { CollectStatusOptions, HypAwareStatusReport } from '../../daemon/types.d.ts' import type { OverviewQueryRunner } from '../../query/types.d.ts' +import type { LoginOutcomeReason } from '../../remote/types.d.ts' import type { PickerDescriptor, PluginCatalog } from '../../types.d.ts' import type { AsyncBackfillConsentPrompt, @@ -150,13 +151,18 @@ export interface LayeredProvenance { /** * What the login lane returns to the join wrapper: the `hyp remote login` - * exit code plus its captured stderr. `classifyLoginFailure` maps the D7 - * taxonomy phrases in `stderr` to `'failed' | 'abandoned'` (LLP 0058 D7); - * the query-only text is surfaced verbatim to the user by the login lane - * itself, so the wrapper only classifies, never re-prints. + * exit code, the reason behind it, and its captured stderr. + * `classifyLoginFailure` maps `reason` to `'failed' | 'abandoned'` + * (LLP 0179#no-prose-control-flow); `stderr` is narration only, echoed back + * as the failure `detail`, never matched against. */ export interface LoginLaneResult { exitCode: number + /** + * Absent only on a test double that predates the outcome return; the + * classifier reads that as retriable. + */ + reason?: LoginOutcomeReason stderr: string } @@ -178,10 +184,15 @@ export interface WizardJoinResult { managed?: boolean /** * On a failure (`'failed' | 'abandoned'`): the login lane's own captured - * explanation, so `runInitWizard`'s `printJoinFailure` can echo the D7 - * meaning without re-deriving it. + * explanation, for narration. The lane already printed it; nothing + * branches on it (LLP 0179#no-prose-control-flow). */ detail?: string + /** + * On a failure: the login lane's reason code, which is what + * `printJoinFailure` branches on to name the wizard-level consequence. + */ + reason?: LoginOutcomeReason } export interface RunWizardJoinOptions { diff --git a/src/core/remote/types.d.ts b/src/core/remote/types.d.ts index f2fc1fec..daa9a213 100644 --- a/src/core/remote/types.d.ts +++ b/src/core/remote/types.d.ts @@ -102,3 +102,40 @@ export interface RemoteOidcRecord { * dropped by the same `removeToken`. */ export type RemoteCredentialRecord = RemoteStaticRecord | RemoteOidcRecord + +/** + * Why a `hyp remote login` ended the way it did (LLP 0179#outcome). A code, + * never a message: the login lane prints its own English, and a caller that + * has to branch reads this instead of the prose. + * + * `no_membership` / `org_not_permitted` / `org_selection_required` are the + * server-surfaced LLP 0058 D7 refusals, definitive in the sense that the same + * bare login cannot fix them. `denied` is a provider denial and `login_failed` + * a transient or local failure (timeout, network, an abandoned browser flow); + * both are worth retrying. The rest name a step that failed after the sign-in + * itself succeeded, which is why they are not folded into one `failed`. + */ +export type LoginOutcomeReason = + | 'ok' + | 'usage' + | 'connected_elsewhere' + | 'no_membership' + | 'org_not_permitted' + | 'org_selection_required' + | 'denied' + | 'login_failed' + | 'store_failed' + | 'seed_failed' + | 'enroll_failed' + | 'daemon_incomplete' + +/** + * What the login lane returns: the exit code `hyp remote login` reports, and + * the reason behind it. `exitCode === 0` iff `reason === 'ok'`, except for + * `daemon_incomplete`, which carries the daemon installer's own non-zero code + * (LLP 0179#outcome). + */ +export interface LoginOutcome { + exitCode: number + reason: LoginOutcomeReason +} diff --git a/test/core/cli/wizard/index.test.js b/test/core/cli/wizard/index.test.js index bfa7bc13..edc08dbd 100644 --- a/test/core/cli/wizard/index.test.js +++ b/test/core/cli/wizard/index.test.js @@ -196,11 +196,12 @@ test('runInitWizard: a failed join explains and returns to the fork', async () = }) test('runInitWizard: a multi-org join failure points at hyp remote login --org', async () => { - const { LOGIN_ORG_SELECTION_MESSAGE } = await import('../../../../src/core/cli/remote_commands.js') const forkChoices = ['team', 'local'] const { opts, stderr } = wizardOpts(await tmpHome(), { fork: async () => forkChoices.shift(), - join: async () => ({ status: 'failed', detail: `hyp remote login: ${LOGIN_ORG_SELECTION_MESSAGE}\n` }), + // The reason picks the branch; `detail` is the lane's own prose, echoed + // but never matched (LLP 0179#no-prose-control-flow). + join: async () => ({ status: 'failed', reason: 'org_selection_required', detail: 'hyp remote login: more than one org\n' }), }) const result = await runInitWizard(opts) assert.equal(result.pathway, 'local') diff --git a/test/core/cli/wizard/join.test.js b/test/core/cli/wizard/join.test.js index e9f3b402..6b9c50f8 100644 --- a/test/core/cli/wizard/join.test.js +++ b/test/core/cli/wizard/join.test.js @@ -4,11 +4,6 @@ import test from 'node:test' import assert from 'node:assert/strict' import { classifyLoginFailure, runWizardJoin } from '../../../../src/core/cli/wizard/join.js' -import { - LOGIN_NO_MEMBERSHIP_MESSAGE, - LOGIN_ORG_NOT_PERMITTED_MESSAGE, - LOGIN_ORG_SELECTION_MESSAGE, -} from '../../../../src/core/cli/remote_commands.js' /** * @import { HypAwareV2Config } from '../../../../hypaware-plugin-kernel-types.js' @@ -88,26 +83,38 @@ function joinOpts(cat, over) { // --- classifyLoginFailure: the D7 taxonomy split --- test('classifyLoginFailure: no_membership is a definitive rejection -> failed', () => { - assert.equal(classifyLoginFailure({ stderr: `hyp remote login: ${LOGIN_NO_MEMBERSHIP_MESSAGE}\n` }), 'failed') + assert.equal(classifyLoginFailure({ reason: 'no_membership' }), 'failed') }) test('classifyLoginFailure: org_not_permitted is a definitive rejection -> failed', () => { - assert.equal(classifyLoginFailure({ stderr: `hyp remote login: ${LOGIN_ORG_NOT_PERMITTED_MESSAGE}\n` }), 'failed') + assert.equal(classifyLoginFailure({ reason: 'org_not_permitted' }), 'failed') }) // A multi-org account is definitive *for the wizard*: its bare login can // never pass --org, so retrying the fork's "Join a team" is futile; the fix // is a manual `hyp remote login --org ` then re-entering `hyp init`. test('classifyLoginFailure: org_selection_required (multi-org account) -> failed', () => { - assert.equal(classifyLoginFailure({ stderr: `hyp remote login: ${LOGIN_ORG_SELECTION_MESSAGE}\n` }), 'failed') + assert.equal(classifyLoginFailure({ reason: 'org_selection_required' }), 'failed') }) -test('classifyLoginFailure: a transient network error is retriable -> abandoned', () => { - assert.equal(classifyLoginFailure({ stderr: 'hyp remote login: connect ETIMEDOUT 10.0.0.1:443\n' }), 'abandoned') +test('classifyLoginFailure: retriable reasons -> abandoned', () => { + for (const reason of /** @type {const} */ (['login_failed', 'denied', 'store_failed', 'seed_failed', 'enroll_failed'])) { + assert.equal(classifyLoginFailure({ reason }), 'abandoned', reason) + } +}) + +// The lane's prose is narration now, not a signal: a stderr that reads like a +// definitive refusal classifies on the reason it carries, not on its words. +// @ref LLP 0179#no-prose-control-flow [tests]: +test('classifyLoginFailure: the message text is not consulted', () => { + const rejectionProse = 'hyp remote login: this account is not a member of any org on this server - ask an admin to invite you\n' + assert.equal( + classifyLoginFailure(/** @type {any} */ ({ reason: 'login_failed', stderr: rejectionProse })), + 'abandoned' + ) }) -test('classifyLoginFailure: an empty/absent stderr defaults to abandoned', () => { - assert.equal(classifyLoginFailure({ stderr: '' }), 'abandoned') +test('classifyLoginFailure: a missing reason defaults to abandoned', () => { assert.equal(classifyLoginFailure(/** @type {any} */ ({})), 'abandoned') }) @@ -117,11 +124,11 @@ test('runWizardJoin: a non-zero login exit returns the classified failure and ne const cat = catalog({ pickerRows: { claude: '@hypaware/claude' } }) let waited = false const opts = joinOpts(cat, { - runLogin: async () => ({ exitCode: 1, stderr: `hyp remote login: ${LOGIN_NO_MEMBERSHIP_MESSAGE}\n` }), + runLogin: async () => ({ exitCode: 1, reason: 'no_membership', stderr: 'hyp remote login: not a member\n' }), waitForConverge: async () => { waited = true; return { ok: true, attached: ['claude'] } }, }) const out = await runWizardJoin(opts) - assert.deepEqual(out, { status: 'failed', detail: `hyp remote login: ${LOGIN_NO_MEMBERSHIP_MESSAGE}\n` }) + assert.deepEqual(out, { status: 'failed', detail: 'hyp remote login: not a member\n', reason: 'no_membership' }) assert.equal(waited, false, 'must not wait for convergence after a failed login') assert.match(opts.stdout.text(), /Joining your team/) }) @@ -129,7 +136,7 @@ test('runWizardJoin: a non-zero login exit returns the classified failure and ne test('runWizardJoin: a transient login failure returns abandoned', async () => { const cat = catalog({ pickerRows: { claude: '@hypaware/claude' } }) const opts = joinOpts(cat, { - runLogin: async () => ({ exitCode: 1, stderr: 'hyp remote login: request timed out\n' }), + runLogin: async () => ({ exitCode: 1, reason: 'login_failed', stderr: 'hyp remote login: request timed out\n' }), }) const out = await runWizardJoin(opts) assert.equal(out.status, 'abandoned') @@ -152,7 +159,7 @@ test('runWizardJoin: on convergence, locks exactly the central-layer picker rows ['@hypaware/claude', '@hypaware/otel', '@hypaware/codex'] ) const opts = joinOpts(cat, { - runLogin: async () => ({ exitCode: 0, stderr: '' }), + runLogin: async () => ({ exitCode: 0, reason: 'ok', stderr: '' }), waitForConverge: async () => ({ ok: true, attached: ['claude'] }), resolveLayered: async () => lc, }) @@ -167,7 +174,7 @@ test('runWizardJoin: convergence with no central-owned rows locks nothing', asyn // A central layer exists but owns a different plugin; codex stays local. const lc = layered(['@hypaware/claude'], ['@hypaware/claude', '@hypaware/codex']) const opts = joinOpts(cat, { - runLogin: async () => ({ exitCode: 0, stderr: '' }), + runLogin: async () => ({ exitCode: 0, reason: 'ok', stderr: '' }), waitForConverge: async () => ({ ok: true, attached: ['claude'] }), resolveLayered: async () => lc, }) @@ -183,7 +190,7 @@ test('runWizardJoin: a convergence timeout narrates and returns an empty lock se const cat = catalog({ pickerRows: { claude: '@hypaware/claude' } }) let resolved = false const opts = joinOpts(cat, { - runLogin: async () => ({ exitCode: 0, stderr: '' }), + runLogin: async () => ({ exitCode: 0, reason: 'ok', stderr: '' }), waitForConverge: async () => ({ ok: false, attached: [] }), // resolveLayered must never run on the timeout path (nothing to lock). resolveLayered: async () => { resolved = true; return layered([], []) }, @@ -199,7 +206,7 @@ test('runWizardJoin: passes the org-config wait budget through to the converge h /** @type {any} */ let sawWaitOpts = null const opts = joinOpts(cat, { - runLogin: async () => ({ exitCode: 0, stderr: '' }), + runLogin: async () => ({ exitCode: 0, reason: 'ok', stderr: '' }), waitForConverge: async (_o, waitOpts) => { sawWaitOpts = waitOpts; return { ok: false, attached: [] } }, }) await runWizardJoin(opts) diff --git a/test/core/remote-login-command.test.js b/test/core/remote-login-command.test.js index c5ae96e9..daaa37a8 100644 --- a/test/core/remote-login-command.test.js +++ b/test/core/remote-login-command.test.js @@ -6,7 +6,7 @@ import fs from 'node:fs/promises' import os from 'node:os' import path from 'node:path' -import { runRemoteLogin, runRemoteRemove, waitForCentralConverge, waitForClientAttach } from '../../src/core/cli/remote_commands.js' +import { remoteLogin, runRemoteLogin, runRemoteRemove, waitForCentralConverge, waitForClientAttach } from '../../src/core/cli/remote_commands.js' import { effectiveDefaultRemote } from '../../src/core/remote/builtin_remotes.js' import { deriveIdentityBase, readCredentials } from '../../src/core/remote/credentials.js' import { computeFirstSyncDeadline, firstSyncHoldMarkerPath, formatFirstSyncDeadline, readFirstSyncDeadline } from '../../src/core/usage-policy/first_sync_hold.js' @@ -111,6 +111,67 @@ test('a successful sign-in whose session write fails reports a store failure, no assert.equal(out.join(''), '') }) +// --- the outcome the wizard reads (LLP 0179 #outcome) --- +// @ref LLP 0179#outcome [tests]: + +test('a server refusal is reported as its own reason, not just an exit code', async () => { + for (const [callbackError, reason] of [ + ['no_membership', 'no_membership'], + ['org_not_permitted', 'org_not_permitted'], + ['org_selection_required', 'org_selection_required'], + ['access_denied', 'denied'], + // A refusal code we do not model reads as retriable: telling a user to + // stop trying over a code we cannot interpret is the worse error. + ['some_new_server_refusal', 'login_failed'], + ]) { + const { ctx, err } = await makeCtx({ hypHome: await tmpHome() }) + const login = /** @type {any} */ (async () => { + throw Object.assign(new Error(`login failed: ${callbackError}`), { callbackError }) + }) + const outcome = await remoteLogin(['prod'], ctx, { login }) + assert.deepEqual(outcome, { exitCode: 1, reason }, callbackError) + // The lane still explains itself in prose; the reason is additional, not a + // replacement. + assert.match(err.join(''), /^hyp remote login: .+$/m, callbackError) + } +}) + +test('a local failure with no server code is retriable, and a success is ok', async () => { + const { ctx: failCtx } = await makeCtx({ hypHome: await tmpHome() }) + const timeout = /** @type {any} */ (async () => { throw new Error('timed out waiting for the browser login to complete') }) + assert.deepEqual(await remoteLogin(['prod'], failCtx, { login: timeout }), { exitCode: 1, reason: 'login_failed' }) + + const { ctx: okCtx } = await makeCtx({ hypHome: await tmpHome() }) + const login = /** @type {any} */ (async () => ({ refreshToken: 'rt', accessJwt: 'jwt', expiresAt: '2999-01-01T00:00:00Z', org: 'acme' })) + assert.deepEqual(await remoteLogin(['prod'], okCtx, { login }), { exitCode: 0, reason: 'ok' }) +}) + +test('post-auth failures name their step rather than collapsing into the login failure', async () => { + const hypHome = await tmpHome() + const { ctx } = await makeCtx({ hypHome }) + // A plain file where the state dir must be makes the session write throw. + await fs.writeFile(path.join(hypHome, 'hypaware'), 'not a dir') + const login = /** @type {any} */ (async () => ({ refreshToken: 'rt', accessJwt: 'jwt', expiresAt: '2999-01-01T00:00:00Z', org: 'acme' })) + assert.deepEqual(await remoteLogin(['prod'], ctx, { login }), { exitCode: 1, reason: 'store_failed' }) +}) + +test('a usage error and the exclusivity gate are distinguishable, both exit 2', async () => { + const { ctx: usageCtx } = await makeCtx({ hypHome: await tmpHome() }) + assert.deepEqual(await remoteLogin(['prod', '--org'], usageCtx, {}), { exitCode: 2, reason: 'usage' }) + + const hypHome = await tmpHome() + const { ctx } = await makeCtx({ hypHome }) + await writeCentralSeed(hypHome, 'https://elsewhere.example') + const login = /** @type {any} */ (async () => { throw new Error('the browser must never open here') }) + assert.deepEqual(await remoteLogin(['prod'], ctx, { login }), { exitCode: 2, reason: 'connected_elsewhere' }) +}) + +test('runRemoteLogin stays the exit-code adapter over the same run', async () => { + const { ctx } = await makeCtx({ hypHome: await tmpHome() }) + const login = /** @type {any} */ (async () => ({ refreshToken: 'rt', accessJwt: 'jwt', expiresAt: '2999-01-01T00:00:00Z', org: 'acme' })) + assert.equal(await runRemoteLogin(['prod'], ctx, { login }), 0) +}) + test('--no-browser passes noBrowser through to the flow', async () => { const hypHome = await tmpHome() const { ctx } = await makeCtx({ hypHome }) From 7cf0a434bc18b4726b94583e006ccf487701319b Mon Sep 17 00:00:00 2001 From: test Date: Tue, 4 Aug 2026 01:52:39 +0000 Subject: [PATCH 2/3] Review fixes: outcome coverage for the post-auth steps, and a stale runLogin doc Assert the reason on the three post-auth failure paths the outcome return names but nothing tested (seed, enroll, daemon install); each one's reason could be swapped for another and the suite stayed green. RunWizardJoinOptions.runLogin still documented a captured stderr as what classifyLoginFailure reads, which this branch is exactly what changes. Co-Authored-By: Claude --- src/core/cli/wizard/types.d.ts | 6 +++--- test/core/remote-login-command.test.js | 28 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/core/cli/wizard/types.d.ts b/src/core/cli/wizard/types.d.ts index e02079f4..65ea47b1 100644 --- a/src/core/cli/wizard/types.d.ts +++ b/src/core/cli/wizard/types.d.ts @@ -207,14 +207,14 @@ export interface RunWizardJoinOptions { */ catalog: PluginCatalog /** - * The command context `runRemoteLogin` runs against (production wiring, + * The command context the login lane runs against (production wiring, * supplied by `runInitWizard`). Optional so tests can inject `runLogin` * and never touch the real login lane. */ ctx?: CommandRunContext /** - * Override the login lane (tests). Defaults to `runRemoteLogin` over - * `ctx` with its stderr captured for `classifyLoginFailure`. + * Override the login lane (tests). Defaults to `remoteLogin` over `ctx`, + * whose returned `reason` is what `classifyLoginFailure` reads. */ runLogin?: () => Promise /** diff --git a/test/core/remote-login-command.test.js b/test/core/remote-login-command.test.js index daaa37a8..2042096c 100644 --- a/test/core/remote-login-command.test.js +++ b/test/core/remote-login-command.test.js @@ -153,6 +153,34 @@ test('post-auth failures name their step rather than collapsing into the login f await fs.writeFile(path.join(hypHome, 'hypaware'), 'not a dir') const login = /** @type {any} */ (async () => ({ refreshToken: 'rt', accessJwt: 'jwt', expiresAt: '2999-01-01T00:00:00Z', org: 'acme' })) assert.deepEqual(await remoteLogin(['prod'], ctx, { login }), { exitCode: 1, reason: 'store_failed' }) + + // The seed, the enroll, and the daemon install are three more steps that run + // after a sign-in that worked; each names itself rather than reporting the + // login as the thing that failed. + const gwLogin = /** @type {any} */ (async () => gatewaySession()) + const { ctx: seedCtx } = await makeCtx({ + hypHome: await tmpHome(), + sinks: { fwd: { plugin: '@hypaware/central', config: { url: 'https://hyp.internal', identity: {} } } }, + }) + const seed = /** @type {any} */ (async () => { throw new Error('disk is sad') }) + assert.deepEqual(await remoteLogin(['prod'], seedCtx, { login: gwLogin, seed }), { exitCode: 1, reason: 'seed_failed' }) + + const { ctx: enrollCtx } = await makeCtx({ hypHome: await tmpHome() }) + const enrollThrows = /** @type {any} */ (async () => { throw new Error('server unreachable') }) + assert.deepEqual( + await remoteLogin(['prod'], enrollCtx, { login: gwLogin, enroll: enrollThrows }), + { exitCode: 1, reason: 'enroll_failed' } + ) + + // An incomplete daemon install carries the installer's own code out, so the + // outcome is not `exitCode: 1` and not `'ok'` either. + const { ctx: daemonCtx } = await makeCtx({ hypHome: await tmpHome() }) + const enroll = /** @type {any} */ (async () => ({ provisioned: true, daemonCode: 3 })) + const waitForAttach = /** @type {any} */ (async () => []) + assert.deepEqual( + await remoteLogin(['prod'], daemonCtx, { login: gwLogin, enroll, waitForAttach }), + { exitCode: 3, reason: 'daemon_incomplete' } + ) }) test('a usage error and the exclusivity gate are distinguishable, both exit 2', async () => { From b0b701d4fef97670fdc29bb17caa1a3c4763ea96 Mon Sep 17 00:00:00 2001 From: test Date: Sun, 9 Aug 2026 20:25:29 +0000 Subject: [PATCH 3/3] Review round: correct LLP 0179's tee rationale and restore the D7 ref (#593) Two findings from the round on the merge head, both left over from the previous review of 7cf0a43: - LLP 0179 #no-prose-control-flow justified keeping the stderr tee on the claim that `WizardJoinResult.detail` "echoes the lane's own explanation to the user". It does not: `detail` is written at join.js:93 and read by nobody once this PR removes printJoinFailure's substring match. What the user sees is the tee's write-through, which needs no capture at all. Reworded to say what is true - the tee writes through, and the copy it keeps is a diagnostic no control flow reads. The decision itself (keep the tee) is unchanged; an Accepted doc should not land freezing a rationale that is false. - Restored `@ref LLP 0058#d7 [constrained-by]` on `classifyLoginFailure`. The three definitive reasons are the D7 refusal codes verbatim, so the function is still constrained by that taxonomy; only the machine-readable link went when the prose one was rewritten. Doc and annotation only; no behaviour change. Co-Authored-By: Claude --- llp/0179-login-lane-returns-its-outcome.decision.md | 11 +++++++---- src/core/cli/wizard/join.js | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/llp/0179-login-lane-returns-its-outcome.decision.md b/llp/0179-login-lane-returns-its-outcome.decision.md index b72c4e04..c9839f71 100644 --- a/llp/0179-login-lane-returns-its-outcome.decision.md +++ b/llp/0179-login-lane-returns-its-outcome.decision.md @@ -61,10 +61,13 @@ seam moves inward rather than up. **Prose is for humans, codes are for control flow.** `classifyLoginFailure` switches on `reason`. The three message constants stop being exported API and go back to being strings -`explainLoginError` prints. The wizard keeps teeing stderr, because -`WizardJoinResult.detail` echoes the lane's own explanation to the user -([LLP 0135](./0135-install-experience-overhaul.design.md#join)) - that -use is narration, which is what captured prose is good for. +`explainLoginError` prints. The wizard keeps teeing stderr. The tee +writes through, so the lane's own explanation reaches the user's terminal +exactly as it always did, and the copy it keeps alongside fills +`WizardJoinResult.detail` +([LLP 0135](./0135-install-experience-overhaul.design.md#join)). No +control flow reads that copy any more; it is narration held as a +diagnostic, which is what captured prose is good for. The classification is unchanged: exactly the three refusals are `'failed'`; everything else non-zero stays `'abandoned'`. A provider diff --git a/src/core/cli/wizard/join.js b/src/core/cli/wizard/join.js index e0ca870c..4dd870da 100644 --- a/src/core/cli/wizard/join.js +++ b/src/core/cli/wizard/join.js @@ -151,6 +151,7 @@ export async function computeCentralLockedSources(opts) { * retriable, which is the same answer the stderr matcher gave when it * recognized nothing. * + * @ref LLP 0058#d7 [constrained-by]: the three definitive reasons are the D7 refusal codes verbatim, so the split is the server's taxonomy and not a wizard-local one * @ref LLP 0179#no-prose-control-flow [implements]: classify on the reason code, never on the lane's English * @param {Pick} login * @returns {'failed' | 'abandoned'}