Skip to content

fix(start-cli): say when the workspace's start-technologies checkout is off live-docs - #3910

Open
MattDHill wants to merge 1 commit into
masterfrom
fix/s9pk-init-guide-branch
Open

fix(start-cli): say when the workspace's start-technologies checkout is off live-docs#3910
MattDHill wants to merge 1 commit into
masterfrom
fix/s9pk-init-guide-branch

Conversation

@MattDHill

@MattDHill MattDHill commented Sep 6, 2026

Copy link
Copy Markdown
Member

Summary

s9pk init-package scaffolds from the package template in whatever branch the workspace's start-technologies checkout is on, and never looks at that branch. A workspace scaffolded by start-cli 1.x cloned master, whose template has pinned @start9labs/start-sdk@3.0.0 since #3900 — a version npm does not have — so npm install fails with ETARGET and nothing names the cause:

$ start-cli s9pk init-package "Hello World"
Installing dependencies (npm install)…
npm error notarget No matching version found for @start9labs/start-sdk@3.0.0.

start-cli 2.0.0 clones live-docs for new workspaces and documents a manual git checkout live-docs for existing ones, which a 1.0.1 workspace cannot even run (its clone is single-branch, shallow, and sparse).

Diagnosis, no git

init-package — right after workspace discovery, before the package-name and destination checks, so the scaffold directory a failed npm install left behind cannot hide it — and init-workspace, when it finds an existing checkout, print one line when start-technologies is on a branch other than live-docs. It names the checkout's path and branch and says what to do:

  • A checkout the workspace owns: move it out of the workspace, then run start-cli s9pk init-workspace '<workspace>', which clones a fresh checkout on live-docs. Moving rather than deleting keeps any local work; the discovered workspace path is single-quoted, and when it is not valid UTF-8 the notice says to run init-workspace from the workspace root instead of printing a path.
  • A symlinked checkout (the "already have the monorepo" layout): the notice names the link and its target and says to remove the link and run the same command, so the workspace gets a checkout of its own and the owner's branch is never touched.

Nothing prescribes or performs a git operation. No single command sequence is right for every checkout state (single-branch refspecs, shallow and sparse clones, stale or diverged local branches, dirty trees, forks), and a fresh blobless clone costs seconds.

Detection

The branch is read from start-technologies/.git/HEAD, following a gitdir: file for a linked worktree or submodule. No git process runs, so nothing walks up into an enclosing repository, no path passes through a subprocess's stdout, and a detached HEAD (no ref: line) says nothing. A missing checkout says nothing either; init-workspace clones it and init-package reports the missing template as before.

Also

  • The notice that start-cli is behind the published release is given only when the checkout is on live-docs. The gate lives inside warn_if_start_cli_outdated, so CliContext::build_key() (e.g. s9pk pack) is covered too; on any other branch the manifest names an unreleased version.
  • Unit tests in the module cover reading a plain .git/HEAD, following a gitdir: file by absolute and by relative path, a detached HEAD, a missing checkout, and the printed command's quoting and UTF-8 gate. They need no git binary.
  • Strings in all five locales; the start-cli changelog under ## [2.0.1] with the matching bump in Cargo.toml, Cargo.lock, and the .TH header and VERSION section of man/start-cli.1; one sentence in the packaging guide's environment-setup page.

🤖 Generated with Claude Code

@MattDHill
MattDHill requested a review from helix-nine September 8, 2026 16:57
@MattDHill

Copy link
Copy Markdown
Member Author

@helix-nine review this PR

@helix-nine helix-nine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed git diff origin/master...HEAD at c346fcad with the high-effort fan-out and independent verification of each candidate. The current CI checks are green, but these cases are not covered:

  1. projects/start-cli/CHANGELOG.md:12start-cli/v2.0.0 already exists, so this must begin the prospective 2.0.1 release rather than add a bare Unreleased heading. Please bump the manifest and synchronize its generated artifacts; otherwise the release pre-check rejects this tree.

  2. shared-libs/crates/start-core/locales/i18n.yaml:173 — this remediation cannot work for workspaces created by start-cli 1.0.0/1.0.1. Those versions cloned master with --depth 1, which implied a single-branch refspec with no origin/live-docs; git checkout live-docs therefore fails. The remediation needs to fetch or otherwise make live-docs available before checkout.

  3. shared-libs/crates/start-core/locales/i18n.yaml:173%{dir} is always the caller-relative literal start-technologies, although init-workspace accepts an arbitrary workspace path and init-package can run from a nested directory. The printed command then fails or targets another checkout. It needs the discovered checkout path, safely quoted for shell use.

  4. shared-libs/crates/start-core/src/s9pk/init.rs:247 — emit this warning immediately after workspace discovery, before the package-name and destination checks. An older CLI's failed npm install leaves the scaffold directory behind; after the user upgrades, retrying exits at package-exists before reaching the new diagnosis.

  5. shared-libs/crates/start-core/src/s9pk/init.rs:154 — this also warns for the supported layout where start-technologies is a symlink to an owner-managed development checkout. The existing agent-context guidance explicitly says not to switch that repository's branch; detect the symlink and direct the user to remove it and rerun init-workspace instead.

  6. shared-libs/crates/start-core/src/s9pk/init.rs:344 — verify that start-technologies itself is the Git worktree root before reading its branch. Git's parent discovery means an existing non-repository/corrupt guide directory inside another repository reports the enclosing repository's branch, and the proposed checkout command can switch that unrelated repository.

