fix: production-readiness pass for v1.0.2 - #1
Merged
Merged
Conversation
`npm run check` only covered `src/`, so nothing validated the test files. That is how a typo in an option key survived: the code read `options.noAutoContinue`, which Commander never sets, and no compiler ever looked at the tests that would have caught it. `tsconfig.check.json` now typechecks `src/` and `tests/` together. `pretest` builds before running, so the suite always exercises the current artifact rather than a stale `dist/`. The `minimatch@3.1.5` override pinned `brace-expansion` to 5.0.8, which has since fallen inside the advisory's vulnerable range. Replaced with direct overrides on the patched versions. Production dependencies were already clean; every finding was in the dev toolchain. Also excludes the unused plugin registry from the published tarball and ignores `npm pack` output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
Human-readable status text went to stdout via `console.log`, so it landed
in redirected output alongside the document. With `--git` the repository
details were printed ahead of the document, corrupting the result of
cat notes.md | superdocs edit --git -p "..." > out.md
stdout now carries only the payload -- the edited document, the dry-run
diff, a completion script, or the JSON stream. Progress, status, hints,
warnings, and errors all go to stderr. `ILogger` gains `output()` for the
payload so the distinction is explicit at every call site.
`--json` printed pretty-formatted objects, so a run that emitted progress
events plus a result could not be parsed by any line-oriented reader.
Output is now one compact object per line, each stamped with
`schema_version` so consumers can detect format changes.
Loading `ora` costs ~100 ms and is pure waste for `--json`, `--quiet`, and
CI, so the spinner defers its import until something actually starts one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
…harden credentials Three separate defects in how the CLI resolves its own configuration. 1. `SUPERDOCS_API_BASE_URL` never worked. The `--api-url` option declared a Commander default of `https://api.superdocs.app`, which always populated `options.apiUrl` and permanently shadowed the environment variable in options.apiUrl ?? process.env.SUPERDOCS_API_BASE_URL ?? DEFAULT Anyone pointing the CLI at a self-hosted or development API was silently sending their documents to the public API instead. The option no longer declares a default, and a single `resolveBaseUrl()` owns the precedence chain so `login` cannot drift from `status` again. 2. `--version` printed a hardcoded `1.0.0` and had already drifted from the published 1.0.1. It now reads `package.json` at runtime. 3. Credentials were protected only by `chmod(0o600)`, which is a no-op on Windows -- the API key sat in a file readable by anything running under the account. `login` now applies an explicit `icacls` ACL on win32 and warns when hardening cannot be applied. Adds `UsageError` so caller mistakes exit 2 rather than the generic 1, and moves zod-free config primitives into `configPath.ts` so `Logger` can read the `verbose` default without dragging the schema layer onto startup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
…prove `--no-auto-continue` had never taken effect. Commander represents a negated flag as `autoContinue: false`, but the code read `options.noAutoContinue`, a key that is never set, so large edits always auto-continued despite the flag being documented in help, the README, and the completion scripts. When the flag is used in a non-interactive terminal the paused job is now cancelled rather than abandoned server-side. `--git` collected the repository root and changed files, printed them, and threw them away -- the model never saw the context the flag advertises. The context, now including the current branch and a bounded file list, is sent with the instruction. Interrupt handling had three owners: `edit`, `editWatch`, and the cleanup manager each registered handlers, and watch mode leaked a SIGINT/SIGTERM pair per run while a second Ctrl+C did nothing. One AbortController now owns cancellation; a second interrupt escalates to a hard exit. Watch mode registered its watcher only after the initial edit completed, so a save made during that first pass was lost until the next unrelated change. The watcher starts first and queued changes drain once the initial pass finishes. Adds `--approve <all|ask>`, exposing the API's approval mode. The client already had the plumbing for `ask_every_time`, but `approve_all` was hardcoded, so a tool that overwrites files had no way to ask first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
Every existing test stubbed `ISuperDocsClient`, so the most failure-prone path in the CLI -- upload, chat, job polling, export, atomic write -- was never actually executed. Two new layers: - integration: drives `executeSingleEditCycle` against an in-process HTTP mock, covering the presigned upload path, 5xx retry, job failure, approval handling, Git context, and the stdout/stderr split. - end-to-end: spawns the built `dist/index.js` as a subprocess against a full mock SuperDocs API, covering every command, flag, exit code, and both output streams. This layer is what surfaced the SUPERDOCS_API_BASE_URL bug: the suite could not reach its own mock server, because the CLI was quietly calling the production API. Runs are sandboxed via SUPERDOCS_CREDENTIALS_PATH and SUPERDOCS_CONFIG_PATH, and the ambient environment is stripped, so the suite can never read or overwrite a developer's real login. `version.test.ts` walks the static import graph to keep zod and ora off the startup path deterministically, rather than asserting on timings. 148 tests total, up from 53. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
The repository had no automation at all. `prepublishOnly` was the only gate, so nothing ran on a pull request and nothing verified behaviour on platforms other than the maintainer's. CI runs lint, formatting, and typechecking once, then the full suite across Linux, macOS, and Windows on Node 20, 22, and 24. The CLI writes files, takes locks, and resolves per-OS config paths, so platform coverage is not optional. A dedicated job asserts the built binary reports the same version as package.json, and checks the packed tarball and audit. The release workflow publishes on a `v*` tag with npm provenance, refusing to publish if the tag and package.json disagree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
DESIGN.md claimed credential files were written "with owner-only permissions where supported", which read as reassuring but was false on Windows, the platform the benchmarks were measured on. It now states what actually happens on each platform. Documents the stdout/stderr contract and the newline-delimited JSON format, both of which the piping examples in the README depend on. Corrects two examples that could never have worked: `superdocs edit --git` with no file or stdin, and a `jq` invocation that assumed single-object output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
`npm install` rewrites package-lock.json in npm's own format, so any contributor who installs dependencies would fail `format:check` through no fault of their own. Lockfiles and build output are generated artifacts and do not belong in the formatter's scope. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
CI caught this on Linux the first time it ever executed. The test called
`saveApiKey("sk_test")`, a seven-character key that fails `ApiKeySchema`'s
minimum length, so it threw a validation error before `saveApiKey` ever
reached `assertNotSymlink`. The assertion then failed to match the message
it was looking for.
It went unnoticed because the test skips when symlink creation is
unavailable, which is the default on Windows, and there was no CI. A
documented part of the credential safety model had therefore never been
verified on any platform.
Also bumps the workflow actions to the current majors, clearing the Node 20
deprecation warnings, and splits the audit gate: production dependencies
are a hard failure, dev-tree advisories are reported but do not block a
release of the CLI.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
AbhinayYendoti
force-pushed
the
fix/v1.0.2-production-readiness
branch
from
August 29, 2026 11:25
09d8c8c to
8912a53
Compare
Windows CI could not find any tests on Node 20:
Could not find 'D:\a\Superdocs-cli\Superdocs-cli\tests\*.test.ts'
`node --test tests/*.test.ts` needs the *shell* to expand the glob. bash and
zsh do; PowerShell and cmd pass the literal string through. Node's own glob
handling in `--test` landed after our Node 20 floor, so on Node 22 and 24 the
pattern happened to work and on Node 20 it did not.
The practical effect: `npm test` was broken for any Windows contributor on
the exact Node version package.json declares as the minimum. It went
unnoticed because the maintainer runs the suite through Git Bash, which
expands the glob before Node sees it.
`scripts/run-tests.mjs` now discovers the files itself, so behaviour is
identical on every shell and every supported Node version. It also accepts
substring filters: `npm test -- e2e`.
Separately, the watch-mode end-to-end test timed out on a Windows runner. It
now uses a one-second poll interval, allows a longer budget for two full edit
cycles, and reports the child process's stderr on timeout so a CI failure is
diagnosable without a rerun.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
The previous commit raised WATCH_TIMEOUT_MS but left the three call sites still passing a hardcoded 25_000, so nothing changed and the Windows runner timed out again with no extra detail. The call sites now use the constant. Waiting out a full timeout to learn nothing is the wrong failure mode. The test now aborts immediately if the watch process exits, and every timeout message carries the child's stdout and stderr, so a CI failure explains itself without a rerun. Windows runners take roughly twice as long as Linux for two full edit cycles, so the budget is 60s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
`superdocs edit --watch` did not fail on Windows -- it died:
Assertion failed: !_wcsnicmp(filename, dir, dirlen), src\win\fs-event.c:72
watch process exited early (code 3221226505, signal null)
libuv's Windows fs-event backend compares the filename the OS reports
against the directory string it was handed. Given an 8.3 short path such as
C:\Users\RUNNER~1\AppData\Local\Temp, the OS reports long names, the
comparison fails, and libuv aborts. 0xC0000409 is a fast-fail: no exception
is raised, so no amount of error handling in the CLI could have caught it.
Any Windows account whose profile name exceeds eight characters can produce
such a path, so this was reachable by real users, not just CI. It never
reproduced locally because the maintainer's profile is `HP` -- too short for
Windows to generate a short name -- and because this volume has 8.3
generation disabled entirely.
`resolveRealPath` expands short components before `fs.watch` sees them, on
both the directory watch and the single-file fallback.
The regression test prefers an ambient short temp directory, which is the
shape that actually crashed, and falls back to asking the OS to generate
one. It skips where 8.3 names are unavailable, so the end-to-end watch test
on the Windows runner remains the real proof.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Correctness and hygiene pass for
v1.0.2. No new commands; one new flag.Thirteen defects, found by building an end-to-end suite that runs the shipped binary against a mock API. Every one is covered by a regression test.
The serious one
SUPERDOCS_API_BASE_URLnever worked.--api-urldeclared a Commander default ofhttps://api.superdocs.app, which always populatedoptions.apiUrland permanently shadowed the environment variable:Anyone pointing the CLI at a self-hosted or development API was silently sending their documents to the public API. This is how it surfaced: the new E2E suite could not reach its own mock server.
Everything else
--version1.0.0; already drifted from the published1.0.1--no-auto-continuenoAutoContinue; Commander setsautoContinue. Never took effect--git)--json--git--watchchmod(0o600)only — a no-op on Windows--promptexited1, not the documented usage code2--versionand--helpnoAutoContinuetypo survivedAdded
--approve <all|ask>. The client already hadask_every_timeplumbing, butapprove_allwas hardcoded, so a tool that overwrites your files had no way to ask first. Default unchanged.Verification
Tests grew 53 → 148 across three layers: unit, integration (in-process mock), and end-to-end (spawns
dist/index.jsagainst a full mock API). E2E runs are sandboxed viaSUPERDOCS_CREDENTIALS_PATH/SUPERDOCS_CONFIG_PATHand strip the ambient environment, so they cannot touch a real login.Startup module-loading overhead dropped ~40% by keeping zod and ora off the registration path; a test walks the static import graph so they cannot creep back.
Reviewer notes
SUPERDOCS_API_BASE_URLnow takes effect. If any deployment was accidentally relying on it being ignored, this will redirect that traffic.--approvedefaults toall, preserving today's unattended behaviour. Makingaskthe default for in-place writes is safer but breaking, so it is left for v1.1.src/plugins/is excluded from the tarball but still in the tree; it is imported by nothing and should be deleted in a follow-up.🤖 Generated with Claude Code
https://claude.ai/code/session_01WQGKLbzQaxvSZzAxfsDe2Y