feat(vale): add the six Vale platform packages and pinned manifest - #72
feat(vale): add the six Vale platform packages and pinned manifest#72thecodedrift wants to merge 10 commits into
Conversation
1897d5b to
6c94ebf
Compare
A spec-only proposal is its own tip until its implementation is stacked on top, so the gate demanded it archive a change nobody had built yet. #70, #71, and #72 have failed this check on every run since July 28 for exactly that reason — weeks of red on PRs that were never merge-eligible, which is how a team learns to stop reading red. A draft cannot merge, so skipping it costs nothing: no unarchived change can reach `main` either way. `ready_for_review` is added to the trigger types because the default set (opened/synchronize/reopened) does not include it — without that, a draft could be marked ready and merged on a green that was never re-evaluated. That event is what preserves the guarantee. The tip rule is unchanged for PRs that are ready for review.
Vale is a load-bearing engine, so its binary has to arrive the way ast-grep's does. findSgBinary already resolves a platform package from our own module context and execs beside it — there is simply no Vale package to resolve. The one that exists downloads at postinstall, which runs under the consumer's package-manager policy; pnpm 10 blocks it by default, yielding no binary and no error. So: per-platform packages published from this repo, binary in the tarball, no bin, no code, no scripts — ast-grep's packaging without ast-grep's installation, whose hardlink step already failed here under pnpm dlx. Versioned <valeVersion>-<yyyymmddhhmmss>, all-prerelease. The Vale version stays legible, a packaging fix is a new timestamp on the same base, and a caret range provably cannot resolve — exact pinning is enforced by semver rather than convention. Binaries are fetched at release time, never committed, and verified against reviewed checksums. The workflow detects upstream releases unattended but opens a PR rather than publishing, so nothing ships on bytes nobody signed off on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q
The reference table only documented TSKL-, which reads as though it's the only bare identifier the integration understands. It isn't — OSS-23 linked and moved to In Review on PR creation for #69. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q
A `git clone --depth=N` implies --single-branch, which leaves the clone with only main tracked. Three visible symptoms — force-with-lease fails "stale info" on every branch, `push -u` can't store an upstream, and `gh pr create` needs --head — but the one that matters is quiet: rebase is only correct while the merge base is inside the shallow window, so as main advances it can reconstruct the wrong base without saying so. The force-with-lease breakage is worth calling out on its own, since its failure mode is people reaching for a bare --force instead. Both fixes are local settings; nothing is committed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q
Every one of these PRs carried skip-changeset while it was spec-only, which becomes wrong the moment implementation lands. Stating the impact in the proposal means the tip PR needs a changeset written, not a label kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q
6c94ebf to
817ae44
Compare
Unit 1 of a forward-merging stack. Adds the workspace packages that will carry the Vale binary, the manifest that pins what goes in them, and the changesets exclusion that keeps release.yml out of their versions. It publishes nothing and no consumer references it: the repository grows six directories and one JSON file. The matrix is six, not ast-grep's seven. Vale 3.17.1 publishes exactly six release assets, and there is no 32-bit Windows build to package. Names carry no libc or toolchain suffix because Vale ships one build per os/cpu pair, so a -gnu or -msvc suffix would disambiguate nothing while asserting a toolchain nobody verified. musl stays on the PATH fallback: upstream ships no musl asset, and the Linux binary is dynamically linked against glibc rather than built as a static Go binary, so it would not run there anyway. Each linux README says so plainly instead of leaving a user to meet it as a loader error. Every package declares os/cpu, files, and the upstream MIT LICENSE with attribution, and carries no bin, no scripts, and no code. The binary is gitignored inside each directory so a clean checkout stays binary-free and pnpm install succeeds with the binaries absent. .github/scripts/vale-manifest.json holds the pinned version once, and per platform the asset-name template, the archive member to unpack, and the SHA256 of the release archive. Upstream's checksums file covers the archives rather than the executables inside them, so a committed digest is independently checkable against upstream and the archive is verified before anything is unpacked from it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds the scaffolding for publishing Vale as per-platform npm packages from this repo (unit 1 of the forward-merging stack), including the six workspace packages, a pinned checksum manifest to gate future binary fetching, and changesets configuration to keep these packages out of the normal release flow.
Changes:
- Add six new
@taskless/vale-<os>-<cpu>workspace packages (metadata + docs; binaries intentionally not committed). - Add
.github/scripts/vale-manifest.jsonto pin Vale 3.17.1 + per-asset SHA256 digests for later verification. - Exclude the new packages from changesets versioning/publishing and record the plan/specs in OpenSpec + minor
CLAUDE.mddoc fixes.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds the six new workspace importers to the lockfile. |
| .changeset/config.json | Adds the six Vale platform packages to changesets ignore. |
| .github/scripts/vale-manifest.json | Introduces pinned Vale version + per-platform release-archive SHA256 digests (future release gate). |
| CLAUDE.md | Documents shallow-clone pitfalls + expands PR issue reference table. |
| packages/vale-darwin-arm64/package.json | New darwin/arm64 package metadata (os/cpu, publish config, files list). |
| packages/vale-darwin-arm64/README.md | New package documentation for darwin/arm64 distribution and versioning model. |
| packages/vale-darwin-arm64/LICENSE | Upstream MIT license included for redistribution attribution. |
| packages/vale-darwin-arm64/.gitignore | Ensures the fetched binary is never committed and local packs are ignored. |
| packages/vale-darwin-x64/package.json | New darwin/x64 package metadata (os/cpu, publish config, files list). |
| packages/vale-darwin-x64/README.md | New package documentation for darwin/x64 distribution and versioning model. |
| packages/vale-darwin-x64/LICENSE | Upstream MIT license included for redistribution attribution. |
| packages/vale-darwin-x64/.gitignore | Ensures the fetched binary is never committed and local packs are ignored. |
| packages/vale-linux-arm64/package.json | New linux/arm64 package metadata (os/cpu, publish config, files list). |
| packages/vale-linux-arm64/README.md | New package documentation for linux/arm64, including glibc/musl note. |
| packages/vale-linux-arm64/LICENSE | Upstream MIT license included for redistribution attribution. |
| packages/vale-linux-arm64/.gitignore | Ensures the fetched binary is never committed and local packs are ignored. |
| packages/vale-linux-x64/package.json | New linux/x64 package metadata (os/cpu, publish config, files list). |
| packages/vale-linux-x64/README.md | New package documentation for linux/x64, including glibc/musl note. |
| packages/vale-linux-x64/LICENSE | Upstream MIT license included for redistribution attribution. |
| packages/vale-linux-x64/.gitignore | Ensures the fetched binary is never committed and local packs are ignored. |
| packages/vale-win32-arm64/package.json | New win32/arm64 package metadata (os/cpu, publish config, files list). |
| packages/vale-win32-arm64/README.md | New package documentation for win32/arm64 distribution and versioning model. |
| packages/vale-win32-arm64/LICENSE | Upstream MIT license included for redistribution attribution. |
| packages/vale-win32-arm64/.gitignore | Ensures the fetched binary is never committed and local packs are ignored. |
| packages/vale-win32-x64/package.json | New win32/x64 package metadata (os/cpu, publish config, files list). |
| packages/vale-win32-x64/README.md | New package documentation for win32/x64 distribution and versioning model. |
| packages/vale-win32-x64/LICENSE | Upstream MIT license included for redistribution attribution. |
| packages/vale-win32-x64/.gitignore | Ensures the fetched binary is never committed and local packs are ignored. |
| openspec/changes/add-vale-binary-packages/.openspec.yaml | Declares the new OpenSpec change metadata. |
| openspec/changes/add-vale-binary-packages/proposal.md | Proposal describing the distribution approach and delivery shape. |
| openspec/changes/add-vale-binary-packages/design.md | Design rationale and detailed decisions for packaging/versioning/trust boundary. |
| openspec/changes/add-vale-binary-packages/tasks.md | Task breakdown for the overall change (unit 1 completed items included). |
| openspec/changes/add-vale-binary-packages/specs/cli/spec.md | CLI requirements for eventually pinning platform packages as optional deps. |
| openspec/changes/add-vale-binary-packages/specs/infrastructure/spec.md | Changesets-ignore requirement for workflow-versioned packages. |
| openspec/changes/add-vale-binary-packages/specs/vale-binary-packages/spec.md | Core requirements for platform packages + checksum-gated publishing flow. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
* origin/main: chore: grow the changeset with the committed sg config docs(openspec): make cli-check internally consistent; correct the generateSgConfig note test(cli): isolate the missing-binary test from the host environment docs(openspec): archive partition-rules-by-engine ref(cli): declare COMMITTED_SG_CONFIG beside the layout it derives from docs(cli): describe the engine-partitioned layout in help and scaffolding feat(cli): run ast-grep against the committed sg config chore: grow the changeset with the runtime discovery path docs(openspec): correct the group-1 note, which no longer describes reality ref(cli): retire layout.ts now that ENGINE_LAYOUTS supersedes it feat(cli): discover runtime rules under the runtime engine directory
* origin/main: fix(cli): size the vitest timeout to what the suite actually does
|
@claude /review |
This comment was marked as outdated.
This comment was marked as outdated.
|
Claude finished @thecodedrift's task in 4m 13s —— View job Review complete
Overall assessmentThis is a clean, scaffolding-only unit: six near-identical workspace packages (
One confirmed issue, posted inline: No other correctness, security, or test-adequacy issues found. I did not re-raise the Not evaluated: CI/build/lint/test results (reported separately by CI on the PR). |
The arm64 README's glibc section was copy-pasted from the x64 one, asserting `interpreter /lib64/ld-linux-x86-64.so.2` — the x86-64 dynamic linker — as a verified fact about the arm64 binary. Checked against the pinned 3.17.1 Linux_arm64 archive (whose sha256 matches the manifest): the real string is `ELF 64-bit LSB executable, ARM aarch64, dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, for GNU/Linux 3.7.0, stripped`. The minimum kernel differs too (3.7.0, not 3.2.0), so both halves of the quoted string were wrong. design.md scoped its verification to x86-64, which is what made the arm64 claim unjustified; it now records both architectures' strings and notes that they differ, so the next README does not inherit the same paste. The x64 README was verified correct byte-for-byte and is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
The one confirmed issue is fixed in — AI Coding Agent |
Stack (root → tip):
Publishes the Vale binary as per-platform npm packages from this repo, so a first-class engine isn't a host prerequisite.
This PR now carries unit 1 of a forward-merging stack: the six
packages/vale-<platform>/workspace packages, the committed checksum manifest that pins what goes into them, and the changesetsignoreentries that keeprelease.ymlout of their versions. It publishes nothing and no consumer references it. Unit 2 (#86) adds the fetch, verify, stamp, and two-phase release workflow. Unit 3, the CLI'soptionalDependenciespin, cannot be written until these names exist on npm.Why binary-in-tarball
The only existing npm distribution,
@vvago/vale, is third-party and downloads at postinstall. That script runs during a consumer's install under a policy we don't set — pnpm 10 blocks dependency build scripts by default — producing no binary and no error. The objection is mechanism, not provenance: it would stand if the Vale project published it. Binary-in-tarball is integrity-hashed, lockfile-pinned, resolves offline, and needs no lifecycle script. Sopackages/vale-<platform>/carries the binary withos/cpudeclared and nobin, no code, no scripts — ast-grep's packaging without ast-grep's installation, whose hardlink step already fails here underpnpm dlx.Versioning
An all-prerelease timestamp,
<valeVersion>-<yyyymmddhhmmss>, with a plain<valeVersion>never published. That keeps the Vale version legible, means a packaging fix is a new timestamp on the same base rather than a spent version, and — because a prerelease only satisfies a range naming the samemajor.minor.patch— makes^3.17.1provably unable to resolve. Exact pinning stops being a convention someone can drift from.Binaries are not committed
Six platforms at 10–20 MB each would live in git history permanently, so a published tarball is not reproducible from a plain clone. SHA256 checksums are committed and reviewed, and the pipeline refuses a mismatch, keeping "what can merge to main" as the trust boundary. The workflow that consumes them (#86) runs in two phases — detect upstream on a schedule and open a PR with the new version and checksums, then publish on merge — so nobody has to notice a Vale release and nothing publishes on bytes nobody signed off on. Safe to automate because publishing is inert: the CLI pins a literal exact version, so a new package reaches nobody until that pin is deliberately bumped.
Resolved open questions
The proposal left three open. All three are answered, and the reasoning is written up in
design.mdunder Resolved Questions.The matrix is six packages, not ast-grep's seven. Vale 3.17.1 publishes exactly six binary assets, and the packages are those six:
darwin-arm64,darwin-x64,linux-arm64,linux-x64,win32-arm64,win32-x64. ast-grep's seventh iswin32-ia32, and Vale ships no 32-bit Windows asset, so there is nothing to package.No libc or toolchain suffix in the names —
vale-linux-x64, notvale-linux-x64-gnu;vale-win32-x64, not-msvc. ast-grep carries those suffixes because Rust target triples disambiguate several builds per platform. Vale publishes exactly one build peros/cpupair, so a suffix would disambiguate nothing while asserting a toolchain nobody verified.musl stays on the
PATHfallback. Upstream publishes no musl asset, so there is nothing to package for Alpine. That is not only a packaging gap: Vale's Linux build is dynamically linked against glibc (verified asdynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0), so it is not a static Go binary and would not run on musl even if it were installed there. The linux packages' READMEs say so plainly rather than leaving a user to discover it as a loader error. This matches the existing gap rather than widening it:findSgBinary()maps every Linux to-gnutoday, so Alpine already falls through for ast-grep.Vale 3.17.1 is the pinned version, recorded in
.github/scripts/vale-manifest.jsonbeside the scripts that consume it. The manifest holds the version once, and per platform the asset-name template, the archive member to unpack, and the SHA256 of the release archive — upstream'svale_<version>_checksums.txtcovers the archives rather than the executables inside them, so a committed digest is independently checkable against upstream and the archive is verified before anything is unpacked from it. Tracking is the detect phase in #86: a weekly schedule opens a PR whenever upstream is ahead, and a security release takes a manualdetectdispatch rather than waiting for the cadence.Which Vale version the CLI pins is a separate decision, made when the CLI's
optionalDependenciesland in unit 3.Also carries two
CLAUDE.mdfixesUnrelated to Vale but too small to spend PRs on:
TSKL-, reading as though it's the only bare identifier the Linear integration resolves. It isn't —OSS-23linked and moved to In Review on PR creation for ref(cli): resolve ast-grep without an install-time step #69.git clone --depth=Nimplies--single-branch, which breaks--force-with-leaseon every branch (it failsstale info, so people fall back to a bare--force) and, more quietly, makesgit rebase maincorrect only while the merge base sits inside the shallow window.Where this sits
This change is the one exception to "one change, one PR": it is stacked, merging forward, with the archive landing on the last unit. The archive gate skips a PR that is not the tip, so
stack: openspec-archivedis not expected on this PR at all — #86 is the tip, and the change is archived on unit 3.ignoreoptionalDependenciespinpartition-rules-by-enginehas landed and is archived, so #71's only remaining prerequisite is a published binary from this stack.#70 and #71 are coupled by exactly one line: whichever lands second adds the engine-selection topic to
TOPICS. Ordering between them doesn't matter.Downstream, the generator's decision router (TSKL-279) needs a published release containing #70 and #71. It consumes a normal release — no prerelease, no path dependency — so it waits without blocking anything here.
skip-changesetstays on this PR. The six packages are in the changesetsignorelist andpackages/cliis untouched, so there is nothing here for changesets to version or release. Unit 3 is where a changeset belongs, since that is where a published CLI actually changes.Fixes OSS-22