I added the required start-cli and StartSDK labels. The code-comment pass was clean.

@MattDHill
MattDHill force-pushed the fix/s9pk-init-guide-branch branch from c346fca to 4cbd077 Compare September 8, 2026 18:09
@MattDHill

Copy link
Copy Markdown
Member Author

All six addressed in the latest push:

  1. Heading is now ## [2.0.1], with the bump in projects/start-cli/Cargo.toml, Cargo.lock, and the .TH line of man/start-cli.1 (the only generated artifact that carries the version).
  2. The printed command is now cd '<checkout>' && git remote set-branches --add origin live-docs && git fetch origin && git checkout live-docs. Verified on a fresh --filter=blob:none --no-checkout --depth 1 --branch master clone: plain checkout live-docs fails with pathspec 'live-docs' did not match, this sequence lands on live-docs, and git pull --ff-only works afterwards.
  3. The command carries the discovered checkout path, single-quoted with embedded quotes escaped; the prose keeps the bare start-technologies name.
  4. Both notices moved to right after find_workspace_root, ahead of the package-name and destination checks.
  5. A symlinked checkout gets its own string (s9pk.init.guide-linked-off-branch): remove the link and rerun init-workspace; no checkout command is printed for it.
  6. git rev-parse --show-toplevel --abbrev-ref HEAD in one call; the notice is skipped unless the toplevel equals the canonicalized start-technologies path, and on a detached HEAD.

@MattDHill
MattDHill requested a review from helix-nine September 8, 2026 18:10

@helix-nine helix-nine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-reviewed the full git diff origin/master...HEAD and the fix delta from c346fcad at head 4cbd07767. The original six findings are addressed, but this pass found three blocking issues:

  1. projects/start-cli/man/start-cli.1:123 — the generated header now says 2.0.1, but the explicit VERSION section still says v2.0.0. I ran make manpages-check; its four generator tests pass, then it exits 2 on exactly this missing v2.0.0v2.0.1 change. Please regenerate and commit the complete man page.

  2. shared-libs/crates/start-core/locales/i18n.yaml:180 — the new command makes live-docs fetchable for 1.0.0/1.0.1 workspaces, but those released versions also created a sparse, shallow checkout. Switching branches preserves the sparse patterns, so projects/start-sdk/lib, projects/start-os, shared-libs, and the CLI manifest remain absent after the warning disappears. Please include the existing migration steps for that population: disable sparse checkout and conditionally unshallow it (the 1.0.2 changelog already documents both).

  3. shared-libs/crates/start-core/src/s9pk/init.rs:358--show-toplevel proves only that start-technologies is a repository root, not that it is this monorepo. init-workspace accepts any existing normal repository at that path, provisions the workspace, and can now tell the user to alter that unrelated repository's origin refspec and branch. Please validate stable monorepo sentinel paths/content before provisioning or printing Git remediation; checking the origin URL would incorrectly reject supported forks.

Two nonblocking cleanup items:

  • shared-libs/crates/start-core/src/s9pk/init.rs:339-363 combines a filesystem path and branch into newline-delimited UTF-8, then lossily decodes it. Valid Unix paths containing a newline or non-UTF-8 byte silently suppress the warning. A root-relative identity probe plus a separate branch query avoids serializing the path.
  • shared-libs/crates/start-core/src/s9pk/init.rs:358 — reduce the explanatory so comment to the invariant, e.g. // `start-technologies` must be the Git worktree root.

@MattDHill
MattDHill force-pushed the fix/s9pk-init-guide-branch branch from 4cbd077 to 7c2cd81 Compare September 8, 2026 19:45
@MattDHill

