fix(start-cli): say when the workspace's start-technologies checkout is off live-docs - #3910
fix(start-cli): say when the workspace's start-technologies checkout is off live-docs#3910MattDHill wants to merge 1 commit into
Conversation
|
@helix-nine review this PR |
helix-nine
left a comment
There was a problem hiding this comment.
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:
-
projects/start-cli/CHANGELOG.md:12—start-cli/v2.0.0already exists, so this must begin the prospective2.0.1release rather than add a bareUnreleasedheading. Please bump the manifest and synchronize its generated artifacts; otherwise the release pre-check rejects this tree. -
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 clonedmasterwith--depth 1, which implied a single-branch refspec with noorigin/live-docs;git checkout live-docstherefore fails. The remediation needs to fetch or otherwise makelive-docsavailable before checkout. -
shared-libs/crates/start-core/locales/i18n.yaml:173—%{dir}is always the caller-relative literalstart-technologies, althoughinit-workspaceaccepts an arbitrary workspace path andinit-packagecan 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. -
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 failednpm installleaves the scaffold directory behind; after the user upgrades, retrying exits atpackage-existsbefore reaching the new diagnosis. -
shared-libs/crates/start-core/src/s9pk/init.rs:154— this also warns for the supported layout wherestart-technologiesis 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 reruninit-workspaceinstead. -
shared-libs/crates/start-core/src/s9pk/init.rs:344— verify thatstart-technologiesitself 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.
c346fca to
4cbd077
Compare
|
All six addressed in the latest push:
|
helix-nine
left a comment
There was a problem hiding this comment.
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:
-
projects/start-cli/man/start-cli.1:123— the generated header now says 2.0.1, but the explicit VERSION section still saysv2.0.0. I ranmake manpages-check; its four generator tests pass, then it exits 2 on exactly this missingv2.0.0→v2.0.1change. Please regenerate and commit the complete man page. -
shared-libs/crates/start-core/locales/i18n.yaml:180— the new command makeslive-docsfetchable for 1.0.0/1.0.1 workspaces, but those released versions also created a sparse, shallow checkout. Switching branches preserves the sparse patterns, soprojects/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). -
shared-libs/crates/start-core/src/s9pk/init.rs:358—--show-toplevelproves only thatstart-technologiesis a repository root, not that it is this monorepo.init-workspaceaccepts any existing normal repository at that path, provisions the workspace, and can now tell the user to alter that unrelated repository'soriginrefspec 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-363combines 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 explanatorysocomment to the invariant, e.g.// `start-technologies` must be the Git worktree root.
4cbd077 to
7c2cd81
Compare
|
Second round addressed in the latest push:
Nonblocking items: |
helix-nine
left a comment
There was a problem hiding this comment.
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:
-
shared-libs/crates/start-core/src/s9pk/init.rs:397-406— the generated command still hardcodesoriginand performs a plaingit checkout live-docs. It fails whenclone.defaultRemoteNamegave the workspace another remote name, when a supported fork does not publishlive-docs, or whencheckout.guess=false. If a stale locallive-docsalready 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. -
shared-libs/crates/start-core/locales/i18n.yaml:172-177— the symlink remediation is still caller-relative. From a package directory, “removestart-technologies” targets the wrong path and bareinit-workspaceis rejected; withinit-workspace /other/workspace, the bare rerun targets the caller's directory. The branch migration also leaves v1.0.0/1.0.1's old rootAGENTS.mdlink dangling unless initialization runs again. Please print quoted discovered link/workspace paths and finish repair withstart-cli s9pk init-workspace '<workspace>'. -
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-workspacereports success, whileinit-packagethen says to reruninit-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. -
shared-libs/crates/start-core/src/s9pk/init.rs:221-222—init-packageruns the outdated-CLI check against the working branch before diagnosing it as offlive-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 onlive-docs.
Nonblocking hardening from the same paths:
- Parse
core.sparseCheckoutas a Git boolean, and avoid checking outlive-docsbefore a later sparse-disable failure can leave an incomplete state that suppresses future repair. - Require a
040000 treefromls-tree;-dalso accepts a160000gitlink. 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.
7c2cd81 to
1dcc3dc
Compare
|
Third round addressed in the latest push, with a design change: the remediation is no longer printed.
Nonblocking: |
helix-nine
left a comment
There was a problem hiding this comment.
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:
-
shared-libs/crates/start-core/src/s9pk/init.rs:452-564— refuse a dirty checkout before any mutation.checkout -Bpreserves non-conflicting tracked edits and untracked files, so this can report success and then scaffold packages from a locally modified template. -
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. -
shared-libs/crates/start-core/src/s9pk/init.rs:474-509,564-570— use fully qualified refs throughout. The fetch writesrefs/remotes/<remote>/live-docs, but validation, ancestry, checkout, and upstream setup use DWIM shorthands. A colliding tag or branch namedlive-docsor<remote>/live-docscan validate/reset against the wrong object. -
shared-libs/crates/start-core/src/s9pk/init.rs:500-524— unshallow before ancestry validation and distinguishmerge-base --is-ancestorexit 1 from operational failure..is_err()currently reports missing/corrupt objects and other Git failures as local divergence, discarding the real error. -
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-docsexcludes it, leaving the configured upstream stale after the one-shot explicit fetch. -
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 namedlive-docswith 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-docsbranch. -
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. -
shared-libs/crates/start-core/src/s9pk/init.rs:440-447— the non-UTF-8 path fallback prints barestart-cli s9pk init-workspace. Frominit-packageinside 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-24says the outdated notice is only emitted onlive-docs, butCliContext::build_key()still calls it unconditionally (for example froms9pk pack). Narrow this entry toinit-package, or gate the shared caller too.projects/start-sdk/docs/src/environment-setup.md:310should sayinit-workspacemoves 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>
1dcc3dc to
2cc13f0
Compare
|
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 Detection now reads the checkout's own Nonblocking: the changelog's Fixed line now holds for every caller — the gate lives inside |
helix-nine
left a comment
There was a problem hiding this comment.
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...HEADmake manpages-checkCARGO_TARGET_DIR=/tmp/pr3910-target cargo test -p start-core s9pk::init --features=test(8 passed)
No blocking findings.
Summary
s9pk init-packagescaffolds from the package template in whatever branch the workspace'sstart-technologiescheckout is on, and never looks at that branch. A workspace scaffolded by start-cli 1.x clonedmaster, whose template has pinned@start9labs/start-sdk@3.0.0since #3900 — a version npm does not have — sonpm installfails withETARGETand nothing names the cause:start-cli 2.0.0 clones
live-docsfor new workspaces and documents a manualgit checkout live-docsfor 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 failednpm installleft behind cannot hide it — andinit-workspace, when it finds an existing checkout, print one line whenstart-technologiesis on a branch other thanlive-docs. It names the checkout's path and branch and says what to do:start-cli s9pk init-workspace '<workspace>', which clones a fresh checkout onlive-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 runinit-workspacefrom the workspace root instead of printing a path.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 agitdir: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 (noref:line) says nothing. A missing checkout says nothing either;init-workspaceclones it andinit-packagereports the missing template as before.Also
start-cliis behind the published release is given only when the checkout is onlive-docs. The gate lives insidewarn_if_start_cli_outdated, soCliContext::build_key()(e.g.s9pk pack) is covered too; on any other branch the manifest names an unreleased version..git/HEAD, following agitdir: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.## [2.0.1]with the matching bump inCargo.toml,Cargo.lock, and the.THheader and VERSION section ofman/start-cli.1; one sentence in the packaging guide's environment-setup page.🤖 Generated with Claude Code