Skip to content

fix(install): take the release channel from the build, not the version - #631

Merged
theCodeDrift merged 5 commits into
mainfrom
fix/625-explicit-release-channel
Nov 8, 2026
Merged

theCodeDrift merged 5 commits into
mainfrom
fix/625-explicit-release-channel

Conversation

@theCodeDrift

@theCodeDrift theCodeDrift commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

A self build reports <next>-self. Several places decided which package a recorded version belongs to by whether it contains a dash, so a self install's install.cliVersion: "0.13.0-self" read as a nightly. Afterwards the version gate refused a nightly ("this project runs @taskless/cli-nightly 0.13.0-self") and a release, the launcher cache kept an entry for it, and pin advice suggested @taskless/cli-nightly@0.13.0-self, which was never published.

What changed

  • Channel from the build. install/launcher.ts gains Channel (release | nightly | self) and buildChannel(), read from __TASKLESS_BUILD_TARGET__. buildLauncherPackage() derives from it. install/pinned-cli.ts judges pins and names bump targets by channel instead of isNightly(version), which is deleted. A self build publishes no package, so it reports no command pin to move and names no bump target. The exception is the unreleased warning after a migration, which it keeps.
  • install.launchers is the only record. The version gate, the install carry-forward, the cache prune and the launcher template read launchers alone. cliVersion is display only. recordedLauncherVersion and the legacy classifier are deleted.
  • No includes("-") channel inference remains in install/ or the template. The one place a channel is still read off a version string is the migration below, and it only reads values written before launchers existed.
  • OpenSpec explicit-release-channel and migrate-legacy-cli-version (both archived in this PR) restate the launcher, version-gate, init and pin requirements and add migration 13. I checked every prior scenario is still present after each archive.

Scaffold migration 13

Step 3 of the issue. filesystem/migrations/0013-launchers-from-cli-version.ts moves a version recorded only as install.cliVersion into install.launchers, under the package that published it:

  • no prerelease: @taskless/cli
  • -self: no package, records nothing
  • any other prerelease: @taskless/cli-nightly, which covers published nightlies and build:next (<next>-next-<sha>)

A value that is not a version records nothing. cliVersion is kept, and every other field in install and outside it is preserved. A manifest whose launchers already records a version is left untouched, so the migration is idempotent and a crash replays it as a no-op.

The scaffold version advances to 13, and this repository's own .taskless/taskless.json is migrated. A CLI older than this release refuses a migrated project with the "scaffold newer than this CLI" message. That is accepted.

Launcher template. I removed its fallback too. The template is only written by init, which runs migrations first. Every CLI since launchers existed keeps the field when it rewrites the manifest. So does every CLI before it: writeInstallState has shallow-merged over the raw install object since May. A template therefore only meets a cliVersion-only manifest after a hand edit. In that case it fails plainly and names init, which applies the migration.

Tests

  • migrate-launchers-from-cli-version.test.ts: release, nightly and build:next versions move. -self and non-versions do not. An existing record is left alone. Unknown fields survive. A rerun is byte-identical. A version-12 scaffold reaches 13 through ensureTasklessDirectory.
  • version-gate.test.ts: launchers is the only source. A cliVersion-only manifest (release, nightly, build:next, self) refuses neither channel. Gate fixtures now record launchers.
  • launcher.test.ts: the template never reads cliVersion and fails plainly without a launchers entry.
  • pinned-cli.test.ts: a self build reports no command pin and describePin names no target.
  • Fixtures that hardcoded schema version 12, or seeded a legacy manifest straight into applyInstallPlan, now use LATEST_SCHEMA_VERSION or go through the migration.
  • pnpm typecheck, pnpm lint, and pnpm --filter @taskless/cli test pass locally.

Fixes #625

@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @theCodeDrift's task in 6m 11s —— View job


Review mode: incremental — read 0 prior review thread(s) before reviewing.

.prior-review.json contained no review threads, no review summaries, and no top-level comments other than the trigger comment itself (@claude /review). So there is nothing previously-raised to reconcile — this is the first full review of this PR.

  • Read prior review data
  • Fetch PR diff and metadata
  • Read changed files for context (install/launcher.ts, install/pinned-cli.ts, assets/launcher/launcher.mjs, install/version-gate.ts, install/state.ts, install/install.ts, test files, OpenSpec deltas)
  • Analyze correctness, security, performance, tests, clarity
  • Post findings

Summary

The core fix is sound. buildChannel() reads the channel from __TASKLESS_BUILD_TARGET__ rather than guessing from the version string, legacyChannel()/legacyPackage() apply the identical -self-aware rule in both install/launcher.ts (TS) and assets/launcher/launcher.mjs (the template), and recordedLauncherVersion/versionFor consistently treat install.launchers as the sole source of truth once it holds any entry, falling back to cliVersion only for a pre-launchers legacy manifest. I traced the carry-forward path in applyInstallPlan (install.ts:597-619) for a self build following a legacy release/nightly install, and the cache-pruning path in pruneLauncherCache/prune() — both correctly avoid treating a -self version as belonging to either published package, and correctly preserve a previously-recorded package's version in launchers rather than losing it when cliVersion is overwritten by a self build's version. The version-gate and pin-advice tests cover the scenarios described in the PR body well.

[New] One concrete issue, posted inline on packages/cli/src/install/pinned-cli.ts:619: generalizing channelVerdict's "self" case to behave like "nightly" (both return false unconditionally) makes the "unreleased" PinReason reachable from a self build + migration, per the updated spec text — but the getPinnedCliNotice paragraph for that reason is still hardcoded to say "This nightly migrated .taskless/ from schema version N to M...", which is simply false when the triggering build was a self build. The same nightly-only framing also lingers in two now-stale doc comments (PinReason.unreleased docstring, and the case false comment in scriptPinReason).