Copy link
Copy Markdown
Member Author

Second round addressed in the latest push:

  1. man/start-cli.1 VERSION section is now v2.0.1, matching the .TH header.
  2. The command is assembled from the checkout's state: git fetch --unshallow origin replaces the plain fetch when rev-parse --is-shallow-repository is true, and git sparse-checkout disable is appended when core.sparseCheckout is true. Verified on a clone made the way 1.0.1 made it (--filter=blob:none --no-checkout --depth 1 --branch master, then sparse-checkout set --no-cone projects/start-sdk/docs): after the printed sequence it is on live-docs, unshallowed, unsparse, projects/start-cli/Cargo.toml is present, and git pull --ff-only works.
  3. The notice now also requires git ls-tree -d HEAD -- projects/start-sdk/docs/package-template to be non-empty, so an unrelated repository at that path gets no Git remediation. The origin URL is not consulted, so forks still qualify. Provisioning in init-workspace is unchanged by this PR.

Nonblocking items: --show-cdup replaces --show-toplevel, so no path passes through git's stdout — the one rev-parse call yields only the cdup, the shallow flag, and the branch name. The comment is reduced to the invariant.

@MattDHill
MattDHill requested a review from helix-nine September 8, 2026 19:46

@helix-nine helix-nine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-reviewed the full git diff origin/master...HEAD and the amended delta from 4cbd07767 at head 7c2cd816. The second-round findings are fixed, and make manpages-check now passes. Four blocking issues remain:

  1. shared-libs/crates/start-core/src/s9pk/init.rs:397-406 — the generated command still hardcodes origin and performs a plain git checkout live-docs. It fails when clone.defaultRemoteName gave the workspace another remote name, when a supported fork does not publish live-docs, or when checkout.guess=false. If a stale local live-docs already exists, checkout succeeds without fast-forwarding it, then future warnings are suppressed. Please resolve the fork-source policy and emit an explicit fetch/create-or-update/upstream sequence that converges on the intended published ref.

  2. shared-libs/crates/start-core/locales/i18n.yaml:172-177 — the symlink remediation is still caller-relative. From a package directory, “remove start-technologies” targets the wrong path and bare init-workspace is rejected; with init-workspace /other/workspace, the bare rerun targets the caller's directory. The branch migration also leaves v1.0.0/1.0.1's old root AGENTS.md link dangling unless initialization runs again. Please print quoted discovered link/workspace paths and finish repair with start-cli s9pk init-workspace '<workspace>'.

  3. shared-libs/crates/start-core/src/s9pk/init.rs:372-377 — requiring the current package-template path suppresses the warning for real older monorepo branches that predate that path. init-workspace reports success, while init-package then says to rerun init-workspace, which repeats the same no-op. Please use a durable identity check and emit a generic off-branch warning when Git mutation cannot be prescribed safely; the changelog and guide currently overstate coverage.

  4. shared-libs/crates/start-core/src/s9pk/init.rs:221-222init-package runs the outdated-CLI check against the working branch before diagnosing it as off live-docs. A supported development checkout or symlink at this PR therefore says released 2.0.0 is behind “published 2.0.1,” even though 2.0.1 is prospective. Gate the version warning on the guide actually being on live-docs.

Nonblocking hardening from the same paths:

  • Parse core.sparseCheckout as a Git boolean, and avoid checking out live-docs before a later sparse-disable failure can leave an incomplete state that suppresses future repair.
  • Require a 040000 tree from ls-tree; -d also accepts a 160000 gitlink.
  • docs.display().to_string() is lossy for valid non-UTF-8 Unix paths, so omit the executable command when the path cannot be represented losslessly.

The code-comment pass is clean.

@MattDHill
MattDHill force-pushed the fix/s9pk-init-guide-branch branch from 7c2cd81 to 1dcc3dc Compare September 8, 2026 20:27
@MattDHill MattDHill changed the title fix(start-cli): say when the workspace's start-technologies checkout is off live-docs fix(start-cli): put the workspace's start-technologies checkout back on live-docs Sep 8, 2026
@MattDHill

Copy link
Copy Markdown
Member Author

