From e25b45bb675017b4c7033332442c18a4e226c1fc Mon Sep 17 00:00:00 2001 From: Brendan McMullen Date: Mon, 27 Jul 2026 17:43:13 -0700 Subject: [PATCH 1/2] hyp sync: one export verb, and a confirmed way out of the first-sync hold (LLP 0101/0100) An onboarding session could not show data on the server. The enrolling login writes a first-sync hold marker (LLP 0101), and the driver returns `{ sinks: [], held: 'first_sync_hold' }` before it ever consults `force: true`, so `hyp sink force` printed "no sinks instantiated; nothing to do" - which is the opposite of the truth - and nothing could end the window early. On an evening enrollment the 4-hour floor rolls the deadline to the *next* day's 11:59pm, so "wait until midnight" could mean 28 hours. LLP 0101 #no-release rejected exactly this verb, for three reasons. Two survive: the printed message stays unconditionally true (reworded to "no later than"), and release-on-review-completion still needs a completion signal from the skill that a finished skill cannot honestly give. The third - "costs only latency on a machine that was not forwarding yesterday either" - is what this concedes. On an attended onboarding the hold blocks the demonstration that the product works, and the window is sized for a review (hypaware-privacy) too slow to run in the meeting the hold was scoped to. #which holds the attended lane while unattended `hyp join` forwards immediately, which is backwards for that case. LLP 0069 R6 ("not forwarded, even once") is untouched: R6 forbids a *silent* first forward, and an explicit confirmation naming the destination is the consent it exists to obtain. The daemon never clears the marker, so an unattended machine still waits out the full window. `hyp sync [instance] [--yes] [--dry-run]` replaces `hyp sink force` rather than aliasing it: both were one `driver.tick({force: true})`, and a second spelling preserves the confusion it was introduced to fix (the #399 doctrine). It always confirms - one rule beats a prompt whose appearance the user has to predict - and prints what it is about to send first: every destination named, annotated off-machine only on the evidence of an http(s) URL and on-machine only on the evidence of a path, plus the directories being withheld. "Are you sure?" with nothing to be sure about is a keystroke, not a decision. `hyp sink maintain` stays; snapshot expiration is not an export tick. The three surfaces that print the deadline (login message, wizard privacy narration, `hyp status`) now name the verb, and the formatted deadline carries its time zone - ECMA-402 forbids mixing timeZoneName with dateStyle/timeStyle, so the zone comes from a second formatter. `purge` and `report delete` had grown one copy each of the same y/N prompt; all three now share src/core/cli/confirm.js with their question text unchanged. LLP 0101 #no-release is amended with the original preserved as a quote, and LLP 0100 R2 ("No export tick may run before the deadline") with it - it was flatly false once this lands. R1 now requires the zone and the release verb. Co-Authored-By: Claude Opus 5 (1M context) --- .../format-parquet/src/index.js | 4 +- .../smoke/flows/local_parquet_export.js | 16 +- llp/0100-enrollment-privacy-review.spec.md | 19 +- llp/0101-first-sync-review-window.decision.md | 49 ++- src/core/cli/confirm.js | 55 ++++ src/core/cli/core_commands.js | 17 +- src/core/cli/remote_commands.js | 13 +- src/core/cli/report_commands.js | 24 +- src/core/cli/wizard/index.js | 4 +- src/core/commands/purge.js | 27 +- src/core/commands/sink.js | 49 --- src/core/commands/status.js | 2 +- src/core/commands/sync.js | 274 +++++++++++++++++ src/core/usage-policy/first_sync_hold.js | 41 ++- test/core/sync-command.test.js | 288 ++++++++++++++++++ 15 files changed, 754 insertions(+), 128 deletions(-) create mode 100644 src/core/cli/confirm.js create mode 100644 src/core/commands/sync.js create mode 100644 test/core/sync-command.test.js diff --git a/hypaware-core/plugins-workspace/format-parquet/src/index.js b/hypaware-core/plugins-workspace/format-parquet/src/index.js index bdb4dc11..fea84a54 100644 --- a/hypaware-core/plugins-workspace/format-parquet/src/index.js +++ b/hypaware-core/plugins-workspace/format-parquet/src/index.js @@ -31,7 +31,7 @@ const DEFAULT_MAX_CLUSTER_KEYS = 16 const DEFAULT_MAX_ROWS_PER_GROUP = 50_000 // Hard ceiling on how many estimated row bytes accumulate in one in-memory // group before it is written out as a row group and freed. This is the knob -// that bounds peak heap during a sink force: the encoder never holds more than +// that bounds peak heap during a forced sync: the encoder never holds more than // ~one group (plus its columnar copy) at once, instead of materializing the // whole partition. Independent of blob size, so a fat-`tools` partition cannot // push a group into the gigabytes. @@ -183,7 +183,7 @@ async function encodePartition(partition, ctx, settings) { // Derive a stable schema from the declared column types (not from the // data) so we can write row groups incrementally: never holding more // than one cluster group of rows (plus its columnar copy) in memory. - // This is what stops `hyp sink force` on a large partition from OOMing + // This is what stops `hyp sync` on a large partition from OOMing // while materializing the whole partition at once. const schema = schemaFromColumnData({ columnData: rowsToColumnSources(columns, []) }) const writer = new ByteWriter() diff --git a/hypaware-core/smoke/flows/local_parquet_export.js b/hypaware-core/smoke/flows/local_parquet_export.js index 063989aa..b4ae7cd5 100644 --- a/hypaware-core/smoke/flows/local_parquet_export.js +++ b/hypaware-core/smoke/flows/local_parquet_export.js @@ -27,7 +27,7 @@ import { dispatch } from '../../../src/core/cli/dispatch.js' * ENOTDIR. The driver then routes the failed batch * into `/sinks/broken/outbox/.json`. * - * The forced tick is driven through `hyp sink force` so the CLI + * The forced tick is driven through `hyp sync` so the CLI * surface is exercised, not just the in-process driver. Dispatch is * handed the daemon's runtime via `opts.kernel` because Phase 7 does * not yet wire config-driven sink instantiation. The smoke owns the @@ -40,7 +40,7 @@ import { dispatch } from '../../../src/core/cli/dispatch.js' * - The good sink produces a Parquet file decodable by * `parquetReadObjects` containing the captured log row. * - The broken sink's failure lands in the outbox. - * - `hyp sink force good` exits 0 and the report mentions the + * - `hyp sync --yes` exits 0 and the report mentions the * instance as `exported`. * - Daemon self-telemetry includes `source.start` (otlp), * `sink.tick`, `sink.export_batch` (status=ok for `good`, !=ok @@ -144,11 +144,13 @@ export async function run({ harness, expect }) { (v) => v !== undefined && v.kind === 'blob', ) - // ----- Drive the forced tick through the CLI (`hyp sink force`) ----- + // ----- Drive the forced tick through the CLI (`hyp sync`) ----- + // `--yes` because the smoke has no TTY and `hyp sync` always confirms + // before exporting (LLP 0101 #no-release). const forceStdout = makeBuf() const forceStderr = makeBuf() const forceCode = await dispatch( - ['sink', 'force'], + ['sync', '--yes'], { stdout: forceStdout, stderr: forceStderr, @@ -157,18 +159,18 @@ export async function run({ harness, expect }) { }, ) expect.that( - `dispatch: hyp sink force exited 0 (stderr=${forceStderr.text()})`, + `dispatch: hyp sync exited 0 (stderr=${forceStderr.text()})`, forceCode, (v) => v === 0, ) const forceOut = forceStdout.text() expect.that( - 'stdout: hyp sink force reported the good instance as exported', + 'stdout: hyp sync reported the good instance as exported', forceOut, (v) => typeof v === 'string' && /good: exported/.test(v), ) expect.that( - 'stdout: hyp sink force reported the broken instance as partial (failed mkdir)', + 'stdout: hyp sync reported the broken instance as partial (failed mkdir)', forceOut, (v) => typeof v === 'string' && /broken: (partial|failed)/.test(v), ) diff --git a/llp/0100-enrollment-privacy-review.spec.md b/llp/0100-enrollment-privacy-review.spec.md index cc04e104..f607b71c 100644 --- a/llp/0100-enrollment-privacy-review.spec.md +++ b/llp/0100-enrollment-privacy-review.spec.md @@ -101,10 +101,12 @@ Its job, in order: ## Requirements {#requirements} - **R1.** An attended enrolling login MUST print the first-sync deadline as an - absolute local time and the skill invocation hint, and MUST state that the - first sync includes backfilled history. A non-TTY enrolling login gets the - same hold and the same message on stderr; nothing prompts - ([LLP 0063 D3](./0063-login-auto-provision-forward-sink.decision.md#d3) + absolute local time **with its time zone**, the skill invocation hint, and + the verb that ends the window early (`hyp sync`), and MUST state that the + first sync includes backfilled history. A deadline the reader cannot place + on a clock, or cannot act on, is a countdown rather than a choice. A non-TTY + enrolling login gets the same hold and the same message on stderr; nothing + prompts ([LLP 0063 D3](./0063-login-auto-provision-forward-sink.decision.md#d3) stands). - **R1a.** The enrolling login's destination surfaces - the forwarding line and the privacy block - MUST name the server by its configured target name and @@ -120,9 +122,12 @@ Its job, in order: paths (`already connected to `, the replaced-identity note) still print origins, where the origin is the fact the user must act on. Revisit if the server root becomes a browsable landing page. -- **R2.** No export tick may run before the deadline - ([LLP 0101](./0101-first-sync-review-window.decision.md)); the hold MUST be - written before `enrollCentralSink` so no daemon tick can precede it. +- **R2.** *(amended 2026-07-27, [LLP 0101 #no-release](./0101-first-sync-review-window.decision.md#no-release))* + No export tick may run before the deadline **except one the user releases + through a confirmed `hyp sync`**; the hold MUST be written before + `enrollCentralSink` so no daemon tick can precede it, and the daemon MUST + NOT clear the marker itself. Originally: "No export tick may run before the + deadline." - **R3.** The skill MUST opt its own session out of capture ([LLP 0066](./0066-session-opt-out.spec.md)) as its first action and verify success; on failure it MUST say so and continue only with explicit user diff --git a/llp/0101-first-sync-review-window.decision.md b/llp/0101-first-sync-review-window.decision.md index 19861809..08e7799f 100644 --- a/llp/0101-first-sync-review-window.decision.md +++ b/llp/0101-first-sync-review-window.decision.md @@ -9,8 +9,10 @@ > On an attended enrolling login, the sink driver exports nothing until a > **printed, absolute deadline**: the next local 11:59pm, rolled to the -> following day when that is less than four hours away. No early release, no -> extension. Supersedes the pick-pending marker of +> following day when that is less than four hours away. The deadline is the +> latest the first sync can happen, not the earliest: a confirmed `hyp sync` +> ends the window early (amended 2026-07-27, see [#no-release](#no-release)). +> No extension. Supersedes the pick-pending marker of > [LLP 0093](./0093-pick-pending-export-hold.decision.md), generalizing its > enforcement point from a 10-minute picker guard to an hours-long review > window. @@ -60,12 +62,35 @@ deadline; `createSinkDriver.tick` exports nothing while `now < deadline`.** fleet rollout by hours would be the surprise in the other direction. Re-logins hold nothing: the daemon is already forwarding, so there is no "first" sync to defer. -- **No early release** {#no-release}: rejected a release verb ("sync now") - and release-on-review-completion. The hold simply runs to its deadline, - even for a user who finished the review in ten minutes. This keeps the - printed message unconditionally true, needs no completion signal from the - skill, and costs only latency on a machine that was not forwarding - yesterday either. +- **Release only by confirmed, attended request** {#no-release} + *(amended 2026-07-27; the original text is kept below it)*: `hyp sync` + prints what would leave, warns that the window is open and that sent + history cannot be un-sent, and on an explicit `y` clears the marker and + exports. Release-on-review-completion stays rejected: it would need a + completion signal from the skill, and a skill that finished is not the + same as a user who decided. The daemon never clears the marker, so an + unattended machine still waits out the full window. + + What the amendment concedes is the third clause of the original: the cost + is *not* only latency. On an attended onboarding the hold blocks the + demonstration that the product works at all, and the person running it has + no way to say "I have seen enough". Worse, the window is sized for a review + (`hypaware-privacy`) too slow to run in the meeting the hold was scoped to + (`#which` holds the attended lane and lets unattended `hyp join` forward + immediately, which is backwards for exactly this case). Nothing in + [LLP 0069](./0069-local-only-dir-selection.spec.md) R6 is given up: R6 + forbids a *silent* first forward, and an explicit confirmation naming the + destination is the consent it exists to obtain. + + The first clause survives with a wording change: the printed message says + "no later than ", so it stays unconditionally true. + + > *Original (2026-07-13):* **No early release**: rejected a release verb + > ("sync now") and release-on-review-completion. The hold simply runs to + > its deadline, even for a user who finished the review in ten minutes. + > This keeps the printed message unconditionally true, needs no completion + > signal from the skill, and costs only latency on a machine that was not + > forwarding yesterday either. - **No extension**: bounded always. A hold that can be pushed out is a kill switch with extra steps ([LLP 0093 #bounded](./0093-pick-pending-export-hold.decision.md#bounded)). @@ -77,7 +102,13 @@ deadline; `createSinkDriver.tick` exports nothing while `now < deadline`.** - Live capture during the window lands in the cache and ships at the deadline unless marked; watermarks ([LLP 0040](./0040-incremental-sink-reads.design.md)) make the first post-deadline tick a plain catch-up. -- `hyp status` shows the pending deadline (LLP 0100 R9). +- `hyp status` shows the pending deadline (LLP 0100 R9), and names `hyp sync` + beside it: a countdown the reader cannot act on is where the original + decision's cost was hiding. +- The three surfaces that print the deadline (the login message, the wizard's + privacy narration, `hyp status`) all name `hyp sync`, and the formatted + deadline carries its time zone - an absolute time is only memorable if the + reader knows which clock it is on. - LLP 0093's pick-pending semantics retire with the picker ([LLP 0102](./0102-skill-replaces-enrollment-picker.decision.md)); its driver-side hold machinery is reused, resized, and renamed. diff --git a/src/core/cli/confirm.js b/src/core/cli/confirm.js new file mode 100644 index 00000000..6dffa411 --- /dev/null +++ b/src/core/cli/confirm.js @@ -0,0 +1,55 @@ +// @ts-check + +import process from 'node:process' +import readline from 'node:readline/promises' + +import { isTty } from './stdio.js' + +/** + * @import { CommandRunContext } from '../../../hypaware-plugin-kernel-types.js' + */ + +/** + * Ask a y/N question on the interactive terminal. The question goes to + * stderr, not stdout, so a command's machine-readable output stays clean + * for a caller that pipes it. + * + * Anything other than `y`/`yes` is a no: the default has to be the safe + * one for a verb nobody can undo. + * + * @param {CommandRunContext} ctx + * @param {string} question rendered verbatim, including its `[y/N]` suffix + * @returns {Promise} + */ +export async function askYesNo(ctx, question) { + const rl = readline.createInterface({ + input: /** @type {NodeJS.ReadableStream} */ (ctx.stdin ?? process.stdin), + output: /** @type {NodeJS.WritableStream} */ (/** @type {unknown} */ (ctx.stderr)), + }) + try { + const answer = await rl.question(question) + return /^y(es)?$/i.test(answer.trim()) + } finally { + rl.close() + } +} + +/** + * The confirmation posture `hyp purge` established and every irreversible + * verb has followed since (LLP 0104, LLP 0111#delete-confirm): prompt on an + * interactive TTY, require an explicit `--yes` anywhere else. A command that + * cannot ask must not assume. + * + * Returning a discriminant rather than a boolean keeps the refusal text with + * the command: "refusing to purge" and "refusing to sync" name different + * flags in their hints, and a shared string would have to be vague about + * both. + * + * @param {{ ctx: CommandRunContext, yes: boolean, question: string }} opts + * @returns {Promise<'confirmed' | 'declined' | 'no-tty'>} + */ +export async function requireConfirmation({ ctx, yes, question }) { + if (yes) return 'confirmed' + if (!isTty(ctx.stdin)) return 'no-tty' + return (await askYesNo(ctx, question)) ? 'confirmed' : 'declined' +} diff --git a/src/core/cli/core_commands.js b/src/core/cli/core_commands.js index 9a554642..c6f7d0f8 100644 --- a/src/core/cli/core_commands.js +++ b/src/core/cli/core_commands.js @@ -36,7 +36,8 @@ import { } from '../commands/daemon.js' import { runMcp } from '../commands/mcp.js' import { runSmoke, runVersion } from '../commands/misc.js' -import { runSinkForce, runSinkMaintain } from '../commands/sink.js' +import { runSinkMaintain } from '../commands/sink.js' +import { runSync } from '../commands/sync.js' import { runInit } from '../commands/init.js' import { runJoin, runLeave } from '../commands/central.js' import { runPurge } from '../commands/purge.js' @@ -406,17 +407,17 @@ function buildCoreCommands(registry) { usage: 'hyp daemon restart', run: runDaemonRestart, }, + { + name: 'sync', + summary: 'Send captured data to its destinations now, after confirming what leaves', + usage: 'hyp sync [instance] [--yes] [--dry-run]', + run: runSync, + }, makeGroupCommand({ registry, name: 'sink', - summary: 'Manage sink instances (force, maintain)', + summary: 'Maintain sink instances (to export now, see `hyp sync`)', }), - { - name: 'sink force', - summary: 'Force the sink driver to fire a tick now (optionally for one instance)', - usage: 'hyp sink force [instance]', - run: runSinkForce, - }, { name: 'sink maintain', summary: 'Run export maintenance (snapshot expiration; data-file compaction with --compact) on table-format sinks', diff --git a/src/core/cli/remote_commands.js b/src/core/cli/remote_commands.js index 0876d128..be22bac7 100644 --- a/src/core/cli/remote_commands.js +++ b/src/core/cli/remote_commands.js @@ -202,7 +202,13 @@ const FIRST_SYNC_RULE = '─'.repeat(62) * this block is deliberately self-contained (stderr, while the forwarding line * is stdout - redirect either and the other must still stand on its own). * - * @ref LLP 0100#requirements [implements]: R1 - absolute deadline, backfill statement, skill hint, same on TTY and non-TTY + * The deadline is the latest the first sync can happen, not the earliest + * (LLP 0101 #no-release, as amended), so it reads "no later than" and the + * block names the verb that ends the window early. A deadline with no way to + * act on it is a countdown, which is how this message read in the onboarding + * session that prompted both changes. + * + * @ref LLP 0100#requirements [implements]: R1 - absolute deadline with its zone, backfill statement, skill hint, release verb, same on TTY and non-TTY * @ref LLP 0100#requirements [implements]: R1a - name the server, never its URL, and name the command that maps the name back * @param {number} deadlineMs * @param {string} serverName @@ -214,12 +220,15 @@ export function firstSyncHoldMessage(deadlineMs, serverName) { `${FIRST_SYNC_RULE}\n` + ' PRIVACY - review before first sync\n' + '\n' + - ` first sync to the '${serverName}' server is ${formatFirstSyncDeadline(deadlineMs)}\n` + + ` first sync to the '${serverName}' server is no later than\n` + + ` ${formatFirstSyncDeadline(deadlineMs)}\n` + ' and includes your backfilled history\n' + " (run 'hyp remote list' to see that server's URL)\n" + '\n' + ' to review what ships before then,\n' + ' open Claude or Codex and run the hypaware-privacy skill\n' + + '\n' + + ' to send it sooner, run: hyp sync\n' + `${FIRST_SYNC_RULE}\n` + '\n' ) diff --git a/src/core/cli/report_commands.js b/src/core/cli/report_commands.js index 74617b22..1cf96c92 100644 --- a/src/core/cli/report_commands.js +++ b/src/core/cli/report_commands.js @@ -4,10 +4,10 @@ import crypto from 'node:crypto' import fs from 'node:fs/promises' import path from 'node:path' import process from 'node:process' -import readline from 'node:readline/promises' import { execFile } from 'node:child_process' import { promisify } from 'node:util' +import { askYesNo } from './confirm.js' import { readObservabilityEnv } from '../observability/env.js' import { effectiveDefaultRemote, effectiveRemotes } from '../remote/builtin_remotes.js' import { @@ -296,7 +296,10 @@ export async function runReportDelete(argv, ctx) { ctx.stderr.write('error: refusing to delete without confirmation - pass --yes to delete non-interactively\n') return 2 } - const ok = await confirm(ctx, `${kind}/${period}/${id}`) + const ok = await askYesNo( + ctx, + `Delete report ${kind}/${period}/${id} for the whole org? This cannot be undone. [y/N] ` + ) if (!ok) { ctx.stdout.write('delete cancelled\n') return 0 @@ -500,20 +503,3 @@ async function fileExists(p) { } } -/** - * @param {CommandRunContext} ctx - * @param {string} what - * @returns {Promise} - */ -async function confirm(ctx, what) { - const rl = readline.createInterface({ - input: /** @type {NodeJS.ReadableStream} */ (ctx.stdin ?? process.stdin), - output: /** @type {NodeJS.WritableStream} */ (/** @type {unknown} */ (ctx.stderr)), - }) - try { - const answer = await rl.question(`Delete report ${what} for the whole org? This cannot be undone. [y/N] `) - return /^y(es)?$/i.test(answer.trim()) - } finally { - rl.close() - } -} diff --git a/src/core/cli/wizard/index.js b/src/core/cli/wizard/index.js index 943b5499..1e0ef36c 100644 --- a/src/core/cli/wizard/index.js +++ b/src/core/cli/wizard/index.js @@ -328,7 +328,9 @@ async function narratePrivacyIfTeamPath(opts) { 'Nothing has been uploaded yet - nothing leaves this machine before\n' + `${formatFirstSyncDeadline(deadline)}. That first sync includes your imported history.\n` + 'To review or exclude anything before then, run the hypaware-privacy\n' + - 'skill in Claude or Codex. `hyp status` shows the countdown.\n' + 'skill in Claude or Codex. `hyp status` shows the countdown.\n' + + 'To send it sooner, run `hyp sync`: it shows what would leave and asks\n' + + 'before sending anything.\n' ) } diff --git a/src/core/commands/purge.js b/src/core/commands/purge.js index e900eacf..9f1599b1 100644 --- a/src/core/commands/purge.js +++ b/src/core/commands/purge.js @@ -3,8 +3,8 @@ import { createHash } from 'node:crypto' import path from 'node:path' import process from 'node:process' -import readline from 'node:readline/promises' +import { askYesNo } from '../cli/confirm.js' import { parseCommandArgv } from '../cli/verb_codec.js' import { isTty } from '../cli/stdio.js' import { Attr, getLogger, withSpan } from '../observability/index.js' @@ -58,7 +58,10 @@ export async function runPurge(argv, ctx) { ) return 2 } - const ok = await confirm(ctx, describeTarget(target)) + const ok = await askYesNo( + ctx, + `Permanently delete ${describeTarget(target)} from the local cache? [y/N] ` + ) if (!ok) { ctx.stdout.write('purge cancelled\n') return 0 @@ -177,26 +180,6 @@ function hashTargetToken(target) { return createHash('sha256').update(token).digest('hex').slice(0, 16) } -/** - * Interactive y/N confirmation for the destructive verb. Only reached when - * stdin is a TTY (the non-TTY path requires `--yes`). - * - * @param {CommandRunContext} ctx - * @param {string} what - * @returns {Promise} - */ -async function confirm(ctx, what) { - const rl = readline.createInterface({ - input: /** @type {NodeJS.ReadableStream} */ (ctx.stdin ?? process.stdin), - output: /** @type {NodeJS.WritableStream} */ (/** @type {unknown} */ (ctx.stderr)), - }) - try { - const answer = await rl.question(`Permanently delete ${what} from the local cache? [y/N] `) - return /^y(es)?$/i.test(answer.trim()) - } finally { - rl.close() - } -} /** * @param {string[]} argv diff --git a/src/core/commands/sink.js b/src/core/commands/sink.js index 732ca4a3..b2b02199 100644 --- a/src/core/commands/sink.js +++ b/src/core/commands/sink.js @@ -1,62 +1,13 @@ // @ts-check -import { readObservabilityEnv } from '../observability/env.js' import { parseCommandArgv } from '../cli/verb_codec.js' /** * @import { CommandRunContext } from '../../../hypaware-plugin-kernel-types.js' - * @import { ExtendedQueryStorageService } from '../../../src/core/cache/types.js' * @import { ExportMaintenanceDatasetReport } from '../../../hypaware-core/plugins-workspace/format-iceberg/src/types.js' * @import { ExtendedSinkRegistry } from '../../../src/core/registry/types.js' */ -/** - * `hyp sink force [instance]` - * - * Drives one tick of the sink driver immediately, bypassing each - * sink's cron schedule. The optional `instance` argument restricts - * the tick to a single sink (useful when an operator just wants to - * flush one configured destination without waking the others. - * - * The driver writes the same `sink.export_batch` span and outbox - * artifacts it does on a scheduled tick. The only difference is the - * trigger. - * - * @param {string[]} argv - * @param {CommandRunContext} ctx - */ -export async function runSinkForce(argv, ctx) { - const instance = argv[0] - const obsEnv = readObservabilityEnv(ctx.env) - const { createSinkDriver } = await import('../sinks/driver.js') - const driver = createSinkDriver({ - sinkRegistry: /** @type {ExtendedSinkRegistry} */ (ctx.sinks), - queryRegistry: ctx.query, - storage: /** @type {ExtendedQueryStorageService} */ (ctx.storage), - stateRoot: obsEnv.stateDir, - config: ctx.config, - }) - const tickOpts = { now: new Date(), force: true, source: /** @type {'manual'} */ ('manual') } - if (instance) /** @type {any} */ (tickOpts).sinkInstance = instance - const report = await driver.tick(tickOpts) - if (report.sinks.length === 0) { - if (instance) { - ctx.stderr.write(`hyp sink force: no sink named '${instance}' was instantiated\n`) - return 1 - } - ctx.stdout.write('no sinks instantiated; nothing to do\n') - return 0 - } - for (const r of report.sinks) { - ctx.stdout.write( - `${r.instance}: ${r.status} (partitions=${r.partitionsExported}, bytes=${r.bytesWritten}${ - r.error ? `, error=${r.error}` : '' - })\n` - ) - } - return report.sinks.some((r) => r.status === 'failed') ? 1 : 0 -} - /** * `hyp sink maintain [instance] [--compact] [--dry-run]` * diff --git a/src/core/commands/status.js b/src/core/commands/status.js index 5ff23cfe..eafc1307 100644 --- a/src/core/commands/status.js +++ b/src/core/commands/status.js @@ -382,7 +382,7 @@ export function renderStatusText({ report, clientNames, datasets, cacheRoot, std // text output is unchanged. if (report.firstSyncHoldDeadline !== null) { stdout.write( - ` first sync: held until ${formatFirstSyncDeadline(report.firstSyncHoldDeadline)} (review with the hypaware-privacy skill)\n` + ` first sync: held until ${formatFirstSyncDeadline(report.firstSyncHoldDeadline)} (review with the hypaware-privacy skill; \`hyp sync\` sends it now)\n` ) } diff --git a/src/core/commands/sync.js b/src/core/commands/sync.js new file mode 100644 index 00000000..2b1ffe05 --- /dev/null +++ b/src/core/commands/sync.js @@ -0,0 +1,274 @@ +// @ts-check + +import { requireConfirmation } from '../cli/confirm.js' +import { parseCommandArgv } from '../cli/verb_codec.js' +import { readObservabilityEnv } from '../observability/env.js' +import { + clearFirstSyncHold, + formatFirstSyncDeadline, + readFirstSyncDeadline, +} from '../usage-policy/first_sync_hold.js' +import { readLocalOnlyEntries } from '../usage-policy/index.js' + +/** + * @import { CommandRunContext } from '../../../hypaware-plugin-kernel-types.js' + * @import { ExtendedQueryStorageService } from '../../../src/core/cache/types.js' + * @import { ExtendedSinkHandle, ExtendedSinkRegistry } from '../../../src/core/registry/types.js' + */ + +const USAGE = 'usage: hyp sync [instance] [--yes] [--dry-run]' + +/** + * `hyp sync [instance] [--yes] [--dry-run]` + * + * Export every configured sink now, rather than on its cron schedule. The + * user-facing name for the one action the driver performs; it replaced + * `hyp sink force`, which spelled the same tick in the driver's vocabulary + * instead of the user's. + * + * Two things make this more than a scheduling shortcut: + * + * 1. **It always confirms.** Not only when data leaves the machine: one + * unconditional rule beats a prompt whose appearance the user has to + * predict. `--yes` is the scripted bypass, as it is for `hyp purge`. + * 2. **It is the only way to end the first-sync review window early.** While + * that hold is live the prompt escalates (see {@link renderFirstSyncWarning}) + * and a confirmation clears the marker. + * + * @ref LLP 0101#no-release [implements]: supersedes "no early release" - a confirmed, attended release verb + * @param {string[]} argv + * @param {CommandRunContext} ctx + * @returns {Promise} + */ +export async function runSync(argv, ctx) { + const parsed = parseCommandArgv( + argv, + { + type: 'object', + properties: { + instance: { type: 'string' }, + yes: { type: 'boolean', default: false }, + 'dry-run': { type: 'boolean', default: false }, + }, + positional: ['instance'], + }, + { aliases: { '-y': '--yes' } } + ) + if ('help' in parsed) { + ctx.stdout.write(`${USAGE}\n`) + return 0 + } + if (!parsed.ok) { + ctx.stderr.write(`hyp sync: ${parsed.error}\n${USAGE}\n`) + return 2 + } + const { instance, yes, 'dry-run': dryRun } = + /** @type {{ instance?: string, yes: boolean, 'dry-run': boolean }} */ (parsed.params) + + const allHandles = /** @type {ExtendedSinkRegistry} */ (ctx.sinks).listHandles?.() ?? [] + const handles = instance ? allHandles.filter((h) => h.instanceName === instance) : allHandles + if (instance && handles.length === 0) { + ctx.stderr.write(`hyp sync: no sink named '${instance}' was instantiated\n`) + const available = allHandles.map((h) => h.instanceName) + if (available.length > 0) ctx.stderr.write(` available: ${available.join(', ')}\n`) + return 1 + } + if (handles.length === 0) { + ctx.stdout.write('no sinks instantiated; nothing to do\n') + return 0 + } + + const stateDir = readObservabilityEnv(ctx.env).stateDir + const deadline = await readFirstSyncDeadline({ stateDir }) + const destinations = handles.map((handle) => describeDestination(handle)) + + ctx.stdout.write(renderPlan({ destinations, exclusions: await readExclusions(stateDir) })) + if (deadline !== null) ctx.stdout.write(renderFirstSyncWarning(deadline)) + + if (dryRun) { + ctx.stdout.write('\n[dry-run] nothing was sent\n') + return 0 + } + + const outcome = await requireConfirmation({ + ctx, + yes, + question: deadline !== null + ? 'Send now and end the review window? [y/N] ' + : `Send now to ${describeScope(destinations)}? [y/N] `, + }) + if (outcome === 'no-tty') { + ctx.stderr.write( + 'error: refusing to sync without confirmation - pass --yes to send non-interactively\n' + ) + return 2 + } + if (outcome === 'declined') { + ctx.stdout.write('sync cancelled\n') + return 0 + } + + // Release before the tick, because the driver reads the marker itself and + // would otherwise hold the very export just consented to. A failed export + // leaves the window ended: consent was given, and the retry belongs to the + // ordinary schedule rather than to a window that has served its purpose. + if (deadline !== null) await clearFirstSyncHold({ stateDir }) + + const { createSinkDriver } = await import('../sinks/driver.js') + const driver = createSinkDriver({ + sinkRegistry: /** @type {ExtendedSinkRegistry} */ (ctx.sinks), + queryRegistry: ctx.query, + storage: /** @type {ExtendedQueryStorageService} */ (ctx.storage), + stateRoot: stateDir, + config: ctx.config, + }) + /** @type {{ now: Date, force: true, source: 'manual', sinkInstance?: string }} */ + const tickOpts = { now: new Date(), force: true, source: 'manual' } + if (instance) tickOpts.sinkInstance = instance + const report = await driver.tick(tickOpts) + + for (const r of report.sinks) { + ctx.stdout.write( + `${r.instance}: ${r.status} (partitions=${r.partitionsExported}, bytes=${r.bytesWritten}${ + r.error ? `, error=${r.error}` : '' + })\n` + ) + } + return report.sinks.some((r) => r.status === 'failed') ? 1 : 0 +} + +/** + * Where a sink writes, and whether that is off this machine. + * + * The driver deliberately has no notion of which sinks leave the host + * ([LLP 0101 #hold](../../../llp/0101-first-sync-review-window.decision.md#hold) + * holds all of them for exactly that reason), so this reads the instance's + * own config rather than inventing a registration concept for one prompt. + * An `http(s)` destination is off-machine on the evidence of the URL; a + * filesystem path is on-machine on the evidence of the path. Anything else + * reports `null` and the summary stays silent about it - a confirmation + * prompt that guesses is worse than one that admits the gap. + * + * @param {ExtendedSinkHandle} handle + * @returns {{ instance: string, text: string, offMachine: boolean | null }} + */ +function describeDestination(handle) { + const config = /** @type {Record} */ (handle.config ?? {}) + const url = config.url + if (typeof url === 'string' && /^https?:\/\//i.test(url)) { + return { instance: handle.instanceName, text: url, offMachine: true } + } + const dir = config.dir + if (typeof dir === 'string' && dir.length > 0) { + return { instance: handle.instanceName, text: dir, offMachine: false } + } + return { instance: handle.instanceName, text: handle.plugin ?? 'unknown destination', offMachine: null } +} + +/** + * Count the directories the machine-local policy lists withhold, so the + * prompt can state what is *not* going as well as what is. + * + * An unreadable list is reported, never swallowed and never fatal: the query + * seam (not this summary) is what actually withholds those rows, so a failed + * read changes what the user is told, not what ships. + * + * @param {string} stateDir + * @returns {Promise<{ localOnly: number, ignore: number } | { error: string }>} + */ +async function readExclusions(stateDir) { + try { + const entries = await readLocalOnlyEntries({ stateDir }) + return { + localOnly: entries.filter((e) => e.class === 'local-only').length, + ignore: entries.filter((e) => e.class === 'ignore').length, + } + } catch (err) { + return { error: err instanceof Error ? err.message : String(err) } + } +} + +/** + * The pre-confirmation summary: every destination, named, with the + * exclusions that will not travel. "Are you sure?" with nothing to be sure + * *about* is a keystroke, not a decision. + * + * @param {{ + * destinations: { instance: string, text: string, offMachine: boolean | null }[], + * exclusions: { localOnly: number, ignore: number } | { error: string }, + * }} args + * @returns {string} + */ +function renderPlan({ destinations, exclusions }) { + const width = Math.max(...destinations.map((d) => d.instance.length)) + const lines = [`hyp sync: ${plural(destinations.length, 'destination')}\n`, '\n'] + for (const dest of destinations) { + const note = dest.offMachine === true + ? ' (leaves this machine)' + : dest.offMachine === false + ? ' (stays on this machine)' + : '' + lines.push(` ${dest.instance.padEnd(width)} ${dest.text}${note}\n`) + } + lines.push('\n') + if ('error' in exclusions) { + lines.push(` warning: could not read the local-only list (${exclusions.error});\n`) + lines.push(' exclusions still apply, but cannot be summarized here\n') + } else if (exclusions.localOnly > 0 || exclusions.ignore > 0) { + const parts = [] + if (exclusions.localOnly > 0) parts.push(`${plural(exclusions.localOnly, 'directory', 'directories')} marked local-only`) + if (exclusions.ignore > 0) parts.push(`${plural(exclusions.ignore, 'directory', 'directories')} marked ignore`) + lines.push(` withholding ${parts.join(', ')}\n`) + } else { + lines.push(' no directories are marked local-only or ignore\n') + } + return lines.join('') +} + +/** + * The escalated warning shown while the first-sync review window is open. + * + * This is the one prompt in the CLI where the user has never sent anything + * before, so it says so, states what confirming gives up (the rest of the + * window), and names the command that excludes a folder - a warning that + * only warns leaves the user with no move except yes or no. + * + * @ref LLP 0100#requirements [implements]: R2's review window ends by deadline or by informed consent + * @param {number} deadlineMs + * @returns {string} + */ +function renderFirstSyncWarning(deadlineMs) { + return ( + '\n' + + ' FIRST SYNC - nothing has left this machine yet\n' + + '\n' + + ` Your review window runs until ${formatFirstSyncDeadline(deadlineMs)}.\n` + + ' Syncing now ends it early and sends your backfilled history.\n' + + ' What has been sent cannot be un-sent. To exclude a folder first:\n' + + ' hyp policy set local-only\n' + ) +} + +/** + * Name the scope of an ordinary (unheld) confirmation by where the data + * goes, so the question is answerable without scrolling back to the plan. + * + * @param {{ offMachine: boolean | null }[]} destinations + * @returns {string} + */ +function describeScope(destinations) { + const offMachine = destinations.filter((d) => d.offMachine === true).length + if (offMachine === 0) return plural(destinations.length, 'destination') + if (offMachine === destinations.length) return `${plural(offMachine, 'destination')} off this machine` + return `${plural(destinations.length, 'destination')} (${offMachine} off this machine)` +} + +/** + * @param {number} n + * @param {string} singular + * @param {string} [pluralForm] + * @returns {string} + */ +function plural(n, singular, pluralForm = `${singular}s`) { + return `${n} ${n === 1 ? singular : pluralForm}` +} diff --git a/src/core/usage-policy/first_sync_hold.js b/src/core/usage-policy/first_sync_hold.js index 64ecd2e7..76fde72a 100644 --- a/src/core/usage-policy/first_sync_hold.js +++ b/src/core/usage-policy/first_sync_hold.js @@ -50,12 +50,25 @@ export function computeFirstSyncDeadline(now = Date.now()) { * R1) and `hyp status` (R9) render "the deadline", so the two consent * surfaces cannot drift apart on wording. * + * The zone is named. The deadline is computed in the machine's local time, + * but a user reading "11:59 PM" on a printed line has no way to know whether + * it means their zone, the server's, or UTC, and the answer decides whether + * they wait an hour or a day. + * * @ref LLP 0100#requirements [implements]: shared formatting keeps R1's login message and R9's status line in sync * @param {number} deadlineMs epoch ms * @returns {string} */ export function formatFirstSyncDeadline(deadlineMs) { - return new Date(deadlineMs).toLocaleString(undefined, { dateStyle: 'medium', timeStyle: 'short' }) + const at = new Date(deadlineMs) + const stamp = at.toLocaleString(undefined, { dateStyle: 'medium', timeStyle: 'short' }) + // ECMA-402 forbids mixing `timeZoneName` with the `dateStyle`/`timeStyle` + // shorthands, so the zone comes from a second formatter and is appended. + // A runtime that names no zone yields the bare stamp rather than an error. + const zone = new Intl.DateTimeFormat(undefined, { timeZoneName: 'short' }) + .formatToParts(at) + .find((part) => part.type === 'timeZoneName')?.value + return zone ? `${stamp} ${zone}` : stamp } /** @@ -99,6 +112,32 @@ export async function writeFirstSyncHoldMarker({ stateDir, now = Date.now(), fs return deadlineMs } +/** + * Remove the hold marker, ending the review window before its deadline. + * + * The deadline is a promise about the *latest* moment the first sync can + * happen, not the earliest: a user who has finished reviewing (or who never + * wanted the wait) can end the window by running `hyp sync` and confirming + * an explicit prompt. That confirmation is the consent + * [LLP 0069](../../../llp/0069-local-only-dir-selection.spec.md) R6 protects; + * what R6 forbids is a *silent* first forward, which nothing here can cause. + * + * Only ever called behind that prompt. The daemon never clears the marker, + * so an unattended machine still waits out the full window. + * + * @ref LLP 0101#no-release [implements]: the confirmed `hyp sync` is the one path that ends the window early + * @param {{ stateDir: string, fs?: typeof fsp }} opts + * @returns {Promise} true when a marker was removed, false when none was present + */ +export async function clearFirstSyncHold({ stateDir, fs = fsp }) { + try { + await fs.unlink(firstSyncHoldMarkerPath(stateDir)) + return true + } catch { + return false + } +} + /** * Read the live first-sync hold deadline, or `null` when no hold applies. * Never throws. diff --git a/test/core/sync-command.test.js b/test/core/sync-command.test.js new file mode 100644 index 00000000..618d2454 --- /dev/null +++ b/test/core/sync-command.test.js @@ -0,0 +1,288 @@ +// @ts-check + +import test from 'node:test' +import assert from 'node:assert/strict' +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import { PassThrough } from 'node:stream' + +import { runSync } from '../../src/core/commands/sync.js' +import { + firstSyncHoldMarkerPath, + writeFirstSyncHoldMarker, +} from '../../src/core/usage-policy/first_sync_hold.js' +import { writeLocalOnlyEntries } from '../../src/core/usage-policy/index.js' + +// `hyp sync` (LLP 0101 #no-release, as amended): the user-facing export verb +// that replaced `hyp sink force`. What these cover is the consent gate, not +// the tick - the driver's export path is already covered by the sink tests +// and the local_parquet_export smoke. +// +// The load-bearing claims: +// 1. Nothing exports without a confirmation, in either tier. +// 2. Confirming during the review window is the one thing that ends it early. +// 3. Declining, or having no TTY to ask, leaves the window intact. +// @ref LLP 0101#no-release [tests]: the confirmed release path and its refusals + +/** @param {string} prefix */ +async function makeHome(prefix) { + const hypHome = await fs.mkdtemp(path.join(os.tmpdir(), `hyp-sync-${prefix}-`)) + await fs.mkdir(path.join(hypHome, 'hypaware'), { recursive: true }) + return hypHome +} + +/** @param {string} hypHome */ +function stateDir(hypHome) { + return path.join(hypHome, 'hypaware') +} + +function captureStream() { + let buf = '' + return { + write(/** @type {string} */ chunk) { buf += String(chunk); return true }, + get text() { return buf }, + } +} + +/** + * A sink handle the driver can tick, recording every exportBatch call so a + * test can assert that nothing was sent. + * + * @param {string} instanceName + * @param {Record} config + * @param {{ status?: string }} [result] + */ +function fakeSink(instanceName, config, result = {}) { + /** @type {unknown[]} */ + const exported = [] + return { + instanceName, + plugin: '@hypaware/fake', + kind: 'blob', + config, + exported, + sink: { + async exportBatch(/** @type {unknown} */ batch) { + exported.push(batch) + return { status: result.status ?? 'exported', partitionsExported: 0, bytesWritten: 0 } + }, + }, + } +} + +/** + * @param {{ hypHome: string, sinks: any[], tty?: boolean, answer?: string }} args + */ +function makeCtx({ hypHome, sinks, tty = false, answer }) { + const stdout = captureStream() + const stderr = captureStream() + const stdin = Object.assign(new PassThrough(), { isTTY: tty }) + if (answer !== undefined) stdin.write(`${answer}\n`) + const ctx = /** @type {any} */ ({ + stdout, + stderr, + stdin, + env: { HYP_HOME: hypHome, HYP_CONFIG: '' }, + cwd: '/home/u', + config: { version: 2 }, + query: { listDatasets: () => [] }, + storage: { + cacheRoot: path.join(hypHome, 'cache'), + tableExists: () => false, + hasPendingSync: () => false, + async flushTable() {}, + }, + sinks: { listHandles: () => sinks }, + }) + return { ctx, stdout, stderr } +} + +/** @param {string} hypHome */ +async function holdExists(hypHome) { + try { + await fs.access(firstSyncHoldMarkerPath(stateDir(hypHome))) + return true + } catch { + return false + } +} + +test('no TTY and no --yes: refuses, exports nothing, and leaves the hold standing', async () => { + const hypHome = await makeHome('no-tty') + await writeFirstSyncHoldMarker({ stateDir: stateDir(hypHome) }) + const sink = fakeSink('central', { url: 'https://hypaware.example.com' }) + const { ctx, stderr } = makeCtx({ hypHome, sinks: [sink], tty: false }) + + const code = await runSync([], ctx) + + assert.equal(code, 2) + assert.match(stderr.text, /refusing to sync without confirmation/) + assert.deepEqual(sink.exported, [], 'a refusal must not export') + assert.ok(await holdExists(hypHome), 'a refusal must not end the review window') +}) + +test('declining at the prompt cancels, exports nothing, and leaves the hold standing', async () => { + const hypHome = await makeHome('decline') + await writeFirstSyncHoldMarker({ stateDir: stateDir(hypHome) }) + const sink = fakeSink('central', { url: 'https://hypaware.example.com' }) + const { ctx, stdout } = makeCtx({ hypHome, sinks: [sink], tty: true, answer: 'n' }) + + const code = await runSync([], ctx) + + assert.equal(code, 0, 'declining is a normal outcome, not an error') + assert.match(stdout.text, /sync cancelled/) + assert.deepEqual(sink.exported, []) + assert.ok(await holdExists(hypHome), 'declining must not end the review window') +}) + +test('confirming during the review window ends it and exports', async () => { + const hypHome = await makeHome('confirm-held') + await writeFirstSyncHoldMarker({ stateDir: stateDir(hypHome) }) + const sink = fakeSink('central', { url: 'https://hypaware.example.com' }) + const { ctx, stdout } = makeCtx({ hypHome, sinks: [sink], tty: true, answer: 'y' }) + + const code = await runSync([], ctx) + + assert.equal(code, 0) + assert.equal(sink.exported.length, 1, 'a confirmed sync exports') + assert.equal(await holdExists(hypHome), false, 'the marker is cleared, not merely bypassed') + assert.match(stdout.text, /central: exported/) +}) + +test('the held prompt states the window, the irreversibility, and the way out', async () => { + const hypHome = await makeHome('held-warning') + await writeFirstSyncHoldMarker({ stateDir: stateDir(hypHome) }) + const { ctx, stdout } = makeCtx({ + hypHome, + sinks: [fakeSink('central', { url: 'https://hypaware.example.com' })], + tty: true, + answer: 'n', + }) + + await runSync([], ctx) + + const text = stdout.text + assert.match(text, /FIRST SYNC - nothing has left this machine yet/) + assert.match(text, /Your review window runs until /) + assert.match(text, /cannot be un-sent/) + assert.match(text, /hyp policy set local-only/) +}) + +test('--dry-run prints the plan, exports nothing, and keeps the window open', async () => { + const hypHome = await makeHome('dry-run') + await writeFirstSyncHoldMarker({ stateDir: stateDir(hypHome) }) + const sink = fakeSink('central', { url: 'https://hypaware.example.com' }) + // A TTY with no answer queued: a dry run must not reach the prompt at all, + // so this would hang if it did. + const { ctx, stdout } = makeCtx({ hypHome, sinks: [sink], tty: true }) + + const code = await runSync(['--dry-run'], ctx) + + assert.equal(code, 0) + assert.match(stdout.text, /\[dry-run\] nothing was sent/) + assert.deepEqual(sink.exported, []) + assert.ok(await holdExists(hypHome)) +}) + +test('the plan names each destination and whether it leaves the machine', async () => { + const hypHome = await makeHome('plan') + const { ctx, stdout } = makeCtx({ + hypHome, + sinks: [ + fakeSink('central', { url: 'https://hypaware.example.com' }), + fakeSink('parquet', { dir: '/home/u/exports' }), + fakeSink('mystery', {}), + ], + tty: true, + }) + + await runSync(['--dry-run'], ctx) + + const text = stdout.text + assert.match(text, /central\s+https:\/\/hypaware\.example\.com\s+\(leaves this machine\)/) + assert.match(text, /parquet\s+\/home\/u\/exports\s+\(stays on this machine\)/) + // An undeclarable destination says nothing rather than guessing either way. + assert.match(text, /mystery\s+@hypaware\/fake\n/) +}) + +test('the plan counts the directories being withheld', async () => { + const hypHome = await makeHome('exclusions') + await writeLocalOnlyEntries({ + stateDir: stateDir(hypHome), + entries: [ + { dir: '/home/u/secret', class: 'local-only' }, + { dir: '/home/u/other', class: 'local-only' }, + { dir: '/home/u/never', class: 'ignore' }, + ], + }) + const { ctx, stdout } = makeCtx({ + hypHome, + sinks: [fakeSink('central', { url: 'https://hypaware.example.com' })], + tty: true, + }) + + await runSync(['--dry-run'], ctx) + + assert.match(stdout.text, /withholding 2 directories marked local-only, 1 directory marked ignore/) +}) + +test('with no hold, --yes exports without inventing a review window', async () => { + const hypHome = await makeHome('unheld') + const sink = fakeSink('parquet', { dir: '/home/u/exports' }) + const { ctx, stdout } = makeCtx({ hypHome, sinks: [sink] }) + + const code = await runSync(['--yes'], ctx) + + assert.equal(code, 0) + assert.equal(sink.exported.length, 1) + assert.doesNotMatch(stdout.text, /FIRST SYNC/) +}) + +test('an unknown instance names the ones that exist', async () => { + const hypHome = await makeHome('unknown') + const { ctx, stderr } = makeCtx({ + hypHome, + sinks: [fakeSink('central', { url: 'https://hypaware.example.com' })], + }) + + const code = await runSync(['nope', '--yes'], ctx) + + assert.equal(code, 1) + assert.match(stderr.text, /no sink named 'nope'/) + assert.match(stderr.text, /available: central/) +}) + +test('an instance argument ticks only that sink', async () => { + const hypHome = await makeHome('one-instance') + const central = fakeSink('central', { url: 'https://hypaware.example.com' }) + const parquet = fakeSink('parquet', { dir: '/home/u/exports' }) + const { ctx } = makeCtx({ hypHome, sinks: [central, parquet] }) + + const code = await runSync(['parquet', '--yes'], ctx) + + assert.equal(code, 0) + assert.equal(parquet.exported.length, 1) + assert.deepEqual(central.exported, [], 'a named instance must not wake the others') +}) + +test('no sinks at all is a no-op, not an error', async () => { + const hypHome = await makeHome('no-sinks') + const { ctx, stdout } = makeCtx({ hypHome, sinks: [] }) + + const code = await runSync([], ctx) + + assert.equal(code, 0) + assert.match(stdout.text, /no sinks instantiated; nothing to do/) +}) + +test('a failed export reports a nonzero exit', async () => { + const hypHome = await makeHome('failed') + const sink = fakeSink('parquet', { dir: '/home/u/exports' }, { status: 'failed' }) + const { ctx, stdout } = makeCtx({ hypHome, sinks: [sink] }) + + const code = await runSync(['--yes'], ctx) + + assert.equal(code, 1) + assert.match(stdout.text, /parquet: failed/) +}) From d61f99dea8dd8fe205a5b7164a70fb035f497166 Mon Sep 17 00:00:00 2001 From: Brendan McMullen Date: Mon, 27 Jul 2026 21:08:59 -0700 Subject: [PATCH 2/2] Review round 1: the hold releases only on unscoped, attended consent Six findings from neutral's review of e25b45b. 1. BLOCKER. `hyp sync ` released the driver-wide hold after a plan built from the one named handle. `hyp sync parquet` on a machine with a central sink showed "parquet /home/u/exports (stays on this machine)", and a `y` there ended the window for `central` too - the next daemon tick then forwarded the whole backfilled history with nothing on screen. That is the silent first forward the hold exists to prevent (LLP 0101 #hold: "one missed forward hold is a permanent leak"), and it falsified this PR's own headline property. A scoped run now refuses while a hold is live and says why. Releasing is all-or-nothing because the hold is; rendering the full destination list from a command that ticks one sink would have been the other option, but "I asked for one sink and it released everything" is a surprise the refusal avoids entirely. 2. A failed unlink turned a confirmed sync into a silent exit 0: the marker survived, the driver held the tick, and the loop over `report.sinks` printed nothing. That is the defect this PR opens by describing, reintroduced one path over. The clear is now checked, the marker re-read, and `report.held` inspected after the tick; each is an error with the marker path, not a quiet success. 3. The release emitted no telemetry, and clearing the marker destroys the only on-disk evidence it happened, so nothing afterwards distinguished "the window expired" from "somebody ran hyp sync". Logs `sync.first_sync_hold_released` with the deadline, how early it was, and the destination counts. 4. `--yes` released a live hold, which the clause titled "Release only by confirmed, attended request" does not license. It now refuses while held. `--yes` still works for ordinary syncs; what it must not buy is somebody's review window. 5. Three prose surfaces still said the deadline was the only way the window ends: README, and the hypaware-privacy skill in both the claude and codex copies - the skill being the surface LLP 0100 designates for the review itself, telling a user mid-review that their window cannot end early. 6. The plan printed the raw server URL that R1a was landed to remove. R1a binds the login's surfaces by its text, but its reason is about terminals, and this prompt appears at the same onboarding moment: servers are now named from `query.remotes` by origin, with the `hyp remote list` pointer, falling back to the host when no name is configured. Both refusals run before the plan renders: a scoped plan is the misleading artifact finding 1 is about, and printing "syncing now ends it early" ahead of "you cannot end it this way" reads as a contradiction. `--dry-run` is exempt from both - it sends nothing, so showing a held machine what one destination would export is information, not consent. LLP 0101 #no-release and LLP 0100 R2 record the two shapes that cannot release and why, so the constraint is not left living in code comments. Tests: the instance-plus-hold combination the review identified as never exercised, plus --yes-plus-hold, an unclearable marker, and the two naming paths. 16 in the file, 2760 in the suite. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 11 +- .../claude/skills/hypaware-privacy/SKILL.md | 4 +- .../codex/skills/hypaware-privacy/SKILL.md | 4 +- llp/0100-enrollment-privacy-review.spec.md | 8 +- llp/0101-first-sync-review-window.decision.md | 17 +++ src/core/commands/sync.js | 133 +++++++++++++++++- test/core/sync-command.test.js | 90 +++++++++++- 7 files changed, 246 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 29386c1a..ee78dc4c 100644 --- a/README.md +++ b/README.md @@ -119,11 +119,12 @@ hyp query sql "select count(*) from ai_gateway_messages" --remote ``` **Privacy review before anything ships.** Nothing is forwarded immediately. -The first sync (which includes backfilled history) waits until at least -11:59pm local time on the day you enroll, and the login prints the exact -deadline. Before then, open Claude or Codex and run the `hypaware-privacy` -skill to review what will ship, mark directories ignore / local-only / sync, -and purge anything sensitive. +The first sync (which includes backfilled history) waits until 11:59pm local +time on the day you enroll, and the login prints the exact deadline. Before +then, open Claude or Codex and run the `hypaware-privacy` skill to review what +will ship, mark directories ignore / local-only / sync, and purge anything +sensitive. Done reviewing, or want to see data on the server sooner? `hyp sync` +shows what would leave and sends it once you confirm. Useful login flags: `--no-forward` signs in for remote queries only (no enrollment), `--no-browser` prints the sign-in URL instead of opening one, diff --git a/hypaware-core/plugins-workspace/claude/skills/hypaware-privacy/SKILL.md b/hypaware-core/plugins-workspace/claude/skills/hypaware-privacy/SKILL.md index a4ddfb3b..330e4b7d 100644 --- a/hypaware-core/plugins-workspace/claude/skills/hypaware-privacy/SKILL.md +++ b/hypaware-core/plugins-workspace/claude/skills/hypaware-privacy/SKILL.md @@ -57,7 +57,7 @@ hyp status --json # daemon running? enrolled (a central sink present)? hyp query status # cache state and last refresh ``` -Then run the enumeration query (Step 3) **twice, a short interval apart** (say ~30-60s). If the per-directory `rows` counts are still climbing, backfill is still landing: **warn the user and offer to wait** until counts stabilize before proposing any markings. Surveying mid-backfill risks marking against an incomplete picture. There is no deadline pressure here - the first-sync hold gives hours. +Then run the enumeration query (Step 3) **twice, a short interval apart** (say ~30-60s). If the per-directory `rows` counts are still climbing, backfill is still landing: **warn the user and offer to wait** until counts stabilize before proposing any markings. Surveying mid-backfill risks marking against an incomplete picture. There is no deadline pressure here - the first-sync hold gives hours. Note the user can also end that window early at any time with `hyp sync` (it prints what would leave and asks first), so if they say they are in a hurry, finishing the review is what unblocks them, not waiting. ## Step 3 - Survey the captured directories, then sample content (R4 applies) @@ -137,6 +137,6 @@ hyp policy set ignore && hyp purge ## After the review -- Nothing you did contacts the server. At the deadline, the hold expires and export begins: `ignore`d data was never recorded (or was purged), `local-only` rows are withheld at the export seam, and everything else - the `sync` directories and anything left at the default - ships, backfill included. +- Nothing you did contacts the server. At the deadline - or sooner, if the user runs `hyp sync` and confirms the prompt - the hold expires and export begins: `ignore`d data was never recorded (or was purged), `local-only` rows are withheld at the export seam, and everything else - the `sync` directories and anything left at the default - ships, backfill included. - Check the pending deadline any time with `hyp status` (it shows the first-sync deadline while the hold is live). - Re-running this skill later is safe and idempotent; already-decided directories drop out of the survey. diff --git a/hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md b/hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md index 21274a18..aa291d71 100644 --- a/hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md +++ b/hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md @@ -85,7 +85,7 @@ hyp status --json # daemon running? enrolled (a central sink present)? hyp query status # cache state and last refresh ``` -Then run the enumeration query (Step 3) **twice, a short interval apart** (say ~30-60s). If the per-directory `rows` counts are still climbing, backfill is still landing: **warn the user and offer to wait** until counts stabilize before proposing any markings. Surveying mid-backfill risks marking against an incomplete picture. There is no deadline pressure here - the first-sync hold gives hours. +Then run the enumeration query (Step 3) **twice, a short interval apart** (say ~30-60s). If the per-directory `rows` counts are still climbing, backfill is still landing: **warn the user and offer to wait** until counts stabilize before proposing any markings. Surveying mid-backfill risks marking against an incomplete picture. There is no deadline pressure here - the first-sync hold gives hours. Note the user can also end that window early at any time with `hyp sync` (it prints what would leave and asks first), so if they say they are in a hurry, finishing the review is what unblocks them, not waiting. ## Step 3 - Survey the captured directories, then sample content (R4 applies) @@ -165,6 +165,6 @@ hyp policy set ignore && hyp purge ## After the review -- Nothing you did contacts the server. At the deadline, the hold expires and export begins: `ignore`d data was never recorded (or was purged), `local-only` rows are withheld at the export seam, and everything else - the `sync` directories and anything left at the default - ships, backfill included. +- Nothing you did contacts the server. At the deadline - or sooner, if the user runs `hyp sync` and confirms the prompt - the hold expires and export begins: `ignore`d data was never recorded (or was purged), `local-only` rows are withheld at the export seam, and everything else - the `sync` directories and anything left at the default - ships, backfill included. - Check the pending deadline any time with `hyp status` (it shows the first-sync deadline while the hold is live). - Re-running this skill later is safe and idempotent; already-decided directories drop out of the survey. diff --git a/llp/0100-enrollment-privacy-review.spec.md b/llp/0100-enrollment-privacy-review.spec.md index f607b71c..303c4064 100644 --- a/llp/0100-enrollment-privacy-review.spec.md +++ b/llp/0100-enrollment-privacy-review.spec.md @@ -124,10 +124,12 @@ Its job, in order: the server root becomes a browsable landing page. - **R2.** *(amended 2026-07-27, [LLP 0101 #no-release](./0101-first-sync-review-window.decision.md#no-release))* No export tick may run before the deadline **except one the user releases - through a confirmed `hyp sync`**; the hold MUST be written before + through an interactive, unscoped `hyp sync` whose plan named every + destination the release unblocks**; the hold MUST be written before `enrollCentralSink` so no daemon tick can precede it, and the daemon MUST - NOT clear the marker itself. Originally: "No export tick may run before the - deadline." + NOT clear the marker itself. An instance-scoped run and `--yes` MUST refuse + to release, since the hold is driver-wide and their consent is not. + Originally: "No export tick may run before the deadline." - **R3.** The skill MUST opt its own session out of capture ([LLP 0066](./0066-session-opt-out.spec.md)) as its first action and verify success; on failure it MUST say so and continue only with explicit user diff --git a/llp/0101-first-sync-review-window.decision.md b/llp/0101-first-sync-review-window.decision.md index 08e7799f..e83c2518 100644 --- a/llp/0101-first-sync-review-window.decision.md +++ b/llp/0101-first-sync-review-window.decision.md @@ -71,6 +71,23 @@ deadline; `createSinkDriver.tick` exports nothing while `now < deadline`.** same as a user who decided. The daemon never clears the marker, so an unattended machine still waits out the full window. + Two shapes of `hyp sync` cannot release, because the consent they carry is + narrower than what the release does: + + - **Instance-scoped** (`hyp sync `) refuses while a hold is live. + The hold is driver-wide (`#hold`), so a plan built from one named handle + omits every destination the release would unblock; confirming it would + forward the others unseen. Releasing is all-or-nothing because the hold + is. + - **`--yes`** refuses while a hold is live. "Attended" is the operative + word: a provisioning script is not a person, and its destination list + scrolls past in a log nobody reads. `--yes` still works for ordinary + syncs; what it must not buy is somebody's review window. + + A release that cannot be completed (an unlink failure) is an error, not a + quiet no-op: the marker survives, the driver holds the tick, and the + command must say so rather than exit 0 having sent nothing. + What the amendment concedes is the third clause of the original: the cost is *not* only latency. On an attended onboarding the hold blocks the demonstration that the product works at all, and the person running it has diff --git a/src/core/commands/sync.js b/src/core/commands/sync.js index 2b1ffe05..67590c84 100644 --- a/src/core/commands/sync.js +++ b/src/core/commands/sync.js @@ -2,9 +2,12 @@ import { requireConfirmation } from '../cli/confirm.js' import { parseCommandArgv } from '../cli/verb_codec.js' +import { Attr, getLogger } from '../observability/index.js' import { readObservabilityEnv } from '../observability/env.js' +import { effectiveRemotes } from '../remote/builtin_remotes.js' import { clearFirstSyncHold, + firstSyncHoldMarkerPath, formatFirstSyncDeadline, readFirstSyncDeadline, } from '../usage-policy/first_sync_hold.js' @@ -41,6 +44,7 @@ const USAGE = 'usage: hyp sync [instance] [--yes] [--dry-run]' * @returns {Promise} */ export async function runSync(argv, ctx) { + const log = getLogger('sync') const parsed = parseCommandArgv( argv, { @@ -80,7 +84,49 @@ export async function runSync(argv, ctx) { const stateDir = readObservabilityEnv(ctx.env).stateDir const deadline = await readFirstSyncDeadline({ stateDir }) - const destinations = handles.map((handle) => describeDestination(handle)) + const remotes = effectiveRemotes(ctx.config) + const destinations = handles.map((handle) => describeDestination(handle, remotes)) + + // Two refusals, both because the hold is driver-wide (LLP 0101 #hold) + // while the consent in front of it would not be. They come before the plan + // is rendered: a scoped plan is exactly the misleading artifact the first + // refusal exists to prevent, and printing "syncing now ends it early" + // ahead of "you cannot end it this way" reads as a contradiction. + // + // A named instance shows a plan built from that one handle, so confirming + // it would release every *other* destination unseen - the silent first + // forward the hold exists to prevent. Releasing is all-or-nothing because + // the hold is. + // + // `--yes` skips the plan's whole purpose. `#no-release` licenses release by + // "confirmed, attended request" and "an explicit `y`"; a provisioning + // script satisfies neither, and its destination list scrolls past in a log + // nobody reads. Ordinary (unheld) syncs keep `--yes` - what it must not buy + // is somebody's review window. + // + // `--dry-run` is exempt: it sends nothing, so showing a held machine what + // one destination would export is information, not consent. + if (deadline !== null && !dryRun) { + const held = `hyp sync: the first-sync review window is open until ${formatFirstSyncDeadline(deadline)}.\n` + if (instance) { + ctx.stderr.write( + held + + ` Ending it is all-or-nothing: the hold covers every destination, so releasing it\n` + + ` from a run that names only '${instance}' would forward the others unseen.\n` + + ' Run `hyp sync` with no instance to review every destination and release,\n' + + ' or wait for the deadline.\n' + ) + return 2 + } + if (yes) { + ctx.stderr.write( + held + + ' Ending it early takes an interactive confirmation, so --yes cannot do it.\n' + + ' Run `hyp sync` from a terminal, or wait for the deadline.\n' + ) + return 2 + } + } ctx.stdout.write(renderPlan({ destinations, exclusions: await readExclusions(stateDir) })) if (deadline !== null) ctx.stdout.write(renderFirstSyncWarning(deadline)) @@ -112,7 +158,35 @@ export async function runSync(argv, ctx) { // would otherwise hold the very export just consented to. A failed export // leaves the window ended: consent was given, and the retry belongs to the // ordinary schedule rather than to a window that has served its purpose. - if (deadline !== null) await clearFirstSyncHold({ stateDir }) + // + // A failed unlink must not read as success. The marker survives, the driver + // holds the tick, and without this check the command would print nothing + // further and exit 0 - the exact "opposite of the truth" output this verb + // was written to replace. + if (deadline !== null) { + const cleared = await clearFirstSyncHold({ stateDir }) + const stillHeld = await readFirstSyncDeadline({ stateDir }) + if (!cleared || stillHeld !== null) { + ctx.stderr.write( + 'hyp sync: could not end the review window - the hold marker could not be removed\n' + + ` ${firstSyncHoldMarkerPath(stateDir)}\n` + + ' Nothing was sent. Check the file\'s permissions and re-run.\n' + ) + return 1 + } + // The one moment a machine's history becomes forwardable ahead of its + // deadline, and clearing the marker destroys the only on-disk evidence it + // happened. Without this line nothing afterwards distinguishes "the window + // expired" from "somebody released it". + log.info('sync.first_sync_hold_released', { + [Attr.COMPONENT]: 'cmd-sync', + [Attr.OPERATION]: 'sync.first_sync_hold_released', + hyp_deadline: new Date(deadline).toISOString(), + hyp_released_early_ms: deadline - Date.now(), + destinations: destinations.length, + off_machine_destinations: destinations.filter((d) => d.offMachine === true).length, + }) + } const { createSinkDriver } = await import('../sinks/driver.js') const driver = createSinkDriver({ @@ -127,6 +201,13 @@ export async function runSync(argv, ctx) { if (instance) tickOpts.sinkInstance = instance const report = await driver.tick(tickOpts) + // A hold that appeared between the check above and the tick (a concurrent + // enrolling login) would otherwise print an empty report and exit 0. + if (report.held) { + ctx.stderr.write(`hyp sync: nothing was sent - the sink driver is holding every tick (${report.held})\n`) + return 1 + } + for (const r of report.sinks) { ctx.stdout.write( `${r.instance}: ${r.status} (partitions=${r.partitionsExported}, bytes=${r.bytesWritten}${ @@ -149,14 +230,24 @@ export async function runSync(argv, ctx) { * reports `null` and the summary stays silent about it - a confirmation * prompt that guesses is worse than one that admits the gap. * + * A server is named, never spelled as a URL. R1a binds the enrolling login's + * surfaces by its own text, but its reason is about terminals, not about + * which command printed the line: any `https://` run autolinks with no way + * to opt out, and the server root answers `{"error":"unknown_path"}` in a + * browser. This prompt appears at the same moment in onboarding and would + * collect the same dead-link click. An origin with no configured name falls + * back to its host, which is still not a URL a terminal will linkify. + * + * @ref LLP 0100#requirements [constrained-by]: R1a's reason - name the server, never its URL - applied to the consent prompt R1a's text does not reach * @param {ExtendedSinkHandle} handle + * @param {Record} remotes configured targets, name to URL * @returns {{ instance: string, text: string, offMachine: boolean | null }} */ -function describeDestination(handle) { +function describeDestination(handle, remotes) { const config = /** @type {Record} */ (handle.config ?? {}) const url = config.url if (typeof url === 'string' && /^https?:\/\//i.test(url)) { - return { instance: handle.instanceName, text: url, offMachine: true } + return { instance: handle.instanceName, text: nameServer(url, remotes), offMachine: true } } const dir = config.dir if (typeof dir === 'string' && dir.length > 0) { @@ -165,6 +256,35 @@ function describeDestination(handle) { return { instance: handle.instanceName, text: handle.plugin ?? 'unknown destination', offMachine: null } } +/** + * Render a server URL as the name the user configured for it, matching on + * origin so a target registered with a trailing path or slash still resolves. + * + * @param {string} url + * @param {Record} remotes + * @returns {string} + */ +function nameServer(url, remotes) { + /** @param {string} value */ + const originOf = (value) => { + try { + return new URL(value).origin + } catch { + return null + } + } + const origin = originOf(url) + if (origin) { + for (const [name, target] of Object.entries(remotes ?? {})) { + if (typeof target?.url === 'string' && originOf(target.url) === origin) { + return `the '${name}' server` + } + } + return new URL(url).host + } + return url +} + /** * Count the directories the machine-local policy lists withhold, so the * prompt can state what is *not* going as well as what is. @@ -210,6 +330,11 @@ function renderPlan({ destinations, exclusions }) { : '' lines.push(` ${dest.instance.padEnd(width)} ${dest.text}${note}\n`) } + // Naming a server instead of its URL is only safe if the name stays + // auditable: R1a's second half, applied here for the same reason. + if (destinations.some((d) => d.offMachine === true)) { + lines.push(" (run 'hyp remote list' to see server URLs)\n") + } lines.push('\n') if ('error' in exclusions) { lines.push(` warning: could not read the local-only list (${exclusions.error});\n`) diff --git a/test/core/sync-command.test.js b/test/core/sync-command.test.js index 618d2454..cdc5b123 100644 --- a/test/core/sync-command.test.js +++ b/test/core/sync-command.test.js @@ -72,9 +72,9 @@ function fakeSink(instanceName, config, result = {}) { } /** - * @param {{ hypHome: string, sinks: any[], tty?: boolean, answer?: string }} args + * @param {{ hypHome: string, sinks: any[], tty?: boolean, answer?: string, remotes?: Record }} args */ -function makeCtx({ hypHome, sinks, tty = false, answer }) { +function makeCtx({ hypHome, sinks, tty = false, answer, remotes }) { const stdout = captureStream() const stderr = captureStream() const stdin = Object.assign(new PassThrough(), { isTTY: tty }) @@ -85,7 +85,7 @@ function makeCtx({ hypHome, sinks, tty = false, answer }) { stdin, env: { HYP_HOME: hypHome, HYP_CONFIG: '' }, cwd: '/home/u', - config: { version: 2 }, + config: remotes ? { version: 2, query: { remotes } } : { version: 2 }, query: { listDatasets: () => [] }, storage: { cacheRoot: path.join(hypHome, 'cache'), @@ -195,17 +195,97 @@ test('the plan names each destination and whether it leaves the machine', async fakeSink('mystery', {}), ], tty: true, + remotes: { prod: { url: 'https://hypaware.example.com/' } }, }) await runSync(['--dry-run'], ctx) const text = stdout.text - assert.match(text, /central\s+https:\/\/hypaware\.example\.com\s+\(leaves this machine\)/) + // A server is named, never spelled as a URL a terminal would autolink + // (LLP 0100 R1a's reason, applied to this surface). + assert.match(text, /central\s+the 'prod' server\s+\(leaves this machine\)/) + assert.doesNotMatch(text, /https:\/\//) + assert.match(text, /\(run 'hyp remote list' to see server URLs\)/) assert.match(text, /parquet\s+\/home\/u\/exports\s+\(stays on this machine\)/) // An undeclarable destination says nothing rather than guessing either way. assert.match(text, /mystery\s+@hypaware\/fake\n/) }) +test('an unnamed server falls back to its host, still not a linkifiable URL', async () => { + const hypHome = await makeHome('unnamed-server') + const { ctx, stdout } = makeCtx({ + hypHome, + sinks: [fakeSink('central', { url: 'https://elsewhere.example.com/ingest' })], + tty: true, + remotes: {}, + }) + + await runSync(['--dry-run'], ctx) + + assert.match(stdout.text, /central\s+elsewhere\.example\.com\s+\(leaves this machine\)/) + assert.doesNotMatch(stdout.text, /https:\/\//) +}) + +test('a named instance cannot release the hold: the plan it showed was not the hold\'s scope', async () => { + // The hold is driver-wide (LLP 0101 #hold). A plan built from one handle + // omits every other destination, so confirming it would forward them + // unseen - the silent first forward the hold exists to prevent. + const hypHome = await makeHome('scoped-held') + await writeFirstSyncHoldMarker({ stateDir: stateDir(hypHome) }) + const central = fakeSink('central', { url: 'https://hypaware.example.com' }) + const parquet = fakeSink('parquet', { dir: '/home/u/exports' }) + const { ctx, stderr } = makeCtx({ + hypHome, + sinks: [central, parquet], + tty: true, + answer: 'y', + }) + + const code = await runSync(['parquet'], ctx) + + assert.equal(code, 2) + assert.match(stderr.text, /review window is open until /) + assert.match(stderr.text, /all-or-nothing/) + assert.match(stderr.text, /would forward the others unseen/) + assert.ok(await holdExists(hypHome), 'the marker must survive a scoped run') + assert.deepEqual(parquet.exported, [], 'nothing exports while the window stands') + assert.deepEqual(central.exported, []) +}) + +test('--yes cannot release the hold: #no-release licenses an attended confirmation only', async () => { + const hypHome = await makeHome('yes-held') + await writeFirstSyncHoldMarker({ stateDir: stateDir(hypHome) }) + const sink = fakeSink('central', { url: 'https://hypaware.example.com' }) + const { ctx, stderr } = makeCtx({ hypHome, sinks: [sink], tty: false }) + + const code = await runSync(['--yes'], ctx) + + assert.equal(code, 2) + assert.match(stderr.text, /--yes cannot do it/) + assert.ok(await holdExists(hypHome), 'a script must not end somebody else\'s review window') + assert.deepEqual(sink.exported, []) +}) + +test('a hold that cannot be cleared fails loudly instead of exiting 0 with nothing sent', async () => { + const hypHome = await makeHome('clear-fails') + await writeFirstSyncHoldMarker({ stateDir: stateDir(hypHome) }) + const policyDir = path.dirname(firstSyncHoldMarkerPath(stateDir(hypHome))) + await fs.chmod(policyDir, 0o500) + const sink = fakeSink('central', { url: 'https://hypaware.example.com' }) + const { ctx, stderr } = makeCtx({ hypHome, sinks: [sink], tty: true, answer: 'y' }) + + try { + const code = await runSync([], ctx) + + assert.equal(code, 1, 'a held tick that exported nothing is not a success') + assert.match(stderr.text, /could not end the review window/) + assert.match(stderr.text, /Nothing was sent/) + assert.deepEqual(sink.exported, []) + } finally { + await fs.chmod(policyDir, 0o700) + } +}) + test('the plan counts the directories being withheld', async () => { const hypHome = await makeHome('exclusions') await writeLocalOnlyEntries({ @@ -253,7 +333,7 @@ test('an unknown instance names the ones that exist', async () => { assert.match(stderr.text, /available: central/) }) -test('an instance argument ticks only that sink', async () => { +test('an instance argument ticks only that sink (no hold in play)', async () => { const hypHome = await makeHome('one-instance') const central = fakeSink('central', { url: 'https://hypaware.example.com' }) const parquet = fakeSink('parquet', { dir: '/home/u/exports' })