Skip to content

feat: hook crash presentation in the Summary (#105)#134

Merged
breferrari merged 7 commits into
mainfrom
feat/105-hook-stderr-presentation
Jun 1, 2026
Merged

feat: hook crash presentation in the Summary (#105)#134
breferrari merged 7 commits into
mainfrom
feat/105-hook-stderr-presentation

Conversation

@breferrari
Copy link
Copy Markdown
Owner

@breferrari breferrari commented Jun 1, 2026

Closes #105. Last open Flagship-UX item.

Problem

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 — a Node stack trace dwarfed the "What happened" / "Next" sections.

Change

  • Crash header: "<hook> exited with code N. Non-fatal; your vault is ready." with the "operation succeeded" detail on its own dim line — the command outcome stays the headline.
  • Truncated, dimmed output: stdout/stderr render dimmed + indented, capped at the first HOOK_OUTPUT_VISIBLE_LINES (5) lines, with a "… N more lines (full log at .shardmind/logs/<slot>.log)" pointer.
  • Full output persisted to a vault-local .shardmind/logs/<slot>.log (new HOOK_LOGS_DIR) when a hook crashes or its output would truncate. The write is non-fatal (a failure just omits the pointer); short clean hooks write nothing; the log is under .shardmind/, so it's excluded from Invariant 1.

Layering: the orchestrator owns the write (attachHookLog — it has vaultRoot + the HookResult + fs access) and threads logPath through HookSummary; the component only reads it. The 5-line threshold lives once in core/hook.ts (headLines), shared by the write-decision and the render.

Cross-platform

CI runs npm test on ubuntu + windows + macos × node 22/24. The two platform-variable axes are handled by construction and pinned by tests that run on every cell:

  • Line endings: headLines splits on /\r?\n/ and re-joins with \n (pure string logic) — a Windows hook's CRLF no longer leaves a visible \r. The on-disk log keeps raw bytes.
  • Path separators: the displayed logPath comes from a forward-slash template (hookLogRelPath), never path.join, so all OSes show .shardmind/logs/<slot>.log. path.join is used only for the fs write. tests/unit/vault-paths.test.ts asserts the forward-slash contract (no \, even on win32) and the orchestrator test reads the file back via OS-native path.join — both run on the Linux + macOS cells. The L2 PTY scenario (Windows-skipped, E2E: bridge SIGINT delivery reliably on GH Actions Windows runner #57) covers the real-terminal render on macOS + Linux.

Quality gate

  • npm run typecheck clean
  • npm test green — 1146 passed / 30 skipped (1125 → 1146, +21 tests)
  • npm run build clean
  • New behavior has new tests (no code without tests)
  • No any / @ts-ignore / as unknown as in the diff (grep clean)
  • Invariant 1 (v6 engine: install --defaults + Invariant 1 CI test #78) + obsidian-mind contract four-branch hook tests stay green (.shardmind/logs/ excluded from byte-equivalence)
  • Roadmap checkbox updated (this PR)

Harden audit

Reference bar: #102 (hook lifecycle), #120 (adopt merge) — adversarial enumeration + property/edge tests + cross-OS CI.

Rounds

  • R1 /simplify (4 agents): clean — headLines/hookLogRelPath/outputBlock have no existing equivalent; attachHookLog matches the established non-fatal-write pattern (update-check.ts). Marginal suggestions skipped.
  • R2 /harden (adversarial + correctness + docs): 1 real bug + 3 missing tests. Docs + correctness clean.
  • R3 re-read: clean (trim consistent between write-decision and render; pointer gated on truncation; all HookSummary branches intact).

Real bug found + fixed

  • CRLF artifacts in headLines — split('\n') left a trailing \r on Windows hook output that rendered as a visible control char (source/core/hook.ts). Fixed → /\r?\n/.

Tests added (unit / component / e2e): headLines matrix incl. CRLF; orchestrator log-persistence (crash / long-clean / short-clean / dry-run / unwritable-non-fatal) + format markers; HookSummarySection truncation + pointer + crash-header + 2 edge cases; vault-paths platform-invariance; L2 PTY scenario 27 (truncated UI + on-disk marker). 1125 → 1146.

Deferrals: none.

Commits

  1. feat(core) — log persistence + headLines + orchestrator wiring
  2. feat(components) — crash header + truncate/dim + pointer
  3. test(e2e) — L2 scenario 27
  4. docs — CHANGELOG + SHARD-LAYOUT + ARCHITECTURE §9.3 + AUTHORING §6
  5. docs(roadmap) — check Summary: hook stderr presentation buries success when hook crashes #105 + done-gate boxes
  6. test(harden) — CRLF fix + cross-platform path guards

breferrari and others added 6 commits June 2, 2026 01:10
@breferrari @claude
feat(core): persist full hook output to .shardmind/logs/<slot>.log (#105
9e00c25 
)

When a hook crashes (non-fatal, Helm semantics) or emits more output than the
Summary shows inline, the orchestrator now writes the complete captured
stdout/stderr to a vault-local log so the Summary can truncate on screen and
point at the full file.

- vault-paths: HOOK_LOGS_DIR + hookLogRelPath(slot) (posix, doubles as the
  user-facing pointer). Under .shardmind/, so excluded from Invariant 1.
- hook.ts: HookSummary.logPath; HOOK_OUTPUT_VISIBLE_LINES + pure headLines()
  helper shared by the write-decision and the on-screen truncation.
- hook-orchestrator: attachHookLog() writes the log only when the hook crashed
  or its output would truncate; non-fatal (a write failure just omits the
  pointer); dry-run / skipped slots write nothing.

Tests: headLines unit matrix; orchestrator log persistence (crash writes log,
long-clean writes log, short-clean writes nothing, dry-run writes nothing,
unwritable logs/ path is non-fatal).

Part of #105.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@breferrari @claude
feat(components): truncate + dim crashed-hook output, point at full l…
9a7dd0e 
…og (#105)

A crashed hook is non-fatal, but the Summary dumped its raw multi-line stderr
verbatim, so a successful install with a crashed hook looked like an engine
failure. Now:

- crash line reads "<hook> exited with code N. Non-fatal; your vault is ready."
  with the "operation succeeded" detail on its own dim line, so the headline
  stays the command outcome.
- stdout/stderr render dimmed + indented, truncated to HOOK_OUTPUT_VISIBLE_LINES
  via the shared headLines() helper, with a dim "… N more lines — full log at
  <path>" pointer when there's more (path present when the orchestrator wrote a
  log).

Bold "Hook stdout:" / "Hook stderr:" labels and the contiguous "exited with
code N" phrasing are preserved, so existing Summary / UpdateSummary tests stay
green. New tests cover crash truncation + log pointer, clean-exit truncation,
within-cap (no pointer), and truncation with no log path.

Part of #105.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@breferrari @claude
test(e2e): L2 scenario 27 asserts truncated crash UI + full on-disk l…
5f92884 
…og (#105)

Under a real PTY, a thrown bootstrap hook now renders the reworded crash
headline ("exited with code N. Non-fatal; your vault is ready.") with the
stderr stack truncated and a "(full log at .shardmind/logs/bootstrap.log)"
pointer, instead of dumping the whole stack. The full thrown marker is verified
in the on-disk log rather than on screen — robust against any node/tsx stderr
preamble shifting it off the 5-line head.

Also drops the em-dash from the pointer separator (parenthetical form).

Verified no regression: the obsidian-mind contract four-branch hook tests and
the Invariant 1 byte-equivalence suite stay green (.shardmind/logs/ is excluded
from byte-equivalence).

Part of #105.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@breferrari @claude
docs: hook crash presentation + .shardmind/logs/ (#105)
5346d34 
- CHANGELOG: Changed entry for the truncate/dim/non-fatal Summary + the
  vault-local full-output log.
- SHARD-LAYOUT: add logs/ to the installed-side .shardmind/ tree; note
  HOOK_LOGS_DIR + that everything under .shardmind/ is excluded from
  Invariant 1.
- ARCHITECTURE §9.3: reword the "if a hook throws" warning to match the new
  copy and describe truncation + the on-disk log.
- AUTHORING §6: tell shard authors where a crashed hook's full output lands.

Part of #105.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@breferrari @claude
docs(roadmap): check #105; flip done Done-gate boxes (#105)
a4ff0e8 
#105 (hook stderr presentation) was the last open Flagship-UX item, so that
gate line is now complete. Also mark the genuinely-finished Done-gate lines:
Foundation (#111/#112) and Parallel (#113/#119) both shipped. Hook lifecycle
stays unchecked (engine done, but obsidian-mind#75 migration pending);
research-wiki (#15), docs polish (#85), and the two-shard smoke remain open.

Part of #105.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@breferrari @claude
test(harden): CRLF-safe headLines + cross-platform path guards (#105)
cf4ba98 
Adversarial audit found one real cross-platform bug and gaps:

- fix: headLines split on /\r?\n/ (was '\n'), so a Windows hook's CRLF output
  no longer leaves a trailing \r that renders as a visible control char; the
  head re-joins with LF (canonical display). The full on-disk log keeps the
  hook's raw bytes.
- new tests/unit/vault-paths.test.ts: hookLogRelPath is always forward-slash
  ".shardmind/logs/<slot>.log" (never a backslash, even on win32) and joins to
  a valid OS-native path under the vault — pins the display contract on the
  full ubuntu/windows/macos CI matrix.
- headLines CRLF unit case; log-format markers pinned in the orchestrator crash
  test; component edge cases (logPath set but output short → no pointer; crash
  with empty output → header only).

Part of #105.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 1, 2026 23:36
Copilot AI reviewed Jun 1, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves how non-fatal hook crashes (Helm semantics) are presented in the final Summary so successful installs/updates/adopts aren’t visually buried by multi-line hook stderr, while still preserving full output for debugging.

Changes:

  • Add a shared headLines helper + HOOK_OUTPUT_VISIBLE_LINES (5) to truncate hook stdout/stderr consistently across the orchestrator and UI, including CRLF normalization.
  • Persist full hook output to vault-local .shardmind/logs/<slot>.log (non-fatal best-effort) when a hook crashes or output would truncate; thread logPath through the hook summary.
  • Update TUI rendering to show a clearer non-fatal crash header, dim/indent truncated output, and include a “full log at …” pointer; add comprehensive unit/component/e2e coverage and docs/roadmap/changelog updates.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unit/vault-paths.test.ts Pins forward-slash display-path contract for hook log pointers across OSes.
tests/unit/hook.test.ts Adds unit tests for headLines, including CRLF and trailing-newline behavior.
tests/unit/hook-orchestrator.test.ts Covers log persistence decision + non-fatal write failure behaviors.
tests/e2e/tui/hook-lifecycle.test.ts Validates truncated UI + presence of persisted full stderr marker in log.
tests/component/HookSummarySection.test.tsx Verifies crash header, truncation, pointer rules, and edge cases.
source/runtime/vault-paths.ts Introduces HOOK_LOGS_DIR and hookLogRelPath (forward-slash display path).
source/core/hook.ts Adds HOOK_OUTPUT_VISIBLE_LINES, headLines, and plumbs logPath into HookSummary.
source/core/hook-orchestrator.ts Adds attachHookLog and on-demand .shardmind/logs/<slot>.log writes.
source/components/HookSummarySection.tsx Updates hook Summary UX: non-fatal crash headline, dim/indented truncated output, log pointer.
ROADMAP.md Marks #105 as completed and updates done-gate checkboxes.
docs/SHARD-LAYOUT.md Documents .shardmind/logs/ as engine metadata for persisted hook output.
docs/AUTHORING.md Documents new crash presentation + where to find full hook logs.
docs/ARCHITECTURE.md Updates hook-crash contract section to include truncation + log persistence behavior.
CHANGELOG.md Records user-visible Summary behavior change and log persistence feature.

@breferrari @claude
fix(test): L2 scenario 27 — throw hook's short stack isn't truncated (#…
bcfa3fc 
…105)

CI (ubuntu/macos) caught a wrong assumption in my own test, not a feature bug.
The thrown-hook fixture's stack is only ~3 lines — under HOOK_OUTPUT_VISIBLE_LINES
(5) — so the Summary does NOT truncate it and shows no "(full log at …)" pointer.
The waitForScreen predicate required that pointer and timed out.

Fix: assert what actually renders (confirmed by the CI screen dump) — the reworded
"exited with code N. Non-fatal; your vault is ready." headline + the marker on
screen — and still verify the full output is persisted to .shardmind/logs/bootstrap.log
on disk (a crashed hook always writes its log, regardless of truncation). The
truncation + pointer path stays covered at the component layer.

Part of #105.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@breferrari breferrari merged commit c118ba1 into main Jun 1, 2026
6 checks passed
@breferrari breferrari deleted the feat/105-hook-stderr-presentation branch June 1, 2026 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Summary: hook stderr presentation buries success when hook crashes

2 participants

@breferrari