Third round addressed in the latest push, with a design change: the remediation is no longer printed. init-workspace performs it on a checkout the workspace owns, and init-package only diagnoses and points at start-cli s9pk init-workspace '<workspace>'.

  1. Fork-source policy: the workspace follows the remote its current branch tracks (branch.<b>.remote), falling back to the sole configured remote, and stops otherwise — the policy the guide already states for forks. The sequence is explicit and converges: fetch <remote> +refs/heads/live-docs:refs/remotes/<remote>/live-docs, verify, add the refspec if uncovered, unshallow, sparse-disable, checkout -B live-docs <remote>/live-docs (refused if a local live-docs is not an ancestor of the remote one), branch --set-upstream-to. checkout.guess and the remote's name no longer matter. Verified with a shell mirror on seven clone shapes, including a stale local live-docs (fast-forwarded), a diverged one (refused), a remote without live-docs (refused with git's message), and clone.defaultRemoteName=upstream (moved via upstream).
  2. Paths: both notices carry the discovered checkout path (and, for a link, its target) and the command start-cli s9pk init-workspace '<workspace>' with the canonical workspace path single-quoted, or the bare command when the path is not UTF-8. Rerunning init-workspace repoints the legacy AGENTS.md link in the same run.
  3. Identity moved to the target: <remote>/live-docs must carry projects/start-sdk/docs/package-template as a 040000 tree. The current branch's tree is not consulted, so an old monorepo branch is moved rather than ignored, and a directory whose remote lacks that branch or tree gets a refusal naming the reason instead of a no-op. init-package's notice fires for any off-branch repository root and prescribes no git.
  4. The outdated-start-cli notice is gated on the checkout being on live-docs.

Nonblocking: core.sparseCheckout is read with --type=bool; unshallow and sparse-disable run before the branch switch; ls-tree must return 040000 tree; no path passes through git's stdout, and Path::to_str gates the quoted command.

@MattDHill
MattDHill requested a review from helix-nine September 8, 2026 20:28

@helix-nine helix-nine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-reviewed the full git diff origin/master...HEAD and the design-change delta from 7c2cd816 at head 1dcc3dcf. The prior findings are addressed and make manpages-check passes, but automatic mutation of the persistent checkout introduces these blocking issues:

  1. shared-libs/crates/start-core/src/s9pk/init.rs:452-564 — refuse a dirty checkout before any mutation. checkout -B preserves non-conflicting tracked edits and untracked files, so this can report success and then scaffold packages from a locally modified template.

  2. shared-libs/crates/start-core/src/s9pk/init.rs:168-184 / locales/i18n.yaml:186-191 — every Git failure advises removing the checkout, including dirty state, unpushed commits, corruption, and transient network errors. Following that advice can destroy user work. Tell the user to resolve the Git error or preserve/move the checkout before replacing it; do not recommend deletion generically.

  3. shared-libs/crates/start-core/src/s9pk/init.rs:474-509,564-570 — use fully qualified refs throughout. The fetch writes refs/remotes/<remote>/live-docs, but validation, ancestry, checkout, and upstream setup use DWIM shorthands. A colliding tag or branch named live-docs or <remote>/live-docs can validate/reset against the wrong object.

  4. shared-libs/crates/start-core/src/s9pk/init.rs:500-524 — unshallow before ancestry validation and distinguish merge-base --is-ancestor exit 1 from operational failure. .is_err() currently reports missing/corrupt objects and other Git failures as local divergence, discarding the real error.

  5. shared-libs/crates/start-core/src/s9pk/init.rs:526-545 — refspec coverage must validate the effective source-to-destination mapping and negative exclusions. A positive source wildcard can be accepted even when it maps elsewhere or ^refs/heads/live-docs excludes it, leaving the configured upstream stale after the one-shot explicit fetch.

  6. shared-libs/crates/start-core/src/s9pk/init.rs:564-573 — branch replacement and upstream setup are separate operations. Failure or interruption after checkout leaves HEAD named live-docs with missing/wrong tracking; the next run skips repair because the branch name matches. Establish tracking with the checkout or validate/repair it on an already-live-docs branch.

  7. shared-libs/crates/start-core/src/s9pk/init.rs:385-573 — this persistent, roughly eleven-command Git migration has no committed regression tests. Please add real temporary-repository coverage for clean success/idempotence, dirty refusal, local commits, shallow+sparse migration, ref collisions, effective refspecs, and final tracking state. The existing module already has temp-directory and real-Git test patterns.

  8. shared-libs/crates/start-core/src/s9pk/init.rs:440-447 — the non-UTF-8 path fallback prints bare start-cli s9pk init-workspace. From init-package inside a package directory that command is rejected or targets another directory. Omit executable recovery guidance when the intended workspace path cannot be represented losslessly.

