Add npm publishing and update documentation

[jbrejner]

Distribute kosli cli as npm package.
Install globally with npm install -g @kosli/cli

The above will install the wrapper package which detects the current platform and architecture and then pull the appriate package with the correct binary.

Testable packages exist, they are tagged with "snapshot" so you can test with npm install -g @kosli/cli --tag snapshot

[AlexKantor87]

PR #719 Multi-Agent Review: "Add npm publishing and update documentation"

Repository: kosli-dev/cli | Author: jbrejner | Status: DRAFT Branch: 4590_cli-as-npm-package → main Stats: +503 / -5 across 30 files | 5 commits PR URL: https://github.com//pull/719

---

The Review Panel

Four agents with distinct personas reviewed this PR independently, then their findings were synthesized. Here's who was on the panel:

Agent	Persona	Focus Area
Sasha	Security Engineer — paranoid, cynical, thinks in attack surfaces	Secrets management, supply chain risks, token handling
Dev	DevOps/CI Engineer — battle-hardened, pragmatic, "what fails at 2am?"	GoReleaser integration, pipeline reliability, failure modes
Priya	NPM Packaging Expert — meticulous, encyclopedic npm knowledge	Package.json correctness, DX, npm ecosystem patterns
Doc	Technical Writer & DX Advocate — empathetic, user-journey focused	Documentation completeness, onboarding experience, error UX

---

Unanimous Verdict: DO NOT MERGE (yet)

All four agents independently reached the same conclusion: the PR has a sound architectural foundation (the esbuild-style per-platform binary distribution pattern is well-chosen) but has several blocking issues that would result in a non-functional npm package if published today. This aligns with the PR's own DRAFT status and the author's listed TODOs.

---

The Big Three: Issues Every Agent Flagged

These three issues were independently identified by all four reviewers, making them the highest-confidence findings:

1. The Missing bin/kosli JS Shim (flagged by: ALL FOUR)

The wrapper's package.json declares "bin": {"kosli": "bin/kosli"}, but the JavaScript shim file that should live at npm/wrapper/bin/kosli is nowhere in the diff. Worse, .gitignore adds npm/wrapper/bin/*, which would prevent committing it.

• Sasha (Security): "Users get command not found — package is non-functional"
• Dev (CI/CD): "No evidence the shim exists or is tested"
• Priya (NPM): "This is the most critical issue — every installation will be broken"
• Doc (DX): "The README describes a file that doesn't exist in the repo. Is it generated? By what?"

Consensus: This is likely an oversight where the shim was developed locally but blocked by the gitignore pattern. The shim must be a committed JS file (not a binary), so it shouldn't be gitignored.

2. Token Variable Mismatch (flagged by: Sasha, Dev, Priya)

The .npmrc files reference ${MY_LOCAL_NPM_TOKEN}, but the CI workflow sets NPM_TOKEN. The publish script doesn't bridge this gap.

• Sasha: "npm publishing will FAIL due to authentication errors — incomplete implementation"
• Dev: "Silent npm ERR! 401 Unauthorized failures in CI"
• Priya: "Confusion about where tokens go; risk of accidentally committing tokens"

Consensus: The token naming needs to be unified, or the publish script needs to explicitly set the npm auth token from the CI environment variable.

3. Silent Postinstall Failures (flagged by: ALL FOUR)

The install.js postinstall script exits with code 0 on every failure path — unsupported platform, missing binary, failed validation. Users get no clear signal their installation is broken.

• Sasha: "Weak validation — a trojanized binary that still executes passes silently"
• Dev: "No binary validation — corrupt/truncated binaries could ship"
• Priya: "Violates the postinstall contract. Users will debug for hours"
• Doc: "No troubleshooting section for the errors this script silently swallows"

Consensus: At minimum, the script should exit non-zero when the binary is missing or fails validation on a supported platform. The current "never fail" approach is too permissive.

---

Where the Agents Diverged: The Interesting Debates

The sed -i Question

Dev and Priya both flagged the use of sed -i in npm-publish.sh to mutate package.json files, but for different reasons:

• Dev worried about atomicity: "If sed fails partway, repo is left corrupted. No backup."
• Priya worried about portability: "macOS BSD sed needs sed -i '', Linux doesn't. This will break local dev."

Sasha didn't flag this — from a security perspective, sed in a CI pipeline is unremarkable. Doc didn't flag it either — it's invisible to users.

The nfpms Template Change

Dev was the only agent to catch that the .goreleaser.yml nfpms section removed the {{ else if eq .Arch "arm64" }}{{ .Arch }}_v8.0 clause. Dev called this "undocumented arm64_v8.0 suffix removal" and questioned whether it was intentional or a regression affecting Linux ARM64 package builds.

The other three agents didn't notice this change buried in the goreleaser config — it's a subtle, domain-specific issue that only a CI specialist would spot.

Supply Chain Irony

Sasha made an observation unique to the security review: "No SHA256 verification, no SLSA attestation verification — ironic given Kosli IS a compliance tool." This philosophical point — that a compliance tool's own distribution lacks the verification it recommends — wasn't raised by the others.

Error Messages vs. Error Handling

Doc and Priya had subtly different takes on the postinstall problem:

• Priya wanted the script to exit non-zero (break the install if things are wrong)
• Doc wanted better error messages and a troubleshooting guide (help users fix it)

