Phase 64 follow-ups: phase-language sweep + migrate fixes + behavioral tag catalog#27
Merged
Conversation
…bility wording
Phase numbers are internal planning artifacts that don't mean anything to
end users. Strip every "Phase X.Y" reference from runtime error messages,
clap help, doc comments rendered into --help, npmrc warning text, and
workflow tests asserting on those messages. Replace with stable
capability-oriented wording ("not supported yet", "isn't supported on
Windows yet", "no global-scope cooldown overrides yet", etc.) that
remains true regardless of internal milestones.
Touched user-visible surfaces:
* lpm patch — range-version rejection
* lpm install -g — --min-release-age + --ignore-provenance-drift gates
* lpm install --policy=triage — LLM-triage availability note
* lpm rebuild / lpm doctor — sandbox unsupported-platform warning
* lpm install --filter — substring-vs-glob diagnostic
* .npmrc parser — per-origin TLS / mTLS unsupported warnings
* lpm-workspace package.json schema — patchedDependencies field doc
Test asserts flipped to encode the user-facing contract instead of the
internal phase number, per the project's test-asserts-user-contracts rule.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related contract bugs in lpm migrate: 1. The CLI advertised "yes implies force" in --help, the dispatcher collapsed them at main.rs, and the migrate flow itself has no prompt path. Result: a CI engineer reaching for `-y` to make migrate non-interactive silently widened the blast radius into clobbering an existing lpm.lock, a behavior no other peer PM exposes through `-y`. Decouple them: `-y` is now reserved as a non-interactive flag (no-op today) and does NOT imply --force. Scripts that already pass `-y` continue to parse cleanly. To overwrite an existing lockfile the user must pass --force explicitly. 2. The rollback path's lpm.lockb cleanup at run_rollback used `restored.iter().any(|f| f == "lpm.lock")`, but the v2 backup manifest appends " (removed)" to every newly-created file's restored entry, so the exact-match comparison failed for fresh migrations and left lpm.lockb orphaned on disk after `lpm migrate --rollback`. Real fix: track lpm.lockb explicitly in MigrationBackup alongside lpm.lock. Lockfile::write_all writes both files atomically, so the backup chain needs both entries to round-trip cleanly. The brittle suffix-string fallback is dropped — the backup layer now handles the round-trip uniformly for every file the migration touched. Folded in: append a "Need to undo?" hint to the migrate success summary so the rollback escape hatch is discoverable on the success path, not only on the verification-failure path it already shows up in. Test coverage: extends migrate_rollback_restores_original to assert lpm.lockb is also gone (regression test for the orphan bug — verified empirically by reverting the backup_file(&lockb_path) call: test fails with "lpm.lockb should be removed after rollback"). New migrate_yes_alone_does_not_overwrite_existing_lockfile pins the -y vs --force decoupling end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t guard
The 22 behavioral tags emitted by lpm audit / lpm query were defined as
Rust struct fields across three sibling modules, with their CLI tokens
in a separate enum and their human-readable descriptions hand-maintained
in two distinct doc pages (security-audit.mdx + query.mdx). Every drift
class — token rename, description rewrite, count drift in prose,
out-of-sync severity claim — could ship undetected because nothing in
the build asserted parity across the surfaces.
This change makes lpm_security::query the single source of truth:
* New `BehavioralTagInfo { token, group, severity, description }`
struct + `behavioral_tag_catalog()` accessor compose the catalog
from `PseudoClass::all_behavioral()`, `group()`, `severity()`, and
a new `description()` method.
* `PseudoClass::group()` returns `Option<TagGroup>` — `Source` /
`SupplyChain` / `Manifest` for the 22 behavioral tags, `None` for
state / severity / special selectors.
* `PseudoClass::description()` returns `Option<&'static str>`. Only
behavioral tags carry a description; state/severity/special return
`None`. The behavioral entries are non-empty by contract.
* 4 unit tests in lpm-security pin the catalog invariants: every
`all_behavioral` entry round-trips through the catalog accessor,
descriptions are non-empty, tokens round-trip through `from_name`,
and the catalog partitions cleanly into three groups.
* 3 cross-repo drift tests in lpm-workflows assert exact parity
between the catalog and both doc pages (security-audit.mdx +
query.mdx). The drift test mirrors the schema_drift gate's resolver
pattern: `LPM_DOCS_DIR=<path>` for explicit enforcement,
`LPM_DOCS_DIR=skip` for opt-out, otherwise walk-up from
CARGO_MANIFEST_DIR with a silent skip when the docs repo isn't
checked out next to rust-client.
Empirically verified: deliberately rewriting one tag description in
security-audit.mdx made `security_audit_mdx_mirrors_catalog` fail with
the exact drift pointed at the offending row. With the catalog as the
single source of truth, future tag additions, renames, or rewrites
land in one place and CI enforces the doc tables follow.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…track deferred surfaces Three follow-ups to the migrate + behavioral-tag tranche, plus a tracking doc for surfaces we marked "not supported yet": 1. **migrate rollback: .gitattributes orphan.** `ensure_gitattributes` creates the file when it's missing and modifies it when present, but the backup chain only registered the file with `backup_file` inside `if exists()`. Result: a fresh migration with no pre-existing .gitattributes left it on disk after `lpm migrate --rollback`, violating the "restore original state" contract the docs promise. Fix: register the file unconditionally — the backup layer's created-vs-restored distinction handles both paths uniformly. The regression test now plants the fixture-NOT-having-.gitattributes precondition, asserts migrate creates it, then asserts rollback removes it. Verified empirically by running the new assertion against pre-fix code (failed) and post-fix (passed). 2. **migrating-from-npm.mdx CI hint drift.** The guide said the migrate flow "prints a hint pointing at `lpm ci setup github-actions`", but the actual CLI hint at migrate.rs:506 says `lpm migrate --ci`. Drop the wrong command; describe the platform detection + the actual `--ci` invocation. 3. **query.mdx drift test asymmetric.** The reverse-direction stray-tag check existed for security-audit.mdx but not query.mdx, so a bogus extra row added to query.mdx would pass CI. Add the parallel check; tighten `parse_tag_rows` to reject combinator syntax samples (`:a, :b`, `:not(:a)`, `:root > :child`) so the Combinators table doesn't trip false positives. Verified by injecting a `:bogus-tag` row: drift test fails with the exact-token-named diagnostic. 4. **DEFERRED_SURFACES.md.** New engineering tracking doc inventorying every "not supported yet" / "isn't supported yet" / "doesn't extend to globals yet" surface this tranche locked into the user-visible contract. Each entry names the source-of-truth string, why the feature is deferred, and the workaround today. Covers patch range selectors, patch renames, Windows sandbox, global-scope cooldown + provenance overrides + sandbox parity, the LLM triage layer, .npmrc mTLS / per-origin TLS, workspace deploy local-package injection, and the reserved `lpm migrate -y` flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GPT's audit caught two contract bugs introduced alongside the deferred-surfaces tracking doc: 1. **`--allow-new` silently dropped on `lpm install -g`.** The global-install validator only rejected `--min-release-age` and the provenance-drift flags; the `-g` dispatch path then explicitly discarded `allow_new` without telling the user. The tracking doc advertised the rejection, but the code shipped a silent-drop — exactly the contract dishonesty the doc was meant to prevent. Fix: thread `allow_new` through `validate_global_install_project_scoped_flags` and reject with the same not-supported-yet wording the cooldown override uses. New regression test `install_global_rejects_allow_new_flag` pins the parser-level rejection symmetric with the existing `--min-release-age` and provenance-drift tests. 2. **DEFERRED_SURFACES.md TLS section overstated the gap.** The entry said `cafile` / `certfile` / `keyfile` (both global and per-origin) are "parsed and warned" and that requests use the global TLS config regardless. That's wrong for global `cafile`: npmrc.rs:1181 parses it, stores it in `tls.extra_roots`, and client.rs attaches those roots to the HTTP client. The genuinely-deferred surfaces are mTLS client certs (global + per-origin) and per-origin TLS overrides. Doc rewritten to scope the deferral honestly + name the global `cafile` / `ca` working path as the today-workaround. 3. **DEFERRED_SURFACES.md cooldown section corrected.** Now says both `--min-release-age` and `--allow-new` are rejected on `-g`, matching the validator's actual behavior post-fix. CI gate at HEAD: workspace clippy clean, fmt clean, **5609/5609 tests pass / 7 skipped** (+1 vs. previous: install_global_rejects_allow_new_flag), Fumadocs build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DEFERRED_SURFACES.md is engineering-internal tracking — not a contract artifact, not a contributor-facing reference. Move it under /private/ alongside other internal docs and gitignore that path so the tree stays for local use without being pushed. The "not supported yet" source-of-truth strings still live in code and CLI surfaces; only the engineering-internal aggregator moved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three coherent tranches addressing Phase 64 findings #8, #10, #29, #37, plus follow-up audits:
-yfrom--force(no more silent destructive widening), tracklpm.lockb+.gitattributesin the migration backup chain so rollback can clean them up, drop the brittle suffix-string fallback, fix the "interactive walk" + "rollback doesn't touch lockfile" doc lies, add rollback hint to success summary.behavioral_tag_catalog()inlpm-securitywith{ token, group, severity, description }per tag. Cross-repo drift test asserts bothsecurity-audit.mdxandquery.mdxmirror the catalog exactly (mirroring theLPM_DOCS_DIRresolver pattern ofschema_drift.rs).Plus three audit-driven follow-ups:
--allow-newwas silently dropped onlpm install -g— now rejected with a clear "not supported yet" message + regression test..gitattributesorphan after migrate rollback (when migrate created it from scratch) — fixed by tracking unconditionally.lpm ci setup github-actionsvs. reallpm migrate --ci) — corrected.Test plan
cargo clippy --workspace --all-targets -- -D warningscleancargo fmt --checkcleancargo nextest run --workspace --exclude lpm-integration-tests— 5609 pass / 7 skippedsecurity-audit.mdxandquery.mdxlpm.lockbrollback regression verified by revertingbackup_file(&lockb_path)— test fails as expected.gitattributesrollback regression verified the same wayrust-client-docsalready merged to main🤖 Generated with Claude Code