chore: back-merge main into develop - #4
Open
github-actions[bot] wants to merge 42 commits into
Open
Conversation
`ailoud setup` refused the sudo step, which is what it is supposed to do: with no terminal to answer a password prompt on it reports the exact command rather than hanging. A runner has no terminal, so the apt install is the workflow's job and only the rest -- the whisper release and the model files, neither needing sudo -- is left to setup. Also makes the back-merge workflow say so when there is no develop branch, instead of dying on `fatal: Not a valid object name origin/develop`. That is how its first run failed, and the copy it came from has the same weakness.
…ck-merge PR Two separate failures, both in what I added rather than in the product. The provisioned e2e job had every doctor check green and every transcribe spec exiting 3. The sandbox writes its own config naming only the MODEL and leaves `binary` at its default, so whisper-cli has to be on PATH -- while `setup` installs it under the data directory and records the absolute path in the user's config. Nothing was wrong with provisioning; the two configs simply disagreed about how the binary is found. The job now reads the installed paths back out of that config and adds their directories to PATH, read rather than spelled so bumping a pinned release cannot silently break it. The back-merge workflow could not open its PR: "GitHub Actions is not permitted to create or approve pull requests", a repository setting that is off by default. Enabled with default_workflow_permissions still `read`, so the token gains nothing beyond what this needs. Also adds the author (contact@lorem.dev) to all four manifests, the dev-tag skill and its spec, the branch and tag rules in AGENTS.md, and a publish-time check that every tarball carries the LICENSE and no source or tests -- pnpm copies the repository LICENSE into each workspace tarball, which is why no package holds its own copy, and an invariant worth relying on is worth checking. The `.agents/` skills were also missed by the rename and still said "laud".
The provisioned end-to-end job runs only on push, so requiring it on a branch would leave every pull request waiting for a check that never arrives.
Every transcribe spec on CI failed with "Exceeded timeout of 5000 ms" -- Jest's default, not the 600 s the config asks for. Jest takes the per-test timeout from the GLOBAL config. Splitting jest.config into `projects` moved `testTimeout` into each project, where it appears in `configs[].testTimeout` while `globalConfig.testTimeout` stays undefined, so every test fell back to 5 s. Confirmed both ways with `jest --showConfig`: projects 600000, global undefined; with the value also at the root, global reads 600000. Locally this was invisible. The transcribe specs were already failing fast for a missing model -- mine are still under the pre-rename path -- so nothing ran long enough to hit a 5 s limit. On CI, where setup had just downloaded them, whisper takes tens of seconds and every spec timed out. The bug was mine and the runner was the only place it could be seen. Separately, the build's own config files were in eslint's ignore list, so every editor reported "File ignored because of a matching ignore pattern" on opening one, and a mistake in them was caught by nothing -- including the mistake above. They are linted now, with Node globals and the recommended rules rather than the type-aware config the packages use. Verified by planting an unused variable in jest.config.cjs and watching eslint catch it.
Cutting 1.0.0 after 1.0.0-dev.1, -dev.2 and -rc.1 left four changelog sections describing one release. `fold-prereleases.mjs` merges them into one, keeping the subsection grouping and dropping duplicates -- including a duplicate whose copies were wrapped differently, which is the common case across two dev tags. Only the same version's pre-releases fold, so an entry from an abandoned line cannot reappear under a release it was never part of. `check-changelog.mjs` is a required step on every tag, before the gate and before anything is built: a version number can never be reused and the unpublish window is 72 hours, so every reason to refuse is worth finding while refusing is free. It checks the section exists and has entries, is inside the hard limit, that nothing is stranded under Development, and that a final tag folded its pre-releases. Problems are collected and reported together, because somebody fixing a changelog wants the list rather than one round trip each. The limits were copied into three scripts, so they now live once in scripts/lib/changelog.mjs alongside the parsing all three need. A limit that differs between the script that warns and the script that refuses is worse than no limit: one of them is wrong and nobody knows which. The soft limit is a warning rather than an error -- console.warn locally, a GitHub annotation under Actions. It read as a failure before. The scripts have tests now: 20 over the shared library, and 22 driving all three end to end. Two of them WRITE, so the tests copy scripts/ into a throwaway directory beside a fixture CHANGES.md, and each asserts it is under the temp directory before running anything. One test reads the repository's own changelog afterwards and fails if it changed -- if a script ever resolves the wrong root, that is how it will be caught. They run in ci.yml, which triggers on branches and pull requests but not on tags, so a broken script is caught before a release depends on it. Also lints scripts/ and the build's config files, which were both in eslint's ignore list: every editor reported "File ignored because of a matching ignore pattern" on opening one, and `node --check` was the only gate on scripts/ -- which sees syntax, not an unused variable. Found while writing the tests: execFileSync discards stderr on success, so the soft-limit test could never have seen the warning it was asserting. spawnSync returns both streams.
Cutting a final tag leaves its `-dev.N` snapshots behind: still installable, still holding the `dev` dist-tag, still tagged. `scripts/retire-prereleases.mjs` clears them in one step, printing the plan and changing nothing without --yes. It deprecates rather than unpublishes -- npm allows unpublishing for 72 hours, the version number can never be reused after, and anyone who pinned it has their install broken. A deprecated version keeps working and says why. It deletes a tag only when the tag's commit is reachable from origin/main. The provenance of a published package names both the commit and the tag: losing the name costs convenience, but deleting a tag that holds the only reference to its commit lets the commit be collected, which costs the attestation its subject. The decision of which tags those are is a pure function in the shared lib, so it is tested without a repository; the CLI is tested against a throwaway one with a tag on each side of the line. Splitting the script tests one module per script also turned the single end-of-file "the real changelog is untouched" test into an afterEach that compares its bytes after every test, and catches a stray RELEASE_NOTES.md too -- verified by planting both.
…pm download Two failures on the runner that no local run could produce. The scripts read $GITHUB_REF_NAME as a tag fallback and print warnings as ::warning:: annotations on stdout when $GITHUB_ACTIONS is set. Both leaked into the tests through the inherited environment, so `check-changelog` with no argument found the tag `main` instead of failing, and the warning the retire test looked for on stderr had gone to stdout. The harness now scrubs every GITHUB_ variable, and tests that want that behaviour pass it explicitly -- which also gets both warning channels covered for the first time. Verified by running the suite with the runner's variables set: two failures before, none after. Separately, `corepack enable` only writes shims, leaving the pinned pnpm to be downloaded by whoever invokes it first -- setup-node's cache probe, where the download crashed on an undici assertion inside Node 24.20.0 and nothing could retry it. The setup is now one composite action, shared by all four jobs, that downloads pnpm in a step of our own and retries it three times.
… skipping Chasing the resolver's "multiple projects" warning turned up something worse behind it: `boundaries/dependencies` never saw a cross-package import written as a package name. `@ailoud/providers` resolves through node_modules to `packages/providers/dist/index.js`, which matched no element pattern, so the rule classified the target as unknown and reported nothing -- while the same import written as `../../providers/src/index.js` was caught. Listing dist under the same type as src makes both forms the same violation. Verified by planting each one in packages/core: the package-name form was accepted before this change and is an error after it, the path form and the node:fs restriction still bite, and a clean tree lints silently. The resolver now reads the root tsconfig, which includes every package's sources, instead of a glob over the per-package ones -- one project rather than four, which is what it was asking for.
The v1.0.0-dev.1 release failed at "Run the gate", not at anything about releasing: the gate includes packages/providers/src/audio/ffmpeg.test.ts, which spawns the real binary, and this job never installed it. CI installs it in both jobs that run tests for exactly this reason, and the publish job runs the same gate.
Every `pnpm test` printed "error: unknown option '--bogus'" twice, a full usage block, and one ExperimentalWarning per worker -- so a real error had to be picked out of noise the tests produce on purpose. Commander writes usage errors and no-argument help to stderr, which is right for a CLI. The four tests that provoke it now silence writeErr only; writeOut stays as buildProgram set it, because that is how help reaches context.write and one of the tests asserts on it. The SQLite notice is the flag the installed binary already carries in its shebang: node:sqlite is experimental and this project knowingly depends on it. Set through NODE_OPTIONS rather than the pool's execArgv -- that route works, but switching the pool to forks took doctor.test.ts from 3 seconds to a 225-second timeout.
`npm publish dist-npm/ailoud-core-1.0.0-dev.1.tgz` never looked at the file: npm parsed the slash as the `owner/repo` GitHub shorthand and ran `git ls-remote ssh://git@github.com/dist-npm/ailoud-core-1.0.0-dev.1.tgz.git`, which failed on a missing public key -- an error about ssh keys in a step that has no business talking to git. An absolute path cannot be read that way. The `ls` that found the file is gone too: it existed to expand a name that was never a glob, and it turned a missing tarball into a confusing npm error instead of saying which one was missing.
`ailoud audio import` takes video because a meeting recording usually is one, and domain/mime.ts maps four containers -- but nothing exercised any of them. The audio path was tested against a tone generated at run time; video needs a real container, so scripts/make-fixtures.mjs now wraps en-short.wav in each of the four and the fixtures are committed, through LFS like the audio. They are deliberately dull: 32x32 of black at 5 fps, which keeps each file under 14 kB while still being a real, decodable video stream. Each container gets the codec pair it carries in the wild, and one test per container asserts what actually matters -- that whatever went in, the 16 kHz mono WAV whisper.cpp needs comes out. The video length comes from the audio rather than from `-shortest`, which produced an 18-second mp4 from a 2.5-second source.
…to a branch Re-running the publish workflow on v1.0.0-dev.1 failed with `no "## Version main" section`. The changelog check was the one step that passed no tag and let the script fall back to $GITHUB_REF_NAME -- which is the tag on a tag push but the BRANCH on a workflow_dispatch, so a manual re-run of a good tag checked the wrong version and could never pass. The tag is now resolved once into a job-level TAG and used by all three steps that need it, so there is no second answer to which tag is being released.
Publishing failed with `404 PUT https://registry.npmjs.org/@ailoud%2fcore` even after the organization existed, and the log never mentioned trusted publishing -- because it never happened. setup-node's registry-url writes an .npmrc holding `_authToken=${NODE_AUTH_TOKEN}`; this workflow has no token secret by design, so npm found a credential, sent an empty one, and had no reason to reach for OIDC. The registry answered as it would to any stranger. Without that .npmrc npm sees it has nothing, and does the exchange the `id-token: write` permission is there for. The default registry is registry.npmjs.org regardless, so nothing else changes -- and the `Unknown user config "always-auth"` warning was setup-node's file too.
…t can work npm answers ENEEDAUTH for @ailoud/core however complete the OIDC setup is -- npm 11.19.0, id-token: write, no stale .npmrc -- because a trusted publisher is attached to a package on npmjs.com and there is no page to attach it to until the package exists. The first version of each of the three has to go out on a credential; nothing about the workflow was wrong. So NPM_TOKEN is used when the secret is present and ignored when it is not, which makes removing the secret the whole of the switch to trusted publishing. Provenance is attached either way. The token is a bootstrap, not a fixture: once all three package pages have the publisher attached, deleting the secret leaves the arrangement with nothing to expire, which was the point of using OIDC in the first place.
Four places said only a final tag moves `latest`, without qualification. The first release proved otherwise: npm set `latest` to 1.0.0-dev.1 on all three packages, because it does that on a package's first publish whatever `--tag` says, and `latest` can be moved but never removed. So `npm install ailoud` returns the snapshot until 1.0.0 exists. Nothing in the workflow can prevent it, which is exactly why it belongs in the documentation rather than in a check.
All three npm pages read "This package does not have a README" after 1.0.0-dev.1 -- the first thing anyone arriving from a search saw. npm shows the README from inside the tarball, and no package directory had one. The two libraries get their own, short and specific: what the package is, how it relates to the other two, and that its interfaces are not stable and there is no reason to depend on it directly. The CLI's README is the repository's, so rather than keep a second copy in git that would drift, its prepack script copies the root file in at pack time -- verified identical in the packed tarball -- and the copy is gitignored. The packing guard now fails a release whose tarball has no README, beside the existing licence and no-source checks: the CLI's copy is a script that could stop working quietly, which is exactly the kind of thing that guard is for.
`ailoud --version` on the published 1.0.0-dev.1 answered `0.0.0`, and the MCP server told every client the same, because buildProgram passed commander a hardcoded string. Found by installing the release from the registry rather than by reading the code -- nothing in the repository disagreed with itself. The version now comes from the package's own manifest, resolved relative to the module so `dist/version.js` finds it one level up both here and in an installed package. The manifest is the one copy a release already updates and it ships inside the tarball, so it cannot go stale. The test asserts agreement with the manifest rather than a literal, so it needs no edit per release -- an edit per release is what would rot it into agreeing with whatever is there. Checked both ways: it fails against the old hardcoded value, and the packed tarballs installed into a scratch project report 1.0.0-dev.2.
The token exists to introduce each package to the registry, because a trusted publisher is attached to a package that already exists and there is no page to attach it to before the first publish. That bootstrap is over the moment the three packages exist -- but a token that keeps working is a token nobody gets round to removing, and the whole point of OIDC was having nothing stored. So the rule is enforced rather than remembered: a pre-release published on the secret logs a warning, and a final release refuses before anything is published and names the two steps that clear it. Publishing with no secret goes through OIDC exactly as before. Simulated all three paths -- pre-release with the token, final with the token, final without -- because the first version of this check read $version above the line that assigns it, which under `set -u` would have failed every release rather than only the ones it means to.
1.0.0-dev.2 cannot be republished -- npm's policy is that a version number is never reused, even after an unpublish -- so verifying that a release needs no stored credential takes a new number. Saying so in the changelog beats leaving someone to wonder what changed between two snapshots that are the same code.
…red token Deprecating the snapshots a release supersedes was a manual step because trusted publishing is defined for publishing: `npm deprecate` in the same job has nothing to authenticate with, and I was not willing to guess otherwise halfway through a release. Reading npm's own lib/utils/oidc.js settles it. Publishing gets its credential by exchanging the CI identity for a per-package token -- a GitHub id token with audience `npm:registry.npmjs.org`, posted to `/-/npm/v1/oidc/token/exchange/package/<name>` -- and the exchange is an ordinary request anything can make. So the script makes it, and retiring a release needs no more stored credential than publishing one. The token never reaches a command line or a log: it goes into a temporary 0600 npmrc that is removed in a finally, which the tests check both ways. `retire.yml` carries this, called by publish.yml after a final release and dispatchable alone -- without `confirm` it exchanges a token, uses it for nothing, and reports whether it worked, so the credential path can be checked without waiting for a release to find out.
A dispatchable retirement was two mistakes at once. Retiring snapshots means nothing unless something supersedes them, and the standalone run could not authenticate anyway: npm binds a trusted publisher to a workflow file, so a run entered through retire.yml is a different identity from one entered through publish.yml and the exchange is refused with `OIDC token exchange error - package not found`. The dry run proved that before a release depended on it. So retire.yml is workflow_call only, reached by publish.yml for a final tag, and its `confirm` input is gone -- it could only ever have been true, and a knob with one reachable value describes a choice that is not there. The script keeps its plan-first default for the laptop, where nothing has been decided. It runs after the publish rather than before. Deprecating the snapshots first would, if the publish then failed, leave every -dev.N pointing at a release that does not exist while `dev` is the only thing installable.
The release rules had grown by accretion: credentials were explained in two places, the retirement rules were spread over four paragraphs written as each came up, and a sentence about the changelog check sat at the end of a section about tags. Anyone reading it would have had to assemble the rules themselves. Now "Branches and Tags" covers branches, tags and the changelog fold, and a "Publishing" section covers the rest: the OIDC exchange with the two calls verbatim, the three consequences that constrain anyone changing it, the bootstrap exception with the token as a table, retirement, and the npm facts none of it can work around -- a version number used up forever, `latest` set on first publish and never removable, and trusted publishing covering `npm publish` and nothing else. Every claim states what it costs to get wrong, because that is what makes a rule followed rather than looked up: why the entry workflow cannot be retire.yml, why deprecating happens after the publish and not before, why the token goes to a file and not a command line.
A new `check-dependencies` skill: advisories first, funding second, updates last -- because the first two decide what an update is for. `pnpm audit` and `pnpm audit --prod` are read separately, since a high-severity advisory in a test runner cannot reach a user and one in `commander` is on their machine. The 14-day rule is a script rather than advice, because advice is what gets skipped at the moment it matters. `scripts/check-dependency-age.mjs` refuses any pinned direct dependency published less than 14 days ago: a compromised release is found by other people and that takes days, and there is no urgency in a patch that has been out two weeks that was not there on day one. The rule yields to a critical advisory -- two weeks with a known exploit is worse than a version nobody has audited yet -- through `scripts/dependency-age-exceptions.json`, where an exemption carries its advisory ID. That makes it a decision in the repository rather than an argument someone remembers to pass, and the check reports entries that have aged out so the file does not accumulate permanent holes. It runs second in `pre-release-check`, before the tests: an update it recommends changes what everything below it is testing.
Dependabot with `cooldown: default-days: 14`. Without it Dependabot opens a pull request the moment a version appears -- exactly the window check-dependency-age exists to refuse -- and the check would then fail on Dependabot's own branch, leaving the two arguing on every update. Security updates ignore cooldown, which is the behaviour we want and the same exception the age check records for a human: a known advisory beats an unaudited release. `versioning-strategy: increase` because exact pins are the convention here; `widen` would turn a pin into a range and hand the choice of version to whatever resolved last, which no age check can judge. The dev toolchain arrives as one grouped pull request since it cannot reach a user, while anything that ships gets its own. CodeQL is committed rather than enabled through the repository's default setup, for the reason every other check here is a file: what runs, when, and over what belongs in a diff. Weekly as well as per-push, because most findings arrive when the queries improve, not when the code changes.
…acktrack
CodeQL's first run found ten things; seven were real and this is them.
Two places decided whether an endpoint is a hosted API by substring:
`baseUrl.startsWith('https://api.openai.com')` and
`/api\.(openai|anthropic)\.com/.test(baseUrl)`. Both answer yes for
`https://api.openai.com.example.net/v1`, where the part of a hostname that
decides where the request goes is the end of it, and yes again for
`https://example.net/?upstream=api.openai.com`. They now share `isHostedLlm`,
which parses the URL and compares the hostname exactly.
Four `replace(/\/+$/, '')` calls stripped trailing slashes with a pattern that
backtracks; on a value that is mostly slashes that is a denial of service, and
the value comes from configuration. `withoutTrailingSlashes` is a loop, which
is what the operation always was. Two of the four CodeQL did not flag -- same
defect, below its threshold.
`escapePackageName` used `replace('/', '%2f')`, which substitutes only the
first match. A package name holds at most one slash, so it was right by
accident rather than by what it said; `replaceAll` says it. The age check had
its own copy of the same line and now imports the one function.
The stale-lock takeover in setupLock had a real window: two runs can find the
same stale lock, both remove it, and the loser's `open(path, 'wx')` failed with
a raw EEXIST about a path the user has never heard of. It now refuses the way
every other contended case does. Not covered by a test -- reproducing it needs
two interleaved processes -- so it is one branch converting one error code.
Dependabot's first run proposed TypeScript 7.0.2, and lint failed outright: "typescript-eslint does not support TS 7.0. Please see ... to run typescript-eslint using the TS 6 API." The bump is blocked by a peer, not by anything here, and left alone it would return every Monday with the same failure. Ignored for majors only, with the tracking issue named, so patches and minors keep arriving. Drop the entry when typescript-eslint supports TS >= 7.1.
CodeQL flagged this again after the last fix, and was right to. The retry I added only handled the case where the file still existed at the second `open`, which is not the dangerous one. The real sequence: we read a stale holder, another run takes the same stale lock and becomes a LIVE holder, and our unconditional `rm` then deletes ITS lock and we create our own. Both runs proceed -- the one outcome this file exists to prevent -- and the retry could never see it, because after the `rm` our `open` always succeeded. Takeover now writes the lock to a scratch path beside the target and renames over it. Rename is atomic and overwrites, so two takeovers both succeed at renaming, but only one of them is in the file afterwards; reading it back and finding another pid is how the loser learns it lost, and it refuses like every other contended case. The losing branch has no unit test -- reproducing it needs two interleaved processes -- so the tests cover what can be checked: the winner is recorded as the holder, and the scratch file is gone whether the rename worked or threw.
…#3) Bumps the actions group with 2 updates in the / directory: [actions/checkout](https://github.com/actions/checkout) and [actions/cache](https://github.com/actions/cache). Updates `actions/checkout` from 5 to 7 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v5...v7) Updates `actions/cache` from 4 to 6 - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@v4...v6) --- updated-dependencies: - dependency-name: actions/cache dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Taken from the npm registry rather than from GitHub releases, which is where skillkeeper's comes from: this repository has no releases at all, so that badge would render an error. npm is also where the README tells people to install from, so the number in the badge is the number they would get. It reads v1.0.0-dev.1 today, because npm set `latest` on the first publish of each package whatever `--tag` said and `latest` cannot be removed, only moved. Publishing 1.0.0 moves it.
The dependency half of this was already enforced -- a --prod advisory or a version younger than 14 days blocks a release -- while what CodeQL finds in our own code reached the release path not at all. Today's first scan found seven real defects, including one that let two provisioning runs proceed at once, so that asymmetry was not theoretical. Only `state=open` counts. A finding that has been reviewed is `dismissed` with its reason attached and does not block; fixing it and dismissing it are both answers, and ignoring it is the one that is not. Three of today's ten are dismissed with reasons and correctly do not register. It runs before the manifest check, the gate and the pack, so a release that will be refused is refused in seconds rather than after publishing two of three packages. A failure to READ the results also refuses: a release that cannot tell whether it ships a known finding is not one to make blind. Verified against the live repository (0 open high/critical, 3 dismissed not counted) and both branches of the shell exercised on fixture JSON.
Three entries out, on the changelog's own rules rather than on taste. The dev.3 entry began "Nothing for users" -- which is the rule for what stays out, quoted back at itself. It described why a snapshot existed, not anything anyone could do with it. The two "Fixed" entries described a missing README on npm and `--version` answering 0.0.0. Both were real, both were fixed, and both existed only in 1.0.0-dev.* snapshots. The rule is explicit that "before the first release, that is every fix", and 1.0.0 is the first release -- someone reading its notes is deciding whether to start using this, not whether to move off a snapshot. Judgement call: if you would rather dev users see them, they go back. The summarize command and the four engines behind it were two entries about one feature, which the rules say to merge; now one. Folding 1.0.0 in a sandbox yields 14 entries and passes, warning on the soft limit of 10 -- defensible for the release that introduces the whole tool.
…unbuilt
AGENTS.md's overview described an early milestone: it listed `import`,
`transcribe`, `ls`, `show`, `doctor` as the surface and said `search`,
`summarize` and the rest "do not exist yet; do not document or assume commands
beyond what M1 lists" -- while instructing the reader to treat that file as the
authority. Both `search` and `summarize` shipped long ago, and are published on
npm. Two independent reviews found this, and the binary settles it: `audio
search --help` and `audio summarize --help` both answer.
That combination is worse than a stale sentence. An agent following it would
refuse to document working commands and would take the absence as fact.
Replaced with the commands as they are, noun by noun, plus the instruction that
matters more than the list: check the binary, which cannot be stale. CLAUDE.md
said the same thing in its own words ("in a later milestone, will summarize")
and now agrees.
Also: docs/mcp.md carried its "your own files are safe" list twice in two
wordings, a copy-paste that survived review; one copy left. And
docs/usage/cli.md's list of top-level spellings omitted `search` -- checked
each of the eight against the built binary rather than trusting either list.
…failing open Five reviews of the 1.0.0 candidate; these are the confirmed defects. THE LOCK, third attempt and this time measured. Rename-then-read-back does not exclude anything: the interleaving A.rename, A.read, B.rename, B.read leaves each run reading its own pid, and both enter the body. A review demonstrated 34 overlaps in 60 runs against the compiled code. Exclusion now sits on the takeover itself, through a second lock created with `wx` -- one syscall, one winner, and the loser told to try again rather than left to guess. The re-create still uses `wx`, so a run that took the lock on the fast path in between wins it legitimately. Release only removes a lock that is still ours; removing it unconditionally let a third run in while a legitimate successor was working. There is now a concurrency test, and it fails on the previous implementation at round one with exactly that ENTER/ENTER trace. RETIREMENT no longer destroys what it could not replace. Every npm failure was a `::warning::`, which fails nothing, and the tag deletion ran regardless -- so a refused credential deleted the tags, deprecated nothing, left `dev` pointing at a snapshot, and reported success. Worse, the tags are what name the pre-releases, so a re-run found nothing to retire and could never repair it. Failures are collected now, and any of them keeps the tags. Deletion also goes to origin before local, because the reverse left the tag on origin with nothing locally to retry it by. The `dist-tag rm` had skipped the credential guard the deprecations use. THE CODE SCANNING GATE could pass on an empty answer: `jq length` printed nothing, `[ "" -gt 0 ]` errored, and a failing `if` condition is not caught by `set -e`, so the else branch announced no alerts. It now refuses a count it cannot read, and paginates -- an alert past the first hundred blocked nothing. THE TOKEN REFUSAL for a final release ran after twenty minutes of gate; it is now the first step, which is what the documentation already claimed. Tarball existence and a credential for all three packages are checked before the first publish, not per iteration: the loop publishes library, library, CLI, and a spent version number cannot be reused. Also: the age check reported `undefined@undefined` for an unpinned dependency, and treated a 429 or 502 from the registry as "unknown", which turned the 14-day rule into a no-op reporting success -- it fails closed now, and a 404 stays the one answer that means "no such package". The OIDC helper documents returning null rather than throwing, which did not hold for a transport failure, i.e. exactly the case the ambient-login fallback exists for. And retire.yml interpolated a tag name straight into a shell in a job holding contents: write.
… on fresh deps Three loose ends from the release review. DOCS no longer race the publish. Both workflows started on the tag push, so a publish that refused -- or failed partway through its three packages -- left the site advertising a version npm did not have. docs.yml now runs on `workflow_run` of Publish, checks out the SHA that was published, and takes the version from that commit's manifest rather than from `head_branch`: on a workflow_run that field is documented as a branch, and a release should not rest on what it happens to hold for a tag. publish.yml already refuses a tag that disagrees with any manifest, so the number is the same one either way. THE GITHUB RELEASE is created after the packages are on the registry, with the body extracted from CHANGES.md by the script that already existed for it, and `--prerelease` for a `-` version. Through `gh` rather than a third-party action: adding one to the release path would sit oddly beside a 14-day rule for everything else we depend on. Re-running a tag edits the notes instead of failing on an existing release. THE 14-DAY RULE now runs in CI, as a warning. Dependabot's version updates already respect the same window through `cooldown`, so the two do not argue -- but a security update deliberately ignores it, and a fresh version that closes an advisory is the one case where taking it beats waiting. Failing the build there would make the rule an obstacle to the thing it protects.
…ng lock js/file-system-race has fired three times, always on setupLock.ts, and its latest report blocked the back-merge PR -- the check is required now, so a finding on a file it cannot reason about stops merges. Two of the three were real and both are fixed: the first implementation deleted a live lock, the second let two runs each read back their own pid and proceed. What remains is genuinely a false positive. The analysis sees a read of a path followed by a write to it; it cannot see that the exclusion is held by a second lock file (`provisioning.lock.steal`, created with `wx`), because that is a different path. Excluded by id in .github/codeql/codeql-config.yml rather than dismissed alert by alert, so the next report does not block a merge again, with the reasoning in the config and at the site. What stands in for the rule is the two-process test that runs against the compiled lock and fails on either old implementation -- a measurement rather than a pattern match. The query set moves into the config too, so each choice sits next to its reason.
Also corrects the documented order of the two changelog steps, which was the wrong way round and would have failed this very release. `bump-version` promotes `## Development` into a `## Version <v>` section, so folding the pre-releases first leaves it an empty one to promote and produces two `## Version 1.0.0` headings -- `check-changelog` then reports "has no entries" against the empty one. Tried both orders in a sandbox; AGENTS.md and docs/development/releasing.md now say bump first, and say why. The release steps in the docs were also stale in two smaller ways: they still told the reader to extract RELEASE_NOTES.md by hand, which publish.yml now does, and their numbering had two step 3s.
The 1.0.0 release settled this. The exchange works and returned a token; npm then refused it: `E404 ... or you do not have permission` on the first call and `E401 ... token is invalid` on every one after. It is publish-scoped and spent. Trusted publishing authenticates `npm publish` and nothing else -- which is what AGENTS.md said before I read npm's oidc.js, found the exchange endpoint, and talked myself out of it. So retire.yml is gone, publish.yml no longer calls it, and the script runs from a terminal under `npm login`. What the attempt leaves behind is worth keeping: npmOidc.mjs, which preflight-npm-auth.mjs uses to establish before publishing that npm will accept all three packages -- a question the exchange CAN answer. Two things the release proved that the docs had as guesses: npm matches `workflow_ref` (the entry workflow) and not `job_workflow_ref`, so a reusable workflow is not the problem; and the token's scope is. The earlier fix held, which is the point of it: the npm side failed, so the script kept the tags and exited non-zero instead of deleting the only record of which snapshots still need retiring. Its release ran green and shipped everything; only the retirement is outstanding.
Four faults, all found by using the thing. NPM ASKED FOR A CODE ON EVERY WRITE. With 2FA on writes -- the default -- that is twelve prompts for one retirement, and without a usable credential npm drops into an interactive web login and waits, so the script looked hung. NPM_TOKEN is now required for `--yes` rather than a preference: one granular token, one temporary 0600 npmrc for the whole run, and a clear refusal up front instead of a prompt or a stall. npm also runs with stdin closed, so a prompt can only ever become an error. Planning still needs no credential. THE VERSION LIST NOW COMES FROM THE REGISTRY, not from git tags. The tags were the list, which made them the only record of what still needed retiring -- and today they were deleted while the npm side had not run. Asking the registry also makes a re-run safe: an already-deprecated version is left alone, and a `dev` tag already on the release is not touched. THE `dev` DIST-TAG MOVES ONTO THE RELEASE instead of being removed. `npm dist-tag rm` makes `install <pkg>@dev` fail outright, which is exactly what happened to `ailoud` before this. A missing `dev` counts as work too, so the script repairs that case rather than calling it done. AND THE TESTS HUNG THE SUITE. The stub registry was an HTTP server; a test that threw before closing it left a live handle, and the run hung with no failing test to show why -- a per-test timeout does not apply to an open handle. It is a JSON fixture file now: no port, no handle, nothing to leak. Also `pnpm retire`, so the command is discoverable, and every place that documents it says the token is required and the step is manual.
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.
Automated back-merge of
mainintodevelop.Latest commit on main:
0387e9b- fix: make retiring a release one authentication, and stop it hangingTriggered by push to
mainin workflowBack-merge main -> developrun33977145898.Merge this PR (or enable auto-merge on it) to keep
developin sync with the latest release commit onmain. Resolve any conflicts manually before merging.