These aren't contradictory — you need both — but they reflect the different priorities of a package maintainer vs. a documentation writer.

---

Per-Agent Unique Findings

Sasha (Security) — Unique Catches

• Attack surface analysis: Identified 5 new supply chain vectors (platform substitution, wrapper hijacking, typosquatting, token leakage, CI escape)
• .npmrc files as a pattern risk: Even with variable placeholders, committing .npmrc files establishes a dangerous pattern for future contributors

Dev (DevOps) — Unique Catches

• No rollback on partial publish: If npm publish fails on package 3 of 7, the script continues. Result: broken release with some platforms available, others missing
• Cleanup hooks don't verify success: The before hooks assume deletion succeeded but don't check for permission issues
• GoReleaser after hook conditional: Noted the snapshot/dry-run conditional is well-designed

Priya (NPM) — Unique Catches

• Missing engines field: No Node.js version constraint in any package.json
• Missing repository.directory: Monorepo packages should include "directory": "npm/wrapper" for npm registry navigation
• Missing trailing newlines: Several package.json files lack POSIX-standard trailing newlines (will cause linter warnings and potential npm auto-fix diffs)
• 0.0.0 version semantics: npm interprets "0.0.0" as an exact pin, not "any version" — the publish script must update these atomically

Doc (Technical Writer) — Unique Catches

• Frontmatter syntax error: Line 1 of install.md changed --- to --- (leading space) — will break Hugo rendering
• No npx documentation: Users discovering the package on npmjs.org will naturally try npx @kosli/cli — is this supported?
• Tree diagram formatting errors: The README has two └── at the same indentation level (cosmetic but sloppy)
• Per-platform READMEs need "install @kosli/cli instead" guidance: Users who find @kosli/cli-linux-x64 on npm need to be redirected

---

Consolidated Recommendations (Priority Order)

Must-fix before merge

1. Commit the bin/kosli JS wrapper shim (and fix .gitignore to allow it)
2. Align token variable naming between .npmrc and CI (NPM_TOKEN vs MY_LOCAL_NPM_TOKEN)
3. Fix install.md frontmatter leading space (line 1)
4. Make postinstall exit non-zero when binary is missing/broken on supported platforms
5. Add error handling for partial publish failures in npm-publish.sh
6. Confirm the nfpms arm64 template change is intentional

Should-fix

7. Add "engines": { "node": ">=16.0.0" } to wrapper package.json
8. Add trailing newlines to all package.json files
9. Add troubleshooting section to install docs
10. Document Node.js version requirements in the NPM install tab
11. Fix tree diagram formatting in npm/README.md
12. Add repository.directory to wrapper package.json

Nice-to-have

13. Add binary integrity verification (checksums)
14. Document npx behavior
15. Improve per-platform README content
16. Add CI test step that actually installs and runs the npm package

---

Process Observations

What worked well about multi-agent review

• Coverage breadth: Each agent caught issues the others missed (Dev's nfpms regression, Doc's frontmatter error, Priya's engines field, Sasha's supply chain vectors)
• Confidence calibration: Issues flagged by 3-4 agents are clearly high-priority; issues flagged by only 1 agent are worth investigating but may be lower priority
• Perspective diversity: The same issue (silent postinstall) was analyzed from four angles — security (attack surface), reliability (failure modes), correctness (npm contract), and UX (user confusion)

What was interesting about the disagreements

• Agents naturally weighted the same evidence differently based on their expertise
• No actual contradictions — disagreements were about priority, not facts
• The "overlap zone" between agents was the most valuable (3-4 agents flagging the same issue independently = high confidence)

---

Review conducted: 2026-03-24 PR URL: https://github.com//pull/719

[claude]

Claude finished @jbrejner's task in 2m 18s —— View job

---

PR Review: Add npm publishing and update documentation

• Run static analysis (build, vet, mod tidy)
• Read and review changed files
• Post review findings

---

Static Analysis

All clean:

• go build ./... — passes
• go vet ./... — passes
• go mod tidy — no changes to go.mod/go.sum

Review Summary

This PR is in good shape. The esbuild-style per-platform binary distribution pattern is well-executed, and most issues from earlier review rounds have been addressed. The bin/kosli shim is committed, .npmrc tokens are aligned (NPM_TOKEN), install.js exits non-zero on unsupported platforms, and the tree diagram is now correct.

Two remaining items (posted as inline comments):

1. scripts/npm-publish.sh:44 — Missing jq failure cleanup on wrapper update. The platform loop (line 38) has || { rm -f "$tmp"; exit 1; } but the wrapper optionalDependencies update on line 43-44 doesn't. Temp file leaks and silent failure if jq/mv fails.

2. scripts/npm-publish.sh:12 — Dead code. [ "$2" = "true" ] is unreachable — GoReleaser passes --dry-run, never "true". Minor cleanup.

What's Working Well

• Architecture: The esbuild-inspired optionalDependencies pattern is the right choice for native binary distribution via npm.
• Publish script: Two-phase pack-then-publish with retry logic and proper error propagation is solid.
• GoReleaser integration: The post-build hooks for copying binaries into npm package dirs and the after hook for publishing are clean.
• CI coverage: The test-npm job across 5 platform runners (including ARM) is thorough. The comment explaining that push/PR tests the published version while release tests the new version is helpful.
• Install validation: The postinstall running kosli version to verify the binary actually executes (not just exists) is a nice touch.