[New, minor] Test coverage gap: no test exercises the self-build + migration → "unreleased" combination that the above finding depends on (only the plain "self build moves no pin" case, without a migration, is tested in pinned-cli.test.ts). Given this is a newly-reachable code path, a test would have caught the wording bug above.

Everything else — the VERSION/legacy-channel regex parity between the TS and JS copies, the Object.keys(launchers).length > 0 gating logic and its interaction with state.ts's empty-object normalization, the Channel-typed switch statements (all exhaustive), and the OpenSpec archive's restated requirements/scenarios — checked out consistent with the implementation.

Note: this session's tooling didn't expose the Actions run ID, so I'm omitting the job-run link rather than guess at it.

Comment thread packages/cli/src/install/pinned-cli.ts Resolved
@theCodeDrift

Copy link
Copy Markdown
Member Author

Re: @github-actions — "Claude finished @theCodeDrift's task in 6m 11s…"
#631 (comment)

Both findings are addressed in 5c8dbd9. The self-build wording is fixed in the unreleased notice and its comments, and the coverage gap is closed: self build + migration now has tests for the unreleased reason and for the notice text.

— AI Coding Agent

@theCodeDrift
theCodeDrift force-pushed the fix/625-explicit-release-channel branch from 5c8dbd9 to e7dc416 Compare November 8, 2026 05:50
@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @theCodeDrift's task in 3m 2s —— View job


Review mode: incremental — read 1 prior review thread(s) before reviewing.

  • Read prior review data (.prior-review.json)
  • Fetch PR diff and metadata
  • Read changed files for context
  • Analyze correctness, security, performance, tests, clarity
  • Post findings

Previously raised, now addressed

The one prior inline thread (packages/cli/src/install/pinned-cli.ts:619, about the unreleased notice hardcoding "nightly" even when a self build triggered it) is resolved and carries the *— AI Coding Agent* marker on commit 5c8dbd9. I verified the fix is actually in the current diff: the getPinnedCliNotice paragraph now branches on channel === "self" ("This development build migrated…"), the PinReason.unreleased doc comment and the scriptPinReason case false comment both now say "a nightly or self build," and pinned-cli.test.ts gained "names a self build, not a nightly, when it migrated the project" plus "raises a release script pin as unreleased after a self build migrated the project". Not repeating this.

What's new since that review

The PR grew substantially since the last pass — two more commits added scaffold migration 13 and removed the legacy cliVersion-sniffing read path entirely (recordedLauncherVersion, legacyChannel/isNightly are gone). I reviewed this new material in full:

  • packages/cli/src/filesystem/migrations/0013-launchers-from-cli-version.ts: legacyLauncherPackage correctly special-cases -self (→ no package), exact x.y.z (→ release), everything else including build:next's <next>-next-<sha> (→ nightly). The idempotency guard (skip if install.launchers already has any string value) matches the "once launchers holds any version, it's the whole record" rule used consistently elsewhere (version-gate.ts, pruneLauncherCache). Non-string cliVersion, missing/unparseable manifest, and missing install are all handled without throwing.
  • Ordering guarantee: the migration's correctness depends on applyInstallPlan never running before migrations do. I traced both callers — commands/init.ts's runNonInteractive (ensureTasklessDirectory at line 474, before readInstallState/applyInstallPlan at 491/494) and wizard/index.ts (ensureTasklessDirectory at line 142, before applyInstallPlan at 154) — both migrate first. install.ts's simplified carry-forward (just { ...previousState.launchers }, no more per-package recordedLauncherVersion fallback loop) is safe given that.
  • assets/launcher/launcher.mjs: the hand-maintained JS copy of versionFor was updated in lockstep with the TS version — both now read install.launchers only, with no cliVersion fallback.
  • This repo's own .taskless/taskless.json: confirmed it was actually migrated (version 13, install.launchers: {"@taskless/cli": "0.11.2"} alongside the unchanged cliVersion), matching the PR's claim.
  • Checked for stragglers: no remaining references to recordedLauncherVersion, legacyChannel, or isNightly anywhere in packages/cli/src.

I found nothing NEW to raise — the migration and the legacy-path removal are both correct and consistent with the rest of the codebase's "launchers is the only record" invariant.

@theCodeDrift

Copy link
Copy Markdown
Member Author

Thanks, nothing new to act on from this pass. Rebasing onto the current main next (no new work).

A self build reports <next>-self, which the dash rule read as a nightly,
so after a self install the version gate refused nightly and release
builds, and pin advice named a nightly version that does not exist.

The channel now comes from the build target. install.launchers is the
only record once it holds a version; cliVersion is read only for a
legacy manifest, where a -self version belongs to no package. The
launcher template applies the same rule.
…e fallback

Scaffold migration 13 moves a version recorded only as install.cliVersion
into install.launchers under the package that published it (a -self
version records nothing). The version gate, the install carry-forward,
the cache prune and the launcher template then read install.launchers
alone, so recordedLauncherVersion and legacyChannel are deleted.

Refs #625
@theCodeDrift
theCodeDrift force-pushed the fix/625-explicit-release-channel branch from e7dc416 to 05bc9a6 Compare November 8, 2026 06:10
@theCodeDrift
theCodeDrift merged commit 58e7543 into main Nov 8, 2026
4 checks passed
@theCodeDrift
theCodeDrift deleted the fix/625-explicit-release-channel branch November 8, 2026 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A self build's version is classified as a nightly, so the version gate refuses nightly and prod builds

1 participant