diff --git a/CHANGELOG.md b/CHANGELOG.md index 515b7d2..cdf26ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ Between releases: see `git log` for merged work and [`ROADMAP.md`](ROADMAP.md) f ## [Unreleased] +### Changed (hook crash presentation in the Summary — #105) + +Closes [#105](https://github.com/breferrari/shardmind/issues/105). + +- **A crashed hook no longer buries the outcome.** Hooks are non-fatal (Helm semantics — the vault is already written when a hook runs, so a failing hook never rolls back), but the Summary dumped the hook's raw multi-line stderr verbatim, so a *successful* install with a *crashed* hook looked indistinguishable from an engine failure. The crash now renders as `" exited with code N. Non-fatal; your vault is ready."` with the "operation succeeded" detail on its own dim line, and the captured stdout/stderr render **dimmed + indented**, truncated to the first 5 lines (`HOOK_OUTPUT_VISIBLE_LINES`) with a `"… N more lines (full log at .shardmind/logs/.log)"` pointer. + +- **Full output is persisted to a vault-local log.** When a hook crashes (non-zero exit / failure) or its output is long enough to truncate, the orchestrator writes the complete captured stdout/stderr to `.shardmind/logs/.log` (new `HOOK_LOGS_DIR`). The write is **non-fatal** — a failure (read-only vault, ENOSPC) just omits the on-screen pointer; it never breaks an install whose hook is already non-fatal by contract. Short, clean hooks write nothing. The log lives under `.shardmind/`, so it is excluded from Invariant 1 byte-equivalence. + +- **Tests:** `headLines` unit matrix; orchestrator log-persistence cases (crash writes a log, long-clean writes a log, short-clean writes nothing, dry-run writes nothing, unwritable `logs/` is non-fatal); `HookSummarySection` truncation + pointer + crash-header rendering; L2 PTY scenario 27 asserts the truncated on-screen UI + the full marker in the on-disk log. Invariant 1 + the contract four-branch hook tests stay green. **Docs:** `docs/SHARD-LAYOUT.md`, `docs/ARCHITECTURE.md §9.3`, `docs/AUTHORING.md §6`. + ### Fixed (update merge rendered copy-origin files — #132) Closes [#132](https://github.com/breferrari/shardmind/issues/132); root cause **reported via [#129](https://github.com/breferrari/shardmind/issues/129)** (thanks!). diff --git a/ROADMAP.md b/ROADMAP.md index 3496f21..e02ed44 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -128,15 +128,15 @@ UX gaps surfaced during real obsidian-mind v6 install + adopt runs. None block t - [x] `multiselect` value type for module-set questions ([#101](https://github.com/breferrari/shardmind/issues/101)) — pairs with #100 (shortens module list). First-class value type + scrollable widget + per-option `default` + min/max; value→module gating stays #80. - [x] Adopt: replace step-by-step install wizard with confirm-or-override values flow ([#104](https://github.com/breferrari/shardmind/issues/104)) - [x] Adopt batch operations (keep all mine / use all theirs / auto-merge non-conflicting) ([#120](https://github.com/breferrari/shardmind/issues/120)) — pairs with #104. -- [ ] Hook stderr presentation in Summary (truncate, dim, label as non-fatal) ([#105](https://github.com/breferrari/shardmind/issues/105)) +- [x] Hook stderr presentation in Summary (truncate, dim, label as non-fatal) ([#105](https://github.com/breferrari/shardmind/issues/105)) ### 0.1.x — Done gate v0.1.x ships when: -- [ ] Foundation closed: [#111](https://github.com/breferrari/shardmind/issues/111) ✅, [#112](https://github.com/breferrari/shardmind/issues/112) ✅. -- [ ] Parallel closed: [#113](https://github.com/breferrari/shardmind/issues/113) (self-update notifier), [#119](https://github.com/breferrari/shardmind/issues/119) (release cadence policy). +- [x] Foundation closed: [#111](https://github.com/breferrari/shardmind/issues/111) ✅, [#112](https://github.com/breferrari/shardmind/issues/112) ✅. +- [x] Parallel closed: [#113](https://github.com/breferrari/shardmind/issues/113) (self-update notifier), [#119](https://github.com/breferrari/shardmind/issues/119) (release cadence policy). - [ ] Hook lifecycle (#102) shipped with [#121](https://github.com/breferrari/shardmind/issues/121) (version-compatibility check) and obsidian-mind hook migration in the same release window. -- [ ] Flagship-UX closed: [#100](https://github.com/breferrari/shardmind/issues/100), [#101](https://github.com/breferrari/shardmind/issues/101), [#104](https://github.com/breferrari/shardmind/issues/104), [#105](https://github.com/breferrari/shardmind/issues/105), [#120](https://github.com/breferrari/shardmind/issues/120). +- [x] Flagship-UX closed: [#100](https://github.com/breferrari/shardmind/issues/100), [#101](https://github.com/breferrari/shardmind/issues/101), [#104](https://github.com/breferrari/shardmind/issues/104), [#105](https://github.com/breferrari/shardmind/issues/105), [#120](https://github.com/breferrari/shardmind/issues/120). - [ ] Research-wiki shard ([#15](https://github.com/breferrari/shardmind/issues/15)) shipped with E2E tests + registry-mode end-to-end proof. - [ ] v6 docs polish ([#85](https://github.com/breferrari/shardmind/issues/85)) closed. - [ ] Smoke gate green against both shards (obsidian-mind + research-wiki). diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index dbbf472..43b97f8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -631,7 +631,7 @@ export default async function(ctx: BootstrapContext): Promise; **Post-hook re-hash**: after the hook phase exits — success OR failure — the engine re-reads each managed file in `state.files` and recomputes `rendered_hash`, then writes the updated `state.json`. This ensures the engine's view of disk reflects actual content even when a hook only partially completed (the hook contract is non-fatal, so a hook that broke can't corrupt the engine). Per-file ENOENT and other I/O errors are tolerated; drift detection picks up any discrepancy on the next status run. Implementation: `source/core/state.ts::rehashManagedFiles`, called by the orchestrator after the hook subprocess returns. -**If a hook throws**: ShardMind logs the error, shows a warning (" hook exited with code N. Install succeeded; the hook's work may be incomplete."), does NOT rollback. Non-fatal. Same pattern as Helm hooks. The post-hook re-hash still runs, and subsequent independent slots still attempt. +**If a hook throws**: ShardMind shows a warning (" exited with code N. Non-fatal; your vault is ready." with "The operation succeeded; the hook's work may be incomplete." on its own dim line), does NOT rollback. Non-fatal. Same pattern as Helm hooks. The post-hook re-hash still runs, and subsequent independent slots still attempt. So a crashed hook's stack trace can't visually dominate the Summary, the captured stdout/stderr render dimmed + indented and **truncated to the first `HOOK_OUTPUT_VISIBLE_LINES` lines**; when a hook crashes or its output is long enough to truncate, the orchestrator persists the complete output to `.shardmind/logs/.log` and the on-screen block points there. The log write is itself non-fatal (a write failure just omits the pointer). See `source/components/HookSummarySection.tsx` and `attachHookLog` in `hook-orchestrator.ts` (#105). **Legacy `post-install`**: a shard declaring the deprecated `hooks.post-install` (and neither new slot) runs it once on install/adopt with the legacy flat `HookContext` (including `valuesAreDefaults`, `newFiles: []`, `removedFiles: []` for source compatibility) and no boundary enforcement, plus a `HOOK_POST_INSTALL_DEPRECATED` warning. Declaring it alongside `bootstrap`/`personalize` is a parse-time `HOOK_SLOT_CONFLICT`. Honored ≥1 minor (deprecate 0.2.0, remove ≥0.3.0). diff --git a/docs/AUTHORING.md b/docs/AUTHORING.md index 8994b87..1bc8efb 100644 --- a/docs/AUTHORING.md +++ b/docs/AUTHORING.md @@ -362,6 +362,8 @@ The old single `post-install` hook bundled three jobs with different lifecycles - **You no longer write `if (!ctx.valuesAreDefaults) …`.** The engine simply doesn't invoke `personalize` on a defaults install. `PersonalizeContext` has no `valuesAreDefaults` field — if your `personalize` hook runs, values are non-default by construction. - **The write boundary is checked.** If `bootstrap` edits a managed file or `personalize` creates an unmanaged one, the engine surfaces a non-fatal warning naming the paths (it does not undo the write). This catches a mis-placed responsibility during your dev loop instead of silently breaking Invariant 1 in the field. +**If your hook crashes**, the install/update/adopt still succeeds (hooks are non-fatal). The Summary shows a dimmed, truncated head of the hook's output under a "Non-fatal; your vault is ready." warning, and writes the **full** captured stdout/stderr to `.shardmind/logs/.log` in the installed vault — point users there (or `cat` it yourself) to debug a failing hook. + ### Per-slot context - **`BootstrapContext`** → `{ slot: 'bootstrap', vaultRoot, values, modules, shard, previousVersion? }`. No `valuesAreDefaults`, no file lists. diff --git a/docs/SHARD-LAYOUT.md b/docs/SHARD-LAYOUT.md index 4c05611..5f42cd0 100644 --- a/docs/SHARD-LAYOUT.md +++ b/docs/SHARD-LAYOUT.md @@ -65,7 +65,9 @@ my-vault/ │ ├── state.json ← ownership hashes + module/agent selections + version + resolved ref │ ├── shard.yaml ← cached manifest (runtime reads without re-extracting the tarball) │ ├── shard-schema.yaml ← cached values schema -│ └── templates/ ← cached source files; merge base for three-way merge on update +│ ├── templates/ ← cached source files; merge base for three-way merge on update +│ └── logs/ ← full output of a crashed or verbose hook (.log); written +│ on demand so the Summary can truncate on screen and point here │ ├── shard-values.yaml ← user's answers from the wizard; vault-root (not under .shardmind/); │ named separately from .shardmind/ per VISION's @@ -83,7 +85,7 @@ my-vault/ └── (no .github/, no CONTRIBUTING.md, no translations, no demo media) ``` -The installed-side path constants are authoritative in [`source/runtime/vault-paths.ts`](../source/runtime/vault-paths.ts): `STATE_FILE`, `CACHED_MANIFEST`, `CACHED_SCHEMA`, `CACHED_TEMPLATES` all live under `.shardmind/`; `VALUES_FILE` lives at vault root. +The installed-side path constants are authoritative in [`source/runtime/vault-paths.ts`](../source/runtime/vault-paths.ts): `STATE_FILE`, `CACHED_MANIFEST`, `CACHED_SCHEMA`, `CACHED_TEMPLATES`, `HOOK_LOGS_DIR` all live under `.shardmind/`; `VALUES_FILE` lives at vault root. Everything under `.shardmind/` (including `logs/`) is engine metadata and is excluded from the Invariant 1 byte-equivalence comparison. ## Personalization model diff --git a/source/components/HookSummarySection.tsx b/source/components/HookSummarySection.tsx index 8fecea1..09c1b89 100644 --- a/source/components/HookSummarySection.tsx +++ b/source/components/HookSummarySection.tsx @@ -1,7 +1,7 @@ import type { ReactElement } from 'react'; import { Box, Text } from 'ink'; import { StatusMessage } from './ui.js'; -import type { HookStage, HookSummary } from '../core/hook.js'; +import { headLines, type HookStage, type HookSummary } from '../core/hook.js'; import type { HookOutcome } from '../core/hook-orchestrator.js'; import { assertNever } from '../runtime/types.js'; @@ -80,9 +80,12 @@ function renderOutcome( {succeeded ? ( {name} completed. ) : ( - - {name} exited with code {exitCode}. The operation succeeded; the hook's work may be incomplete. - + + + {name} exited with code {exitCode}. Non-fatal; your vault is ready. + + The operation succeeded; the hook's work may be incomplete. + )} {summary.deprecated && ( @@ -92,18 +95,40 @@ function renderOutcome( {summary.violation && ( {violationMessage(stage, summary.violation)} )} - {stdout && ( - - Hook stdout: - {stdout} - - )} - {stderr && ( - - Hook stderr: - {stderr} - - )} + {stdout && outputBlock('Hook stdout:', stdout, summary.logPath, 'stdout')} + {stderr && outputBlock('Hook stderr:', stderr, summary.logPath, 'stderr')} + + ); +} + +/** + * One captured-output block: a bold label, then the first + * `HOOK_OUTPUT_VISIBLE_LINES` lines dimmed + indented so the hook's output + * never reads as the primary outcome. When the output is longer, a dim + * "… N more lines" pointer follows — naming the full log on disk when one was + * written (`summary.logPath`). A long Node stack trace from a crashed hook + * thus collapses to a few dim lines plus a path, instead of dominating the + * Summary. See #105. + */ +function outputBlock( + label: string, + text: string, + logPath: string | undefined, + key: string, +): ReactElement { + const { head, hidden } = headLines(text); + return ( + + {label} + + {head} + {hidden > 0 && ( + + … {hidden} more line{hidden === 1 ? '' : 's'} + {logPath ? ` (full log at ${logPath})` : ''} + + )} + ); } diff --git a/source/core/hook-orchestrator.ts b/source/core/hook-orchestrator.ts index 64d8f31..eeded68 100644 --- a/source/core/hook-orchestrator.ts +++ b/source/core/hook-orchestrator.ts @@ -19,6 +19,8 @@ * Spec: docs/SHARD-LAYOUT.md §Hook lifecycle; docs/IMPLEMENTATION.md §4.16a. */ +import fsp from 'node:fs/promises'; +import path from 'node:path'; import type { AnyHookContext, ModuleSelections, @@ -26,10 +28,12 @@ import type { ShardSchema, ShardState, } from '../runtime/types.js'; +import { HOOK_LOGS_DIR, hookLogRelPath } from '../runtime/vault-paths.js'; import { DEFAULT_HOOK_TIMEOUT_MS } from './manifest.js'; import { runHook, summarizeHook, + headLines, type HookResult, type HookStage, type HookSummary, @@ -208,7 +212,11 @@ export async function runHooks(plan: HookRunPlan, ui: HookRunUi): Promise.log` when it + * is worth pointing at — the hook crashed (non-zero exit / failure) or either + * stream is long enough that the Summary will truncate it — and return the + * summary with `logPath` set. A short, clean hook writes nothing (no clutter, + * no perturbation of clean-path E2E) and is returned unchanged. + * + * Non-fatal: a log-write failure (read-only vault, ENOSPC) just omits the + * pointer; it must never break an install whose hook is already non-fatal by + * contract (ARCHITECTURE.md §9.3). The log lives under `.shardmind/`, so it is + * excluded from Invariant 1 byte-equivalence. + */ +export async function attachHookLog( + vaultRoot: string, + slot: HookStage, + summary: HookSummary, +): Promise { + const stdout = summary.stdout ?? ''; + const stderr = summary.stderr ?? ''; + if (stdout === '' && stderr === '') return summary; + + const crashed = (summary.exitCode ?? 0) !== 0; + const willTruncate = + headLines(stdout.trim()).hidden > 0 || headLines(stderr.trim()).hidden > 0; + if (!crashed && !willTruncate) return summary; + + try { + await fsp.mkdir(path.join(vaultRoot, HOOK_LOGS_DIR), { recursive: true }); + const relPath = hookLogRelPath(slot); + await fsp.writeFile(path.join(vaultRoot, relPath), formatHookLog(slot, summary), 'utf-8'); + return { ...summary, logPath: relPath }; + } catch { + return summary; + } +} + +/** Readable full-output dump written to `.shardmind/logs/.log`. */ +function formatHookLog(slot: HookStage, summary: HookSummary): string { + return [ + `# ShardMind ${slot} hook — full captured output`, + `# exit code: ${summary.exitCode ?? 0}`, + '', + '=== stdout ===', + summary.stdout ?? '', + '', + '=== stderr ===', + summary.stderr ?? '', + '', + ].join('\n'); +} + function valuesAreDefaultsSafe(values: Record, schema: ShardSchema): boolean { try { return valuesAreDefaults(values, schema); diff --git a/source/core/hook.ts b/source/core/hook.ts index 1035063..56e232d 100644 --- a/source/core/hook.ts +++ b/source/core/hook.ts @@ -189,6 +189,45 @@ export interface HookSummary { stdout?: string; stderr?: string; exitCode?: number; + /** + * Vault-relative path of the full captured output on disk (e.g. + * `.shardmind/logs/bootstrap.log`), written by the orchestrator when a hook + * crashes or its output is long enough to truncate on screen. The Summary + * shows the first `HOOK_OUTPUT_VISIBLE_LINES` lines and points here for the + * rest. Absent when nothing was written (short clean hook, or the log write + * failed — non-fatal). See `hook-orchestrator.ts`. + */ + logPath?: string; +} + +/** + * How many lines of a hook's stdout / stderr the Summary renders inline before + * truncating with a "… N more lines — full log at " pointer. Shared by + * the orchestrator (which decides whether a log file is worth writing) and + * `HookSummarySection.tsx` (which does the on-screen truncation) so the two + * never disagree on the threshold. + */ +export const HOOK_OUTPUT_VISIBLE_LINES = 5; + +/** + * Split `text` into the first `max` lines plus a count of how many were + * dropped. A single trailing newline is not counted as an extra empty line + * (so `"a\nb\n"` is two lines, not three). Pure — used both to decide whether + * a hook log is worth persisting and to render the truncated on-screen block. + */ +export function headLines( + text: string, + max: number = HOOK_OUTPUT_VISIBLE_LINES, +): { head: string; hidden: number } { + // Split on CRLF or LF — a Windows hook (Git for Windows, some Node scripts) + // emits CRLF, and `split('\n')` alone would leave a trailing `\r` on each + // line that renders as a visible control character in the Summary. The head + // is re-joined with LF (the canonical display ending), matching the rest of + // the engine. + const lines = text.split(/\r?\n/); + if (lines.length > 0 && lines[lines.length - 1] === '') lines.pop(); + if (lines.length <= max) return { head: lines.join('\n'), hidden: 0 }; + return { head: lines.slice(0, max).join('\n'), hidden: lines.length - max }; } /** diff --git a/source/runtime/vault-paths.ts b/source/runtime/vault-paths.ts index 2678403..ec71253 100644 --- a/source/runtime/vault-paths.ts +++ b/source/runtime/vault-paths.ts @@ -14,6 +14,24 @@ export const CACHED_MANIFEST = path.join(SHARDMIND_DIR, 'shard.yaml'); export const CACHED_SCHEMA = path.join(SHARDMIND_DIR, 'shard-schema.yaml'); export const CACHED_TEMPLATES = path.join(SHARDMIND_DIR, 'templates'); +/** + * Where a crashed (or verbose) hook's full captured output is persisted so + * the Summary can truncate the on-screen block and point the user at the + * complete log. Under `.shardmind/`, so it is excluded from Invariant 1 + * byte-equivalence ("modulo `.shardmind/` metadata"). See `hook-orchestrator.ts`. + */ +export const HOOK_LOGS_DIR = path.join(SHARDMIND_DIR, 'logs'); + +/** + * Vault-relative path of one slot's full hook log, e.g. + * `.shardmind/logs/bootstrap.log`. Returned with forward slashes — it doubles + * as a user-facing pointer in the Summary, and `path.join(vaultRoot, …)` + * resolves a posix-style relative path correctly on every platform. + */ +export function hookLogRelPath(slot: string): string { + return `${SHARDMIND_DIR}/logs/${slot}.log`; +} + /** User-authored values file. Engine creates it on install, never overwrites. */ export const VALUES_FILE = 'shard-values.yaml'; diff --git a/tests/component/HookSummarySection.test.tsx b/tests/component/HookSummarySection.test.tsx index e055040..af4a407 100644 --- a/tests/component/HookSummarySection.test.tsx +++ b/tests/component/HookSummarySection.test.tsx @@ -63,6 +63,80 @@ describe('HookSummarySection', () => { expect(frame).toContain('hook boom'); }); + it('truncates a long crashed-hook stderr and points at the full log (#105)', () => { + const stderr = Array.from({ length: 9 }, (_, i) => `trace line ${i}`).join('\n'); + const frame = out([ + { + slot: 'bootstrap', + summary: { stdout: '', stderr, exitCode: 1, logPath: '.shardmind/logs/bootstrap.log' }, + }, + ]).lastFrame() ?? ''; + // Crash header: clearly non-fatal + vault safe, still names the exit code. + expect(frame).toContain('Bootstrap hook exited with code 1'); + expect(frame).toContain('Non-fatal; your vault is ready'); + // First HOOK_OUTPUT_VISIBLE_LINES lines shown; the tail is hidden. + expect(frame).toContain('trace line 0'); + expect(frame).toContain('trace line 4'); + expect(frame).not.toContain('trace line 8'); + // Pointer to the full on-disk log. + expect(frame).toContain('… 4 more lines'); + expect(frame).toContain('full log at .shardmind/logs/bootstrap.log'); + }); + + it('truncates long output on a clean exit too, with the log pointer (#105)', () => { + const stdout = Array.from({ length: 7 }, (_, i) => `out ${i}`).join('\n'); + const frame = out([ + { + slot: 'post-update', + summary: { stdout, stderr: '', exitCode: 0, logPath: '.shardmind/logs/post-update.log' }, + }, + ]).lastFrame() ?? ''; + expect(frame).toContain('Post-update hook completed.'); + expect(frame).toContain('out 0'); + expect(frame).not.toContain('out 6'); + expect(frame).toContain('… 2 more lines'); + expect(frame).toContain('full log at .shardmind/logs/post-update.log'); + }); + + it('omits the "more lines" pointer for output within the visible cap (#105)', () => { + const frame = out([ + { slot: 'bootstrap', summary: { stdout: 'a\nb\nc', stderr: '', exitCode: 0 } }, + ]).lastFrame() ?? ''; + expect(frame).toContain('a'); + expect(frame).not.toContain('more line'); + }); + + it('shows the truncation count without a path when no log was written (#105)', () => { + const stderr = Array.from({ length: 8 }, (_, i) => `e${i}`).join('\n'); + const frame = out([{ slot: 'bootstrap', summary: { stdout: '', stderr, exitCode: 1 } }]).lastFrame() ?? ''; + expect(frame).toContain('… 3 more lines'); + expect(frame).not.toContain('full log at'); + }); + + it('shows the log pointer only when output actually truncates, not just because logPath is set (#105)', () => { + // A crashed hook with SHORT output: a log may exist, but the on-screen + // block fits — so no "more lines"/pointer noise. + const frame = out([ + { + slot: 'bootstrap', + summary: { stdout: 'ok', stderr: 'boom', exitCode: 1, logPath: '.shardmind/logs/bootstrap.log' }, + }, + ]).lastFrame() ?? ''; + expect(frame).toContain('boom'); + expect(frame).not.toContain('more line'); + expect(frame).not.toContain('full log at'); + }); + + it('renders only the crash header when a crashed hook produced no output (#105)', () => { + const frame = out([ + { slot: 'bootstrap', summary: { stdout: '', stderr: '', exitCode: 1, logPath: '.shardmind/logs/bootstrap.log' } }, + ]).lastFrame() ?? ''; + expect(frame).toContain('Bootstrap hook exited with code 1'); + expect(frame).not.toContain('Hook stdout:'); + expect(frame).not.toContain('Hook stderr:'); + expect(frame).not.toContain('full log at'); + }); + it('renders a bootstrap managed-write boundary violation', () => { const frame = out([ { slot: 'bootstrap', summary: { exitCode: 0, violation: { kind: 'managed-write', paths: ['Home.md', 'brain/North Star.md'] } } }, diff --git a/tests/e2e/tui/hook-lifecycle.test.ts b/tests/e2e/tui/hook-lifecycle.test.ts index fd85c83..8915a7d 100644 --- a/tests/e2e/tui/hook-lifecycle.test.ts +++ b/tests/e2e/tui/hook-lifecycle.test.ts @@ -244,33 +244,44 @@ describe.skipIf(skipOnWindows)( }, ); try { - // The Summary must say: + // The Summary must say (#105 presentation): // - "Installed l2hooks/throw@0.1.0" — install itself // succeeded (Helm semantics: hook is non-fatal). - // - "Bootstrap hook exited with code " — the - // warning headline from HookSummarySection. - // - the thrown message (or its stderr trail) somewhere - // in the captured stderr block. + // - "Bootstrap hook exited with code . Non-fatal; your + // vault is ready." — the reworded crash headline, so a + // crashed hook no longer reads as an engine failure. + // This hook's thrown stack is only ~3 lines (under the 5-line + // cap), so it is NOT truncated and shows no "(full log at …)" + // pointer — that path is covered at the component layer. What + // the real PTY proves here is the reworded headline + that the + // full output is persisted to the vault-local crash log. await handle.waitForScreen( (s) => /Installed l2hooks\/throw@0\.1\.0/.test(s) && - /Bootstrap hook exited with code/.test(s), + /Bootstrap hook exited with code/.test(s) && + /Non-fatal; your vault is ready/.test(s), { timeoutMs: 30_000, - description: 'install summary + hook warning', + description: 'install summary + non-fatal crash headline', }, ); const screen = handle.screen.serialize(); - // The thrown error string ("L2_HOOK_THROW_BANG") lands in - // stderr via Node's default uncaught-error printer; under - // PTY's 50-row viewport it surfaces in the Hook stderr - // section. + // Short stack stays on screen (dimmed); the marker is present. expect(screen).toMatch(/L2_HOOK_THROW_BANG/); const exit = await handle.waitForExit(); // Install itself succeeded — exit 0 even though hook // failed (the contract). expect(exit.exitCode).toBe(0); + + // A crashed hook always persists its full output verbatim to the + // vault-local log, even when short enough to show on screen. + const log = await fs.readFile( + path.join(vault, '.shardmind', 'logs', 'bootstrap.log'), + 'utf-8', + ); + expect(log).toMatch(/L2_HOOK_THROW_BANG/); + expect(log).toContain('=== stderr ==='); } finally { await handle.dispose(); } diff --git a/tests/unit/hook-orchestrator.test.ts b/tests/unit/hook-orchestrator.test.ts index e74c00a..b8886e7 100644 --- a/tests/unit/hook-orchestrator.test.ts +++ b/tests/unit/hook-orchestrator.test.ts @@ -491,3 +491,84 @@ describe('runHooks — legacy + resilience', () => { await expect(fsp.access(path.join(vault, 'p.txt'))).rejects.toBeTruthy(); }, 30_000); }); + +describe('runHooks — full-output log persistence (#105)', () => { + const LOG = path.join('.shardmind', 'logs', 'bootstrap.log'); + + it('writes a log + sets logPath when a hook crashes (non-zero exit)', async () => { + await hook( + 'bootstrap.ts', + `export default async function () { process.stderr.write('boom-marker\\n'); process.exit(2); }`, + ); + const result = await runHooks( + installPlan({ manifest: manifest({ bootstrap: { script: '.shardmind/hooks/bootstrap.ts' } }) }), + NOOP_UI, + ); + const boot = result.outcomes.find((o) => o.slot === 'bootstrap'); + expect(boot?.summary?.exitCode).toBe(2); + expect(boot?.summary?.logPath).toBe('.shardmind/logs/bootstrap.log'); + const contents = await fsp.readFile(path.join(vault, LOG), 'utf-8'); + expect(contents).toContain('boom-marker'); + expect(contents).toContain('# exit code: 2'); + // Pin the log format so on-disk logs stay readable. + expect(contents).toContain('# ShardMind bootstrap hook'); + expect(contents).toContain('=== stdout ==='); + expect(contents).toContain('=== stderr ==='); + }, 30_000); + + it('writes a log for a clean hook whose output is long enough to truncate', async () => { + await hook( + 'bootstrap.ts', + `export default async function () { for (let i = 0; i < 8; i++) console.log('line ' + i); }`, + ); + const result = await runHooks( + installPlan({ manifest: manifest({ bootstrap: { script: '.shardmind/hooks/bootstrap.ts' } }) }), + NOOP_UI, + ); + const boot = result.outcomes.find((o) => o.slot === 'bootstrap'); + expect(boot?.summary?.exitCode).toBe(0); + expect(boot?.summary?.logPath).toBe('.shardmind/logs/bootstrap.log'); + expect((await fsp.readFile(path.join(vault, LOG), 'utf-8'))).toContain('line 7'); + }, 30_000); + + it('writes NO log (and no logPath) for a short, clean hook', async () => { + await hook('bootstrap.ts', `export default async function () { console.log('ok'); }`); + const result = await runHooks( + installPlan({ manifest: manifest({ bootstrap: { script: '.shardmind/hooks/bootstrap.ts' } }) }), + NOOP_UI, + ); + const boot = result.outcomes.find((o) => o.slot === 'bootstrap'); + expect(boot?.summary?.logPath).toBeUndefined(); + await expect(fsp.access(path.join(vault, '.shardmind', 'logs'))).rejects.toBeTruthy(); + }, 30_000); + + it('does not write a log on a dry run, even when the hook crashes', async () => { + await hook('bootstrap.ts', `export default async function () { process.exit(2); }`); + const result = await runHooks( + installPlan({ + manifest: manifest({ bootstrap: { script: '.shardmind/hooks/bootstrap.ts' } }), + dryRun: true, + }), + NOOP_UI, + ); + const boot = result.outcomes.find((o) => o.slot === 'bootstrap'); + expect(boot?.summary?.logPath).toBeUndefined(); + await expect(fsp.access(path.join(vault, '.shardmind', 'logs'))).rejects.toBeTruthy(); + }, 30_000); + + it('is non-fatal when the log cannot be written (logs/ path is a file)', async () => { + // Block the log dir: put a FILE where `.shardmind/logs/` should be, so the + // recursive mkdir fails. The hook outcome must still be reported; only the + // pointer is omitted. + await fsp.mkdir(path.join(vault, '.shardmind'), { recursive: true }); + await fsp.writeFile(path.join(vault, '.shardmind', 'logs'), 'not a dir', 'utf-8'); + await hook('bootstrap.ts', `export default async function () { process.exit(2); }`); + const result = await runHooks( + installPlan({ manifest: manifest({ bootstrap: { script: '.shardmind/hooks/bootstrap.ts' } }) }), + NOOP_UI, + ); + const boot = result.outcomes.find((o) => o.slot === 'bootstrap'); + expect(boot?.summary?.exitCode).toBe(2); // outcome intact + expect(boot?.summary?.logPath).toBeUndefined(); // pointer omitted, no throw + }, 30_000); +}); diff --git a/tests/unit/hook.test.ts b/tests/unit/hook.test.ts index 5a6a203..eaa1a8c 100644 --- a/tests/unit/hook.test.ts +++ b/tests/unit/hook.test.ts @@ -24,6 +24,8 @@ import { runHook, executeHook, tailAtUtf8Boundary, + headLines, + HOOK_OUTPUT_VISIBLE_LINES, } from '../../source/core/hook.js'; import type { BootstrapContext, @@ -42,6 +44,50 @@ function makeManifest(hooks: ShardManifest['hooks']): ShardManifest { }; } +describe('headLines (#105)', () => { + it('returns the whole text with hidden:0 when at or below the cap', () => { + expect(headLines('a\nb\nc', 5)).toEqual({ head: 'a\nb\nc', hidden: 0 }); + const exact = 'a\nb\nc\nd\ne'; + expect(headLines(exact, 5)).toEqual({ head: exact, hidden: 0 }); + }); + + it('does not count a single trailing newline as an extra line', () => { + // "a\nb\n" is two lines, not three — the trailing "" is dropped. + expect(headLines('a\nb\n', 5)).toEqual({ head: 'a\nb', hidden: 0 }); + const fiveWithEol = 'a\nb\nc\nd\ne\n'; + expect(headLines(fiveWithEol, 5)).toEqual({ head: 'a\nb\nc\nd\ne', hidden: 0 }); + }); + + it('keeps the first `max` lines and counts the remainder', () => { + expect(headLines('1\n2\n3\n4\n5\n6\n7', 5)).toEqual({ head: '1\n2\n3\n4\n5', hidden: 2 }); + }); + + it('treats a single long line with no newline as one line', () => { + const long = 'x'.repeat(10_000); + expect(headLines(long, 5)).toEqual({ head: long, hidden: 0 }); + }); + + it('handles the empty string', () => { + expect(headLines('', 5)).toEqual({ head: '', hidden: 0 }); + }); + + it('normalizes CRLF input — no trailing \\r artifacts (Windows hooks)', () => { + // A Windows hook emits CRLF; `split('\n')` alone would leave a trailing + // \r on each line that renders as a visible control character. + expect(headLines('a\r\nb\r\nc\r\n', 5)).toEqual({ head: 'a\nb\nc', hidden: 0 }); + const long = '1\r\n2\r\n3\r\n4\r\n5\r\n6\r\n7'; + const { head, hidden } = headLines(long, 5); + expect(head).toBe('1\n2\n3\n4\n5'); + expect(head).not.toContain('\r'); + expect(hidden).toBe(2); + }); + + it('defaults to HOOK_OUTPUT_VISIBLE_LINES', () => { + const lines = Array.from({ length: HOOK_OUTPUT_VISIBLE_LINES + 3 }, (_, i) => `L${i}`).join('\n'); + expect(headLines(lines).hidden).toBe(3); + }); +}); + describe('tailAtUtf8Boundary', () => { it('returns the whole string when byte length is under the cap', () => { expect(tailAtUtf8Boundary('hello', 100)).toBe('hello'); diff --git a/tests/unit/vault-paths.test.ts b/tests/unit/vault-paths.test.ts new file mode 100644 index 0000000..52a8ac5 --- /dev/null +++ b/tests/unit/vault-paths.test.ts @@ -0,0 +1,37 @@ +/** + * Platform-invariance guards for the vault-relative path helpers. + * + * These constants double as user-facing strings (the Summary prints the hook + * log pointer verbatim), so they MUST be byte-identical on every OS — a Windows + * `path.join` that leaked backslashes would show `.shardmind\logs\bootstrap.log` + * to a Windows user while Linux/macOS users saw forward slashes. This suite runs + * on the full ubuntu/windows/macos CI matrix and pins the forward-slash contract. + */ + +import { describe, it, expect } from 'vitest'; +import path from 'node:path'; +import { hookLogRelPath, HOOK_LOGS_DIR, SHARDMIND_DIR } from '../../source/runtime/vault-paths.js'; + +describe('hookLogRelPath — platform-invariant display path', () => { + it('is always forward-slash, on every OS', () => { + for (const slot of ['bootstrap', 'personalize', 'post-update', 'post-install']) { + const rel = hookLogRelPath(slot); + expect(rel).toBe(`.shardmind/logs/${slot}.log`); + expect(rel).not.toContain('\\'); // never a backslash, even on win32 + } + }); + + it('resolves to a valid OS-native absolute path under the vault when joined', () => { + // The display string is forward-slash; the on-disk location is whatever + // path.join produces for the host OS — both must point at the same file. + const vault = path.resolve('some-vault'); + const abs = path.join(vault, hookLogRelPath('bootstrap')); + expect(abs.startsWith(vault)).toBe(true); + expect(path.basename(abs)).toBe('bootstrap.log'); + expect(path.dirname(abs)).toBe(path.join(vault, HOOK_LOGS_DIR)); + }); + + it('HOOK_LOGS_DIR lives under .shardmind/', () => { + expect(HOOK_LOGS_DIR).toBe(path.join(SHARDMIND_DIR, 'logs')); + }); +});