fix: stack exceptions, merge-class content checks, and pnpm import gaps - #1
Merged
Merged
Conversation
ADAPTING.md told an adopter to "record the exception" for an entry that does not fit, without saying where - and the obvious place, this stack's own stack.manifest.json, is exactly the file self-verify.mjs (core repo) never reads exceptions from: it merges files/sections/guards from the stack manifest but not its exceptions array, so a recorded deviation there is silently never checked, forever. Names the actual workaround until that core-repo gap closes: record the exception in standard.manifest.json instead, in the shape self-verify.mjs already reads (kind/match/reason). The merge fix itself is out of scope here - it belongs in the core repo's self-verify.mjs.
…wo silent gaps
ADAPTING.md's only migration move for pnpm-workspace.yaml was "pnpm import
converts the existing lockfile", stated as if it held for any source tool.
Reproduced against two real repos this week: bun fails immediately
(ERR_PNPM_LOCKFILE_NOT_FOUND - bun.lock/bun.lockb are not a supported input
at all), and yarn 4 (Berry) fails outright while packageManager still names
yarn ("This project is configured to use yarn").
Adds the by-source-tool table: npm and yarn classic are the only inputs
pnpm import actually documents; yarn Berry works once packageManager is
dropped, but silently ignores any `resolutions` override (re-create it as
pnpm.overrides by hand); bun and Deno have no importer path at all - delete
the lockfile and run a fresh pnpm install, then hand-repin from the deleted
lockfile since package.json ranges resolve fresh rather than to what the
old tool had pinned. The historical yarn ERR_PNPM_INVALID_OVERRIDE_SELECTOR
crash on a nested-path selector did not reproduce against this stack's
pinned pnpm (11.1.2) in testing - noted as fixed upstream, with the caveat
that hitting it anyway means a different pnpm is the one actually running.
Deno's row is by inspection of pnpm import's supported-input list, not
independently verified against a live Deno repo - stated as such.
…e content Deleting pnpm-workspace.yaml's entire supply-chain policy block (minimumReleaseAge/saveExact/enablePrePostScripts, all becoming undefined) still reported compliant, because stack.manifest.json's file entry for that path only asserts the path exists - a merge-class entry has no content check, by design, and this is the one entry where the merged-in content is the whole point (DECISIONS#8). The starter's own CI already knows how to check these three keys (starter-boot.yml, via pnpm config get) - self-verify did not. Adds scripts/check-pnpm-workspace-policy.mjs: reads the three keys directly off pnpm-workspace.yaml with no YAML dependency (they are always top-level scalars in the reference copy) and no pnpm invocation - so it runs the same whether or not pnpm is even on PATH. Wired in as both a shipped file entry and a guard in stack.manifest.json, and listed in SHIPPED.md's checked table. Tested against the real starter file (passes) and against a stripped and a weakened copy (both fail with the specific key and expectation). Core is independently adding a generic required-keys mechanism for merge-class entries (tracked there, not landed as of this change) - once that ships, this custom guard should fold into it rather than staying a one-off; noted so the two don't drift apart.
…quisite Carrying stack.manifest.json means self-verify's only compliance signal for format/types/lint is shelling out to pnpm check:all. That check genuinely cannot be replaced by reading config files - it runs the actual toolchain - so pnpm on PATH plus a completed pnpm install (the full tree, network, no smaller alternative) is a real prerequisite, not something fixable from this side. What was fixable: with pnpm absent, the guard failed with a bare shell "command not found", indistinguishable in the drift report from a real lint failure on a compliant repo. The guard's run command now checks for pnpm first and says so explicitly before failing. SHIPPED.md documents the prerequisite next to the check it belongs to, and points at core's docs/method/prerequisites.md, which lists every guard's generic needs (Node, git, bash, jq) but had no Node-stack entry for this one. Also lists scripts/check-pnpm-workspace-policy.mjs (added for the supply-chain-policy content check) in SHIPPED.md's own "what ships" table - missed when that guard landed.
core's self-verify.mjs now checks specific keys inside a merge-class file directly (repository-standards/core PR #16), the same mechanism this stack's custom scripts/check-pnpm-workspace-policy.mjs was built to work around before that landed. Declaring requiredKeys on the pnpm-workspace.yaml manifest entry does the same check with one fewer file to ship and maintain, and the manifest-side workaround note in ADAPTING.md for the exceptions-merge gap (repository-standards/core PR #17) is no longer needed either - a stack-level exception is honoured directly now. Verified end to end against the real starter file: the three keys pass when present and each fails by name, individually, when removed.
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
Four findings from this week's validation round, all reproduced against the real starter:
stack.manifest.jsonhad no effect -self-verify.mjs(core) merged this stack'sfiles/sections/guardsinto the check it runs, but neverexceptions, so a deviation recorded exactly whereADAPTING.mdtold an adopter to record it was silently discarded. Fixed on the core side (repository-standards/corePR #17);ADAPTING.mdhere now points at the real, working mechanism.minimumReleaseAge/saveExact/enablePrePostScripts) still reported compliant. Originally worked around with a custom guard script; once core shipped a genericrequiredKeysmechanism for exactly this (repository-standards/corePR #16), switched to declaring the three keys directly on the manifest entry and deleted the now-redundant custom script. Verified end to end: passes when the keys are present, fails by name individually when any is stripped.pnpmon PATH read as an opaque lint failure - thestack-check-allguard now checks forpnpmfirst and fails with an explicit prerequisite message.ADAPTING.md's only documented migration path (pnpm import) breaks on bun and yarn 4 - reproduced both failures in scratch projects; added the real working paths (delete-and-reinstall for bun, droppingpackageManagerfor yarn) plus a documented gap (yarn'sresolutionsfield is silently ignored, not translated topnpm.overrides).Test plan
pnpm check:all,pnpm build,pnpm test:uniton the starter - all pass.requiredKeyssubstitution against core's shippedself-verify.mjs: three keys checked and passing on the real starter file, each failing individually by name when stripped.