Two nonblocking documentation corrections:

  • projects/start-cli/CHANGELOG.md:23-24 says the outdated notice is only emitted on live-docs, but CliContext::build_key() still calls it unconditionally (for example from s9pk pack). Narrow this entry to init-package, or gate the shared caller too.
  • projects/start-sdk/docs/src/environment-setup.md:310 should say init-workspace moves a workspace-owned checkout; symlinked checkouts are only diagnosed.

Comments:

  • shared-libs/crates/start-core/src/s9pk/init.rs:385 — delete; the private helper's body and return type state these cases.
  • shared-libs/crates/start-core/src/s9pk/init.rs:450-451 — rewrite to the caller-visible result, e.g. Moves the guide to MONOREPO_BRANCH without discarding local commits.

Given the state space exposed here, a diagnosis-only design remains a valid smaller alternative to making init-workspace mutate the checkout.

…is off live-docs

`s9pk init-package` scaffolds from the package template in whatever branch
the workspace's `start-technologies` checkout is on, and never looks at that
branch. A workspace scaffolded by start-cli 1.x cloned `master`, whose
template now pins an unpublished `@start9labs/start-sdk`, so `npm install`
fails with ETARGET and nothing names the cause. Re-running `init-workspace`
there has the same blind spot.

Both commands now print a one-line notice when the checkout is on a branch
other than `live-docs`, naming the checkout and its branch. A checkout the
workspace owns is to be moved out of the workspace and `init-workspace`
rerun, which clones fresh on `live-docs`; a symlinked one is left alone and
the link removed instead. No git operation is prescribed or performed: a
1.0.1 clone is single-branch, shallow and sparse, and no one command
sequence is right for every checkout state.

The branch is read from the checkout's own `.git/HEAD` (following a
`gitdir:` file), so an enclosing repository is never consulted and a
detached HEAD stays silent. The outdated-start-cli notice is gated on the
same check, since off `live-docs` the manifest names a version that has not
shipped. In `init-package` the notice fires right after workspace
discovery, so a scaffold left behind by a failed run cannot hide it.

Bumps start-cli to 2.0.1.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@MattDHill
MattDHill force-pushed the fix/s9pk-init-guide-branch branch from 1dcc3dc to 2cc13f0 Compare September 8, 2026 21:05
@MattDHill MattDHill changed the title fix(start-cli): put the workspace's start-technologies checkout back on live-docs fix(start-cli): say when the workspace's start-technologies checkout is off live-docs Sep 8, 2026
@MattDHill

Copy link
Copy Markdown
Member Author

Fourth round: took the diagnosis-only alternative. Nothing prescribes or performs a git operation any more.

1–7. Moot with the mutation gone: no dirty-tree check, ref qualification, ancestry test, refspec semantics, upstream setup, or migration to test, because there is no migration. The notice says to move the checkout out of the workspace (never to delete it) and run start-cli s9pk init-workspace '<workspace>', which clones fresh on live-docs. That is correct for every checkout state — unpushed commits and dirty trees travel with the moved directory.
8. When the workspace path is not valid UTF-8, the notice says to run start-cli s9pk init-workspace from the workspace root; no path is printed as a command.

Detection now reads the checkout's own .git/HEAD, following a gitdir: file for a linked worktree or submodule, instead of invoking git: no upward repository discovery, no path through a subprocess, and a detached HEAD is silent. Unit tests cover a plain .git/HEAD, a gitdir: file by absolute and relative path, a detached HEAD, a missing checkout, and the printed command's quoting and UTF-8 gate; they need no git binary.

Nonblocking: the changelog's Fixed line now holds for every caller — the gate lives inside warn_if_start_cli_outdated, so CliContext::build_key() is covered; environment-setup.md describes the move-aside-and-rerun step and no longer claims a move. Comments: the private helpers carry no doc comments.

@MattDHill
MattDHill requested a review from helix-nine September 8, 2026 21:05

@helix-nine helix-nine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the full diff and the rewrite from 1dcc3dcf at head 2cc13f0dd. The diagnosis-only design resolves the prior findings: it does not mutate or prescribe Git state, preserves workspace-owned checkouts by telling the user to move them aside, handles symlinked checkouts separately, gates the outdated-client notice inside the shared helper, and gives non-UTF-8 workspace paths usable cwd-based guidance.

I also verified the direct and gitfile HEAD parsing, path quoting, version/manpage/changelog/docs synchronization, and all five locale placeholder sets. Detached HEAD remains outside the branch-specific notice's stated contract.

Checks:

  • git diff --check origin/master...HEAD
  • make manpages-check
  • CARGO_TARGET_DIR=/tmp/pr3910-target cargo test -p start-core s9pk::init --features=test (8 passed)

No blocking findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants