diff --git a/llp/0185-unpicked-client-stays-attached.decision.md b/llp/0185-unpicked-client-stays-attached.decision.md index d70898bc..8b8c0c6f 100644 --- a/llp/0185-unpicked-client-stays-attached.decision.md +++ b/llp/0185-unpicked-client-stays-attached.decision.md @@ -6,6 +6,7 @@ **Author:** Brendan / Claude **Date:** 2026-08-04 **Related:** LLP 0045 (the one disk-driven undo), LLP 0129 (Reconfigure re-enters the picker), LLP 0086 (the existing attach drift diagnostic), LLP 0180 (the finale's attach lane), LLP 0031 (central layer) +**Extended-by:** LLP 0230 (the finale's print stays where §warn-do-not-detach put it; the wizard, whose closing sequence buries it, repeats it short at the end of the run) > Re-running `hyp init` and unchecking a client the previous run attached > regenerates a config without that client's adapter, while the client's own @@ -90,6 +91,12 @@ of the `config_unreadable` finding that is the real repair. The known gap: a The finale still warns about it at the time, which is where the state is created. +> **Extended-by [LLP 0230](./0230-stranded-attach-warning-repeats-at-the-end.decision.md):** +> the finale's print is where this decision put it, but it is not the last +> thing `hyp init` writes. The wizard repeats it, short, after its first look +> and before the privacy narration; `runPickerWalkthrough`, which writes +> nothing substantial after the finale, still prints it once. + **"Not configured" means what `hyp status` already means by it.** Both surfaces read a **local layer** plugin entry with `enabled: false` as inactive, so a switched-off adapter strands its diff --git a/llp/0230-stranded-attach-warning-repeats-at-the-end.decision.md b/llp/0230-stranded-attach-warning-repeats-at-the-end.decision.md new file mode 100644 index 00000000..b32aed3b --- /dev/null +++ b/llp/0230-stranded-attach-warning-repeats-at-the-end.decision.md @@ -0,0 +1,135 @@ +# LLP 0230: The stranded-attach warning repeats where the run ends + +**Type:** Decision +**Status:** Accepted +**Systems:** Onboarding, CLI, Clients +**Author:** Brendan / Claude +**Date:** 2026-08-05 +**Related:** LLP 0185 (an unpicked client is named, not detached; this doc is its `Extended-by`), LLP 0135 (the wizard's closing sequence), LLP 0100/0101 (the privacy narration), LLP 0086 (the attach drift diagnostic) + +> [LLP 0185](./0185-unpicked-client-stays-attached.decision.md) put the +> stranded-attach warning in the finale, before the daemon restart. It is +> still there. But `hyp init` writes tens of lines more after the finale +> returns, so on a real terminal the warning is not on screen when the run +> ends, and the user who does not scroll back never reads it. + +## Context {#context} + +`runPickerFinale` prints the warning and returns. What happens next depends on +which entry point called it: + +- `runPickerWalkthrough` writes its short run summary and stops. The warning + is within a handful of lines of the last thing on screen. +- `runInitWizard` writes the run summary, then `runWizardFirstLook` (a block of + real query output, by its own design in + [LLP 0135 §first-look](./0135-install-experience-overhaul.design.md) and + sized by + [LLP 0198 §wizard-sections](./0198-setup-ends-on-a-question.decision.md)), + then on the team path the privacy narration. Nothing pauses between them. + +So the same print is prominent on one path and buried on the other. It is +buried worst exactly where it is needed most: on a managed host, +[LLP 0185 §status-backstop](./0185-unpicked-client-stays-attached.decision.md#status-backstop) +gates the `client_attached_not_configured` diagnostic to hosts with no central +layer, so the finale's print is the *only* signal a wizard-attached client the +reconfigure run left unpicked ever gets. A joined host has no backstop to +scroll back to. + +The obvious repair, echoing `finaleSummary.attachedNotConfigured` from +`writeWalkthroughRunSummary`, is wrong twice over. That function is shared by +both entry points, so `runPickerWalkthrough` would print the same warning +twice within a few lines of itself, and the wizard would still bury the echo +under the first look, which is the whole complaint. + +## Decision {#decision} + +**The finale's print stays where it is, and the +caller that buries it repeats it at its own end.** LLP 0185's placement is +unchanged: the finale still names the stranded clients after the config write +and before the daemon restart. `runInitWizard` additionally writes a short +repeat after the first look and before the privacy narration, naming the same +clients and the same `hyp detach --client ` lines. + +The repeat is the *caller's*, not the shared run summary's, because only the +caller knows whether anything came between. `runPickerWalkthrough` prints +nothing substantial after the finale, so it does not repeat, and no path +prints the warning twice on one screen. + +**The repeat is conditional on the closing sequence having +written something, and that fact is measured rather than inferred.** +`runWizardFirstLook` reports `wrote` beside `shown`, counted at the writable +the step writes through, and the wizard gates on `wrote`. + +The two are different questions, and every wrong version of this gate has been +an attempt to answer the first with the second. "The first look ran" is too +wide: the step is documented to degrade rather than fail a finished install +([LLP 0135 §first-look](./0135-install-experience-overhaul.design.md)), and an +unregistered dataset, an unreadable cache or a render that throws leave an +attended run that attempted the block and printed none of it, so nothing +buried the finale's print. "The block rendered" is too narrow: an expired +deadline with nothing renderable prints two lines saying the look was skipped +and reports `shown: false`, and those lines bury the finale's print exactly as +a full render would. On a managed host that is the run where the repeat +matters most, because §status-backstop of +[LLP 0185](./0185-unpicked-client-stays-attached.decision.md#status-backstop) +gates the mirror diagnostic off there. + +Measuring closes the class rather than the instance. A skip reason added later +is counted by the same writable, so it carries its own answer instead of +requiring this gate to be revisited, and a caller that wants "does the user +have their numbers" still reads `shown`. + +That single condition covers the team pathway as well, because a pathway is +only ever resolved on an interactive run, so a team run that is neither +cancelled nor a dry run has already run the first look and repeats on what it +wrote. Widening the condition +to "or the pathway is `team`" on the theory that the privacy narration follows +would admit only the runs where the first look did *not* run, and those are +exactly the runs that wrote nothing between the finale and here: the repeat +would land a few lines under the print it repeats, which is the same double +print the shared run summary was rejected for. + +So a scripted `--yes` or `--dry-run` wizard run stays on the single finale +print and its output is unchanged, matching the attended-only rule +([LLP 0131](./0131-configure-phase.decision.md)), and so does an interactive +run cancelled at the backfill consent, whose summary is all that follows the +finale. + +**The repeat is shorter than the original.** The first +print explains the consequence in full; repeating that paragraph verbatim +reads as a bug rather than a reminder. The repeat carries only what a user +acts on: the client names and one `hyp detach --client ` line each. Both +prints share the dry-run tag rule the rest of the finale uses. + +**The privacy narration is still the last +words on the team path.** The repeat goes before it, not after +([LLP 0135 §first-look](./0135-install-experience-overhaul.design.md), +LLP 0100/0101). Seven lines of narration is a reminder that survives on one +screen; a block of query output on top of it is not, and that is the difference this +decision turns on. + +## Consequences {#consequences} + +- `writeAttachedNotConfiguredReminder` is exported from + `src/core/cli/walkthrough.js` beside the finale's own + `writeAttachedNotConfiguredWarning`, which stays private to the finale. +- `FirstLookResult` carries `wrote` as well as `shown`, so the first look + answers "did this put text on the screen" for itself. It is the step's fact + to report, not the orchestrator's to deduce. +- `writeWalkthroughRunSummary` is untouched. It still reports only what the + finale *did*, per + [LLP 0135](./0135-install-experience-overhaul.design.md); a warning is not + an action taken. +- A stranded attach on an attended wizard run is printed twice in one run, in + two places, deliberately. That is the cost of the finale's placement being + correct for the restart and wrong for the end of the run. +- The managed-host gap LLP 0185 recorded is narrowed, not closed: status still + says nothing on a joined host, but the run that creates the state now ends + on the warning instead of burying it. + +## References + +- Issue #614 (deferred review finding from PR #608), PR #608 +- `src/core/cli/walkthrough.js` (`writeAttachedNotConfiguredReminder`), + `src/core/cli/wizard/index.js` (the closing sequence) +- LLP 0185, LLP 0135, LLP 0131, LLP 0100/0101 diff --git a/src/core/cli/walkthrough.js b/src/core/cli/walkthrough.js index 5ced86ff..0032d763 100644 --- a/src/core/cli/walkthrough.js +++ b/src/core/cli/walkthrough.js @@ -1851,6 +1851,38 @@ function writeAttachedNotConfiguredWarning({ clients, stdout, dryRun }) { for (const client of clients) stdout.write(` hyp detach --client ${client}\n`) } +/** + * The closing repeat of the stranded-attach warning, for an entry point whose + * finale is not the last thing it writes. + * + * The finale's own warning stays exactly where LLP 0185 put it, before the + * daemon restart that is the point of no return. This is a second, compact + * print for the caller that keeps writing afterwards: the wizard follows the + * finale with a run summary, a first look that is roughly sixty lines of real + * query output, and on the team path a privacy narration, none of which pause, + * so on a real terminal the original warning is scrolled away by the time the + * run ends. It matters most on a managed host, where the finale's print is the + * only signal there is because `hyp status`'s mirror diagnostic is gated to + * hosts with no central layer (LLP 0185 #status-backstop). + * + * Only a caller that printed something substantial in between calls this. + * `runPickerWalkthrough` writes a short run summary and stops, so it keeps the + * single finale print and never repeats it onto the same screen. + * + * @ref LLP 0230#repeat-at-the-end [implements]: the repeat belongs to the caller whose own output buried the first print + * @param {{ + * clients: string[], + * stdout: NodeJS.WritableStream | { write(chunk: string): unknown }, + * dryRun: boolean, + * }} args + */ +export function writeAttachedNotConfiguredReminder({ clients, stdout, dryRun }) { + stdout.write('\n') + stdout.write(`${dryRun ? '(dry-run) ' : ''}Still attached, no longer collected: ${clients.join(', ')}\n`) + stdout.write('Their requests can start failing until you run:\n') + for (const client of clients) stdout.write(` hyp detach --client ${client}\n`) +} + /** * Run the onboarding backfill step. For each picked client that has a * registered backfill provider (intersection of `clientsPicked` and diff --git a/src/core/cli/wizard/first_look.js b/src/core/cli/wizard/first_look.js index 2df8719c..9aa25e55 100644 --- a/src/core/cli/wizard/first_look.js +++ b/src/core/cli/wizard/first_look.js @@ -12,7 +12,7 @@ * This module owns only the wizard's half of the contract: which sections * run, when the step runs, and that it can never fail a finished install. * - * @import { FirstLookResult } from '../../../../src/core/cli/wizard/types.js' + * @import { FirstLookOutcome, FirstLookResult } from '../../../../src/core/cli/wizard/types.js' * @import { OverviewNotice, OverviewQueryRunner } from '../../../../src/core/query/types.js' */ @@ -140,7 +140,18 @@ async function withDeadline(promise, ms) { * materialized) degrades to a skipped step, because setup itself already * succeeded by the time this runs. * + * Returns `wrote` alongside `shown`, and it is *measured*: the writable the + * body sees is a counter in front of the caller's, so every branch, present + * or future, reports whether it put text on the screen. The two questions + * are not the same one. `shown` is "did the block render", and a caller that + * needs "did this push what came before it out of view" cannot infer that + * from `shown`: the `slow` skip renders no block and still writes two lines + * saying so. The wizard's closing repeat asks the second question + * (LLP 0230 #when), and inferring it from `shown` is what broke across the + * no-dataset, error and slow branches in turn. + * * @ref LLP 0135#first-look [implements]: setup ends on the user's own rows, and never fails on them + * @ref LLP 0230#when [implements]: the caller needs "wrote something", so measure it here rather than let the caller guess * * @param {{ * runner?: OverviewQueryRunner | undefined, @@ -150,8 +161,23 @@ async function withDeadline(promise, ms) { * }} args * @returns {Promise} */ -export async function runWizardFirstLook({ runner, stdout, color = false, budgetMs = FIRST_LOOK_BUDGET_MS }) { - return withSpan( +export async function runWizardFirstLook({ runner, stdout: target, color = false, budgetMs = FIRST_LOOK_BUDGET_MS }) { + /** @type {boolean} */ + let wrote = false + // The only `stdout` in scope below, so a branch cannot write without being + // counted. Set before delegating rather than after: a `write` that throws + // part-way (EPIPE on a closed pipe) may have emitted, and the safe error + // is an extra reminder, not a lost one. + /** @type {{ write(chunk: string): unknown }} */ + const stdout = { + /** @param {string} chunk */ + write(chunk) { + wrote = true + return target.write(chunk) + }, + } + /** @type {FirstLookOutcome} */ + const outcome = await withSpan( 'wizard.first_look', { [Attr.COMPONENT]: 'wizard', @@ -260,4 +286,7 @@ export async function runWizardFirstLook({ runner, stdout, color = false, budget }, { component: 'wizard' } ) + // One exit, so `wrote` is attached to whatever the body decided rather + // than restated per branch. + return { ...outcome, wrote } } diff --git a/src/core/cli/wizard/index.js b/src/core/cli/wizard/index.js index 568e258e..33207d90 100644 --- a/src/core/cli/wizard/index.js +++ b/src/core/cli/wizard/index.js @@ -6,6 +6,7 @@ * @import { CollectStatusOptions, HypAwareStatusReport } from '../../../../src/core/daemon/types.js' * @import { * FirstAskResult, + * FirstLookOutcome, * FirstLookResult, * InitWizardResult, * RunInitWizardOptions, @@ -23,7 +24,15 @@ import { discoverBundledPlugins } from '../../runtime/bundled.js' 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, buildWalkthroughClientDescriptorMap, defaultConfirmSelectPromptFactory, defaultPickerDetect, runPickerFinale, writeWalkthroughRunSummary } from '../walkthrough.js' +import { + LOCAL_INSTALL_RETENTION_DAYS, + buildWalkthroughClientDescriptorMap, + defaultConfirmSelectPromptFactory, + defaultPickerDetect, + runPickerFinale, + writeAttachedNotConfiguredReminder, + writeWalkthroughRunSummary, +} from '../walkthrough.js' import { isPromptBackError, isPromptCancelledError } from '../tui/runtime.js' import { useColor } from '../stdio.js' import { evaluateReturningGate, runWizardFork } from './fork.js' @@ -642,14 +651,26 @@ export async function runInitWizard(opts) { // still have to type. Attended and non-dry-run only: a scripted `--yes` // install gets no extra output, and a dry run has no writes to look at. // @ref LLP 0135#first-look [implements]: placed after the finale (backfill has landed) and before the privacy narration, which stays the last words + const firstLookRan = interactive && !cancelled && opts.finale?.dryRun !== true + // `firstLookResult.wrote` is whether the step put text on the screen, which + // is neither "it ran" nor "the block rendered". The step is documented to + // degrade rather than fail a finished install (LLP 0135 #first-look), and it + // degrades in two different ways: an unregistered dataset, an unreadable + // cache or a render that throws leave `firstLookRan` true and stdout + // untouched, while an expired deadline with nothing renderable writes two + // lines saying so and still reports `shown: false`. `runWizardFirstLook` + // measures the writes, so this is the fact itself rather than an inference + // from which branch it took, and a branch added later reports itself without + // a change here. /** @type {FirstLookResult | undefined} */ let firstLookResult - if (interactive && !cancelled && opts.finale?.dryRun !== true) { + if (firstLookRan) { const notices = firstLookNoticeSink(opts.stderr) firstLookResult = await runWizardFirstLook({ runner: opts.firstLook ?? firstLookRunnerFromCtx(opts.ctx, notices), stdout: opts.stdout, color: useColor(opts.stdout, opts.env), + ...(opts.firstLookBudgetMs !== undefined ? { budgetMs: opts.firstLookBudgetMs } : {}), }) // The abandoned queries from an expired deadline keep running and can // still resolve with a withheld-row report. Close the sink so that @@ -658,6 +679,37 @@ export async function runInitWizard(opts) { notices.close() } + // The finale already named these, before the daemon restart that strands + // them (LLP 0185 #warn-do-not-detach). That print is no longer on screen by + // the time this run ends: the summary, then the first look's block, then + // the narration below all follow it without a pause. Repeat it here, short, + // and only when this closing sequence actually wrote something, so the + // direct `runPickerWalkthrough` entry point (whose summary follows the + // finale with nothing in between) keeps its single print. + // + // `firstLookResult.wrote` is the whole condition, the team pathway included. + // It is read rather than `firstLookRan` because a first look that wrote + // nothing buried nothing, rather than `shown` because a skip that explains + // itself on stdout (the expired deadline) buried the finale's print exactly + // as a full render would, and rather than `pathway` because a `pathway` is + // only ever resolved on an interactive run: a team run that is neither + // cancelled nor a dry run has already run the first look, so + // `|| pathway === 'team'` would widen this to exactly the runs where the + // first look did *not* run (cancelled at the backfill consent, or a dry + // run). Every run either of those conditions would add wrote nothing + // between the finale and here, so the repeat would land a few lines under + // the print it repeats. + // @ref LLP 0230#when [constrained-by]: nothing written in between, no repeat + // @ref LLP 0230#repeat-at-the-end [implements]: the wizard repeats what its own closing output buried + const stranded = finaleSummary?.attachedNotConfigured ?? [] + if (stranded.length > 0 && firstLookResult?.wrote === true) { + writeAttachedNotConfiguredReminder({ + clients: stranded, + stdout: opts.stdout, + dryRun: opts.finale?.dryRun === true, + }) + } + // The wizard's last words on a run that enrolled: when the first upload // happens and that nothing has shipped yet (LLP 0100/0101, narration // only - the hold itself was written by the join lane's login). Keyed @@ -759,8 +811,12 @@ export async function runInitWizard(opts) { * cache: the dataset exists and holds nothing yet, which is exactly the * fresh-install case (LLP 0198#empty-cache). * + * Takes the outcome half, not the whole {@link FirstLookResult}: this + * question is answered from what the step found, and `wrote` (LLP 0230 + * #when) says nothing about whether the cache has rows. + * * @ref LLP 0198#empty-cache [tests]: `no-dataset`, `slow`, and `error` each resolve to a distinct answer; collapsing any two is the bug - * @param {FirstLookResult | undefined} result + * @param {FirstLookOutcome | undefined} result * @returns {boolean | undefined} */ export function firstLookHadRows(result) { diff --git a/src/core/cli/wizard/types.d.ts b/src/core/cli/wizard/types.d.ts index 6c1ac78b..66cf4e08 100644 --- a/src/core/cli/wizard/types.d.ts +++ b/src/core/cli/wizard/types.d.ts @@ -526,15 +526,25 @@ export interface RunWizardPickOptions { } /** - * What the first look did. `shown: false` is a normal outcome, not a + * What the first look decided. `shown: false` is a normal outcome, not a * failure: `no-dataset` when no gateway source was picked, `error` when the * query itself failed, `slow` when summarizing the cache would have * outlasted the step's budget (setup had already succeeded in every case). */ -export type FirstLookResult = +export type FirstLookOutcome = | { shown: true; providerRows: number; dayRows: number; partial?: true } | { shown: false; reason: 'no-dataset' | 'error' | 'slow' } +/** + * The outcome plus whether the step wrote anything to stdout, which is a + * different question and is measured rather than derived from `shown`: the + * `slow` skip renders no block and still writes two lines saying so, while + * `no-dataset` and `error` write nothing. A caller asking "did this push + * earlier output out of view" reads `wrote`; a caller asking "does the user + * now have their numbers" reads `shown` (LLP 0230 #when). + */ +export type FirstLookResult = FirstLookOutcome & { wrote: boolean } + /** * A client this run can actually start on a question: picked by the * user, and with its `contributes.client.launch` binary resolved on @@ -676,6 +686,12 @@ export interface RunInitWizardOptions { * built from `ctx`; the step is skipped when neither is available. */ firstLook?: OverviewQueryRunner + /** + * Override the first look's deadline (tests). Defaults to the step's own + * budget; a test driving the slow-skip branch through the orchestrator + * would otherwise have to wait out the real one. + */ + firstLookBudgetMs?: number /** * Overrides for the closing first ask (tests): the PATH resolver, the * spawn seam, the select seam. Production callers pass none, and the diff --git a/test/core/cli/wizard/first-look.test.js b/test/core/cli/wizard/first-look.test.js index 99e00bb1..17dda91b 100644 --- a/test/core/cli/wizard/first-look.test.js +++ b/test/core/cli/wizard/first-look.test.js @@ -63,7 +63,7 @@ test('runWizardFirstLook: writes the two setup sections, names the fuller comman // `hyp query overview` is where the other two live. // @ref LLP 0198#wizard-sections [tests]: assert.deepEqual(seen, [OVERVIEW_PROBE_SQL, SECTION_SQL.models, SECTION_SQL.daily]) - assert.deepEqual(result, { shown: true, providerRows: 1, dayRows: 1 }) + assert.deepEqual(result, { shown: true, providerRows: 1, dayRows: 1, wrote: true }) const text = stdout.text() assert.match(text, /First look at what HypAware has recorded/) assert.match(text, /claude-opus-5/) @@ -99,6 +99,7 @@ test('runWizardFirstLook: an expired deadline keeps the sections that finished', // What completed is shown rather than thrown away. assert.equal(result.shown, true) assert.equal(result.partial, true) + assert.equal(result.wrote, true) assert.match(text, /First look at what HypAware has recorded/) assert.match(text, /claude-opus-5/) // The unfinished section is named as unfinished, not as empty - and only @@ -129,7 +130,10 @@ test('runWizardFirstLook: a slow cache skips within budget and says what to run' }), }, }) - assert.deepEqual(result, { shown: false, reason: 'slow' }) + // The block did not render, and the step still put two lines on the + // screen. Both facts are reported, because callers ask both questions + // (LLP 0230 #when). + assert.deepEqual(result, { shown: false, reason: 'slow', wrote: true }) // Setup moved on rather than waiting out the query. assert.ok(Date.now() - started < 2000) assert.match(stdout.text(), /Skipped the first look/) @@ -150,6 +154,7 @@ test('runWizardFirstLook: a cache inside the budget still renders', async () => }, }) assert.equal(result.shown, true) + assert.equal(result.wrote, true) assert.match(stdout.text(), /claude-opus-5/) assert.ok(!stdout.text().includes('Skipped the first look')) }) @@ -160,7 +165,7 @@ test('runWizardFirstLook: an unregistered dataset skips silently', async () => { stdout, runner: { hasDataset: () => false, async run() { throw new Error('must not run') } }, }) - assert.deepEqual(result, { shown: false, reason: 'no-dataset' }) + assert.deepEqual(result, { shown: false, reason: 'no-dataset', wrote: false }) assert.equal(stdout.text(), '') }) @@ -170,7 +175,7 @@ test('runWizardFirstLook: a query failure degrades to a skipped step, not a thro stdout, runner: { hasDataset: () => true, async run() { throw new Error('cache unreadable') } }, }) - assert.deepEqual(result, { shown: false, reason: 'error' }) + assert.deepEqual(result, { shown: false, reason: 'error', wrote: false }) assert.equal(stdout.text(), '') }) @@ -200,7 +205,10 @@ test('runWizardFirstLook: a synchronous write failure cannot escape and fail a f }, }, }) - assert.deepEqual(result, { shown: false, reason: 'error' }) + // `wrote` is true here: the write was attempted and a partial chunk may + // have landed before the throw. It is counted before the delegation for + // exactly that reason, and the failure direction is the safe one. + assert.deepEqual(result, { shown: false, reason: 'error', wrote: true }) }) test('runWizardFirstLook: a render failure is contained too', async () => { @@ -220,7 +228,8 @@ test('runWizardFirstLook: a render failure is contained too', async () => { }, }, }) - assert.deepEqual(result, { shown: false, reason: 'error' }) + assert.deepEqual(result, { shown: false, reason: 'error', wrote: false }) + assert.equal(stdout.text(), '') }) test('firstLookNoticeSink: discloses withheld rows, drops the freshness line', async () => { @@ -248,6 +257,6 @@ test('firstLookNoticeSink: a closed sink drops a late disclosure', async () => { test('runWizardFirstLook: no runner (no query registry) skips', async () => { const stdout = makeBuf() const result = await runWizardFirstLook({ stdout }) - assert.deepEqual(result, { shown: false, reason: 'no-dataset' }) + assert.deepEqual(result, { shown: false, reason: 'no-dataset', wrote: false }) assert.equal(stdout.text(), '') }) diff --git a/test/core/cli/wizard/index.test.js b/test/core/cli/wizard/index.test.js index 10148370..24b454b5 100644 --- a/test/core/cli/wizard/index.test.js +++ b/test/core/cli/wizard/index.test.js @@ -824,6 +824,163 @@ test('runInitWizard: a non-interactive or dry run skips the first look', async ( assert.equal(dry.seen.length, 0) }) +// --- the stranded-attach warning's closing repeat (LLP 0230) --- + +/** + * A finale summary that reports clients this run left attached but no longer + * collects. The finale itself printed the full warning before the daemon + * restart; this is what it hands back for the closing repeat to read. + * + * @param {string[]} clients + */ +function strandedFinale(clients) { + return /** @type {any} */ ({ + daemonInstall: { skipped: true, dryRun: false }, + globalInstall: { skipped: true, installed: false }, + attach: [], + skillsInstalled: [], + agentsInstalled: [], + daemonRestart: { skipped: true, dryRun: false, ok: false }, + backfill: [], + attachedNotConfigured: clients, + }) +} + +// The finale names the stranded clients before the daemon restart (LLP 0185 +// #warn-do-not-detach) and then the wizard writes the run summary, the first +// look's ~60 lines, and the privacy narration on top of it, so by the time an +// attended run ends the warning has scrolled away. On a managed host it is +// the only signal there is, because `hyp status`'s mirror diagnostic is gated +// to hosts with no central layer. +// @ref LLP 0230#repeat-at-the-end [tests]: the wizard repeats what its own closing output buried +test('runInitWizard: an attended run repeats the stranded-attach warning after the first look', async () => { + const home = await tmpHome() + await writeFirstSyncHoldMarker({ stateDir: path.join(home, '.hyp', 'hypaware') }) + const stub = firstLookStub( + [{ provider: 'anthropic', model: 'claude-opus-5', input_tokens: 400, cached_tokens: 4000, output_tokens: 40 }], + [{ date: '2026-07-24', sessions: 3, input_tokens: 400, cached_tokens: 4000, output_tokens: 40 }] + ) + const { opts, stdout } = wizardOpts(home, { + fork: async () => 'team', + firstLook: stub.runner, + finaleRunner: async () => strandedFinale(['codex']), + }) + await runInitWizard(opts) + const text = stdout.text() + + // The names and the one command that clears each, not a bare mention. + assert.match(text, /Still attached, no longer collected: codex/, text) + assert.match(text, /hyp detach --client codex/, text) + // Past the block that buried the finale's own print. + assert.ok(text.indexOf('First look') >= 0, text) + assert.ok(text.indexOf('hyp detach --client codex') > text.indexOf('First look'), text) + // And still ahead of the privacy narration, which stays the last words. + assert.ok( + text.indexOf('hyp detach --client codex') < text.indexOf('Nothing has been uploaded yet'), + text + ) +}) + +// The repeat exists because the wizard's closing sequence buries the finale's +// print. A scripted run writes nothing between the two, so repeating there +// would be the double-print on one screen the shared run summary would have +// caused. Its output stays byte-identical to what the finale alone produced. +// @ref LLP 0230#when [tests]: no closing sequence, no repeat +test('runInitWizard: a scripted run does not repeat the stranded-attach warning', async () => { + const { opts, stdout } = wizardOpts(await tmpHome(), { + picks: { sources: ['claude'], exportChoice: 'local-parquet', retentionDays: 30 }, + finaleRunner: async () => strandedFinale(['codex']), + }) + await runInitWizard(opts) + assert.doesNotMatch(stdout.text(), /hyp detach --client/, stdout.text()) +}) + +// A cancel at the backfill consent skips the first look, so the run summary is +// the only thing between the finale's own warning (which the finale prints +// before its restart block, cancelled or not) and the end of the run. The team +// pathway is not on its own a reason to repeat: a pathway is only resolved on +// an interactive run, so an uncancelled non-dry team run has already run the +// first look, and the runs a `pathway === 'team'` clause would add are exactly +// the ones with nothing in between. +// @ref LLP 0230#when [tests]: a cancelled team run buried nothing, so it does not repeat +test('runInitWizard: a run cancelled at the finale does not repeat the stranded-attach warning', async () => { + const home = await tmpHome() + await writeFirstSyncHoldMarker({ stateDir: path.join(home, '.hyp', 'hypaware') }) + const { opts, stdout } = wizardOpts(home, { + fork: async () => 'team', + finaleRunner: async () => ({ ...strandedFinale(['codex']), cancelled: true }), + }) + const result = await runInitWizard(opts) + assert.equal(result.cancelled, true) + assert.doesNotMatch(stdout.text(), /hyp detach --client/, stdout.text()) +}) + +// The first look is documented to degrade rather than fail a finished install +// (LLP 0135 #first-look): an unregistered dataset, an unreadable cache, or a +// render that throws all leave an attended run that attempted the block and +// printed none of it. The gate that admits the repeat is therefore what the +// step wrote, not what it attempted: on a silent skip the finale's own print +// is still the last thing above the summary, and repeating under it would be +// the same-screen double print. +// @ref LLP 0230#when [tests]: a first look that printed nothing buried nothing +test('runInitWizard: an attended run whose first look skips itself does not repeat the stranded-attach warning', async () => { + const home = await tmpHome() + await writeFirstSyncHoldMarker({ stateDir: path.join(home, '.hyp', 'hypaware') }) + const { opts, stdout } = wizardOpts(home, { + fork: async () => 'team', + // The shape `firstLookRunnerFromCtx` yields when the overview dataset is + // not registered: the step returns `{ wrote: false }` without writing. + firstLook: { hasDataset: () => false, async run() { return { columns: [], rows: [] } } }, + finaleRunner: async () => strandedFinale(['codex']), + }) + await runInitWizard(opts) + const text = stdout.text() + assert.doesNotMatch(text, /First look/, text) + assert.doesNotMatch(text, /hyp detach --client/, text) +}) + +// The skip that is not silent, and the reason the gate measures writes rather +// than reading `shown`. When the deadline expires with nothing renderable +// (`reason: 'slow'`, the branch `FIRST_LOOK_BUDGET_MS` exists for: a +// pathological day, a disk that stalls), the block does not render and two +// lines saying so do land on stdout. Those lines, plus the run summary and +// the privacy narration, bury the finale's own warning exactly as a full +// render would, so this run must repeat it. A gate reading `shown` drops the +// repeat here, which on a managed host is the only signal there is: LLP 0185 +// #status-backstop gates `hyp status`'s mirror diagnostic off on a joined +// machine. +// @ref LLP 0230#when [tests]: a skip that still wrote buried the finale's print, so it repeats +test('runInitWizard: an attended run whose first look skips slowly still repeats the stranded-attach warning', async () => { + const home = await tmpHome() + await writeFirstSyncHoldMarker({ stateDir: path.join(home, '.hyp', 'hypaware') }) + const { opts, stdout } = wizardOpts(home, { + fork: async () => 'team', + firstLook: { + hasDataset: () => true, + // Far longer than the budget below, so no section ever lands. `unref` + // so the abandoned query does not hold the test runner open. + run: () => new Promise((resolve) => { + setTimeout(() => resolve({ columns: [], rows: [] }), 5000).unref() + }), + }, + firstLookBudgetMs: 40, + finaleRunner: async () => strandedFinale(['codex']), + }) + await runInitWizard(opts) + const text = stdout.text() + // The block itself never rendered. + assert.match(text, /Skipped the first look/, text) + assert.doesNotMatch(text, /First look at what HypAware has recorded/, text) + // The repeat still ran, under what the skip wrote and ahead of the privacy + // narration, which stays the last words. + assert.match(text, /Still attached, no longer collected: codex/, text) + assert.ok(text.indexOf('hyp detach --client codex') > text.indexOf('Skipped the first look'), text) + assert.ok( + text.indexOf('hyp detach --client codex') < text.indexOf('Nothing has been uploaded yet'), + text + ) +}) + /** * A first look that finds something, so the closing first ask has data * for its questions to be about (LLP 0198#empty-cache). diff --git a/test/core/walkthrough-unpicked-attached-clients.test.js b/test/core/walkthrough-unpicked-attached-clients.test.js index ff6ddcea..9f0ba2bb 100644 --- a/test/core/walkthrough-unpicked-attached-clients.test.js +++ b/test/core/walkthrough-unpicked-attached-clients.test.js @@ -89,6 +89,13 @@ test('the finale names a still-attached client the new config no longer collects const out = stdout.text() assert.match(out, /codex/, out) assert.match(out, /hyp detach --client codex/, out) + // Exactly once. This entry point writes only its short run summary after the + // finale, so the warning is still on screen when the run ends and repeating + // it (the shape an echo from the shared `writeWalkthroughRunSummary` would + // take) would print the same paragraph twice within a few lines of itself. + // @ref LLP 0230#repeat-at-the-end [tests]: the repeat belongs to the caller that buried the first print, and this one does not + assert.equal(out.match(/hyp detach --client codex/g)?.length, 1, out) + assert.equal(out.match(/Still attached, no longer collected/g)?.length, 1, out) }) test('a picked client that stays configured draws no stranded-attach warning', async () => {