Skip to content

Add markpost push --dry-run to preview files before creating records - #140

Open
grimicorn-agent wants to merge 5 commits into
mainfrom
agent/push-dry-run
Open

Add markpost push --dry-run to preview files before creating records#140
grimicorn-agent wants to merge 5 commits into
mainfrom
agent/push-dry-run

Conversation

@grimicorn-agent

Copy link
Copy Markdown
Collaborator

What & why

Adds markpost push --dry-run, a read-only preview of a bulk import. A push resolves its whole plan up front via resolveMarkdownInputs (files, missing, skipped) before pushFiles creates any records, so --dry-run prints that resolved plan and returns before any createRecord / POST /api/records call. A bulk import across globs/directories otherwise has no way to confirm the glob matched the right notes before it hits the API once per file; the dry run catches a wrong glob cheaply and mirrors the sync --dry-run affordance.

Viewable by running markpost push <glob> --dry-run (CLI; no hosted page).

Closes #136

Behaviour

  • markpost push --dry-run <path...> resolves inputs, prints the missing/unreadable inputs, then a yellow Dry run — header over a dim Would push N file(s): list, and returns without creating records.
  • Exits non-zero when inputs went unresolved (a wrong glob / unreadable path) — the same honesty the real run has, minus per-file push failures a dry run can't produce.
  • A dry run makes no network calls, so it does not require a configured token (preview a glob before setting up auth). The real push still gates on config.

Decisions

  • Did not reuse sync's reportDryRunPlan. That previews server records written to local files; push previews local files sent to the server — disjoint inputs. Only the visual style is shared (mirrored), not the code, so there's no shared logic to abstract (rule of three).
  • Mistyped-flag guard. Push creates server records, so a fat-fingered --dryrun must fail loud rather than silently run the real push. Unknown -- flags are rejected (Unexpected arguments: …), matching the guard sync gives its one destructive command. A genuine dash-leading filename stays pushable via the POSIX -- end-of-options separator (push -- --notes.md).
  • Terminal sanitization. The new preview and the existing per-file push output can surface a server-controlled record title/uuid or a directory-walk filename the user never typed; those lines are now run through sanitizeForTerminal (as sync's printWritePreview already does) so a crafted title/name can't inject a live escape sequence.

Tests

New cases in tests/commands/push.test.ts: dry run previews without calling createRecord/readMarkdown; lists each resolved file; surfaces missing and skipped inputs and exits 1; empty resolution still exits 1; --dry-run excluded from resolved paths and doesn't check config; mistyped flag rejected (with and without a valid --dry-run present); -- separator keeps dash-leading paths pushable; control characters stripped from a previewed path and from a pushed record title. Full suite: 718 passing.

Follow-up suggestions

  • Preview and push should flag unreadable regular files — a mode-000 regular file stats fine so it lands under "would push" / gets attempted, then the real read throws EACCES; resolveMarkdownInputs could pre-check readability (accessSync R_OK) so both the preview and the real push report it as skipped up front (suggested: P3, effort: M, evidence: src/libs/files.ts resolveMarkdownInputs + src/libs/markdown.ts readMarkdown)
  • Share one --dry-run flag literal across commandsDRY_RUN_FLAG = '--dry-run' is declared independently in src/commands/push.ts and src/index.ts; a single exported constant (e.g. in src/libs/usage.ts) would stop the literal drifting as more commands gain the flag (suggested: P4, effort: S, evidence: src/commands/push.ts + src/index.ts)

@grimicorn-agent grimicorn-agent added the has-suggestions PR carries follow-up suggestions for the improvement digest label Aug 27, 2026
@grimicorn-agent

Copy link
Copy Markdown
Collaborator Author

Independent code review trail

Ran the independent reviewer (claude --model opus) on the branch diff across multiple rounds; each round's findings and disposition:

Round 1

  • Typo'd flag silently runs the real pushfixed: reject unknown -- flags with Unexpected arguments, mirroring sync's guard for its destructive command.
  • Dry run requires a configured token it never usesfixed: checkConfig is now skipped on a dry run (no network calls).
  • README doesn't document push --dry-runfixed: updated the command table row.
  • DRY_RUN_FLAG duplicated in push.ts and index.tsskipped: two occurrences is under rule-of-three, and extracting would edit the unrelated sync flow. Filed as a follow-up suggestion instead.

Round 2

  • Nested exit-code guard in the dry-run branchfixed: folded the exit-code decision into reportPushDryRun, matching how reportSummary owns its own exit code.
  • skipped inputs untested on the dry-run pathfixed: added a dry-run test with a skipped input.
  • - flag prefix rejects dash-leading paths (regression)fixed: switched to -- prefix so single-dash filenames still push.
  • Preview paths printed unsanitizedfixed: sanitized the preview output like sync's printWritePreview.

Round 3

  • Adjacent missing/skipped error lines still unsanitizedfixed: sanitized both (the skipped path is directory-walk-derived and genuinely untrusted).
  • Sanitization not covered by a testfixed: added a test that strips a control char from a previewed path (fails if the sanitize call is removed).

Round 4

  • -- prefix rejects --notes.md from a glob with no escape hatchfixed: honor the POSIX -- end-of-options separator so dash-leading filenames stay pushable.
  • Server-controlled record.title printed unsanitizedfixed: sanitized all of push's per-file output lines (success, per-file failure, abort summary).
  • Preview can't detect a mode-000 regular fileskipped: the preview faithfully mirrors the real command's own resolution phase; an accessSync pre-check would make the preview stricter than the actual push, creating divergence. Filed as a follow-up suggestion.

Unresolved after the loop: none blocking. The two skipped items are recorded as follow-up suggestions in the PR body. Lint clean; full suite 718 passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has-suggestions PR carries follow-up suggestions for the improvement digest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add markpost push --dry-run to preview which files would be pushed before creating records

2 participants