fix(remove,doctor,xim,hints): remove means removed, doctor tells the truth and walks every subos, local is an overlay, notices say it once (2026.9.12.1) - #591
Merged
Conversation
…d the plan Two documents under .agents/docs: the survey with the six decisions measured on a real home (159 local recipes, 157 byte-identical to the index; a plain `self doctor` that says "no index provides" for 100 entries the index does provide; 65 unreferenced mcpp versions the fix ladder would re-download), and the task-by-task implementation plan this version implements.
`Config::hint_seen`/`mark_hint_seen` already persist an opaque id set in `.xlings.json`, but nothing decided WHEN to use it: four call sites printed "run xlings self doctor --fix" on every command, and a namespace-priority warning printed on every command too, because the process that decided to warn exits with no memory that it already told the user. `xlings.core.notice` is that decision, injectable so it is testable without a home: seen(id+fingerprint) -> say nothing; unseen -> emit the Diagnostic at Note level, then mark it (swallowing a read-only home's write failure, since the note was already shown). The fingerprint half of the key is what lets an upgrade to a NEWER version, or a conflict with a DIFFERENT loser, announce again while the identical case stays quiet. Tasks 2 and 3 build on this primitive; this commit only adds it and wires it into `xlings.core`.
…nnounced once
Four call sites (`xvm use`, `xim install`, two in `xim list`) printed "run
xlings self doctor --fix" via a per-process `static bool`, so it repeated on
every command instead of showing once. `self update`'s own three-line nudge
had the same shape. Both are deleted: `migration_hint` is now a pure fact
for doctor's own report ("set up by X; last verified by Y", no `--fix`
command embedded in it — that command is the report it is already part of),
and a new `show_upgrade_notice_once_` in cli.cpp says the same fact once per
home on an ordinary command, via `notice::notice_once`.
That needed a second stamp: `.xlings.json["verifiedBy"]`, distinct from
`"version"` (which `self install` moves on every upgrade whether or not
anything was checked). `verifiedBy` moves only when `self doctor --fix`
converges (`outstanding == 0`, no longer additionally gated on
`foreignPayloads == 0` — a foreign payload is doctor's business, not the
migration stamp's), and it is what the upgrade notice and the doctor report
compare against Info::VERSION to decide whether either has anything to say.
…a real one is said once
A `local:` build sitting next to the index's own copy at the SAME version is
the ordinary shape of a dev machine, not a namespace conflict — there is
nothing to pick between. `announce_demotion_` used to say so (and remember
having said so) unconditionally whenever ANY candidate lost the bare-name
tiebreak, version included.
`PackageMatch::demoted` (and `NamespaceRankResult_::demoted`) now carries a
`DemotedCandidate{coordinate, version}` instead of a bare coordinate string,
so the decision can ask "did every loser lose only because it's a duplicate
of the version already chosen" without re-parsing `@` out of a command line.
When they all did, nothing is printed and nothing is remembered. When a real
alternative exists, it is said once per home via `notice::notice_once`
rather than an unconditional `log::warn`, with the in-process
`demotionsAnnounced_` memo kept as a cheap gate in front of the persisted
one (`resolve_target` has sixteen callers, several resolving the same target
twice in one run).
The decision itself is exposed as a free function, `detail_::demotion_notice`,
taking an injected `notice::Memo` — the same shape as `notice_once` itself —
so it is unit-testable without a loaded catalog or a home.
…ve, clear, and GC what the index already has
…all/--all-subos make "remove" mean removed
`xlings remove` now resolves against the version DB rather than the
index (a versionless name uses the active binding, or the DB's sole
version, or lists candidates and asks for --all when there are several
with none active), withdraws version-DB/workspace/shim state BEFORE
the recipe's uninstall() hook runs, and tolerates both a missing
recipe (deleted from every index) and a failing hook -- the state
withdrawal happens either way, and --force decides whether that
counts as done. --all removes every installed version, --all-subos
removes it from every subos that references it, and --subos NAME
targets one directly.
Installer::uninstall's UninstallOutcome gains hookFailure and
recipeUnavailable so cmd_remove can report which shape happened
instead of returning unexpected() and leaving state untouched.
Found and fixed along the way:
- a version record whose payload was deleted by hand (or already
claimed by GC) was refused as "not installed" and left registered
forever, because the check only asked the payload, never the DB
record it was supposed to be describing
- resolving a recipe-less removal from the DB record's own
bindingGroup provider, not the bare target name, since a namespaced
install's provider ("xim:plain") does not equal the bare name and
the removal-context ownership check compares them literally
- Config::set_active_subos_override's own reload reads the OLD
paths_.activeSubos before recomputing it, so remove --subos/--
all-subos now issues an explicit extra reload after each override
change (the pattern's other caller, subos.cpp, was masked by
cmd_install reloading a second time on its own)
New e2e: tests/e2e/remove_force_contract_test.sh (E2E-102), covering
a failing hook with and without --force, a recipe removed from the
index, a payload deleted by hand, a DB record with no active
workspace binding (#578), --all-subos across two subos, and --all
across two versions.
… not a mismatch
Review of the three prior commits came back "needs fixes":
- `announce_demotion_` and `detail_::demotion_notice` each independently
recomputed "is every demoted candidate a duplicate of the chosen version"
and rebuilt the `losers` string -- the same verdict derived by two copies
of the same code. Extracted `detail_::demotion_verdict(target, chosen) ->
optional<DemotionVerdict{fingerprint, losers}>` as the one place that rule
lives; both the in-process gate and the persisted notice now call it.
- `show_upgrade_notice_once_` fired on a home with NEITHER "verifiedBy" NOR
"version" on record -- not a version mismatch, an absent record -- and
said so with fabricated wording ("last verified by an unrecorded
version"). Same rule as `migration_hint`'s own empty guard: suppress the
notice entirely when there is nothing on record to compare against.
Minor: dropped the dead `import xlings.core.xself.repair;` in
xvm/commands.cpp (its only use went with the prior commit), and the
unreachable `cmd != "--version"`/`cmd != "-h"` clauses in the upgrade-notice
guard (the parsing loop above never assigns `cmd` from a `-`-prefixed arg).
Added: three unit tests for `demotion_verdict` (same-version -> nullopt,
no candidates -> nullopt, mixed -> fingerprint+losers); e2e scenario I0
(a bare `self init` home, no `version` key at all, prints nothing on two
runs).
…er replaces three copies Review findings on the local-overlay-with-provenance feature: - overlay::load_with_files() merges .overlay.json with a scan of pkgs/*/*.lua, synthesizing an entry for every recipe added before this module existed (or dropped in by hand). --list-xpkg, --clear-xpkg, and gc_identical now see the whole overlay instead of only what has a provenance record -- the 157-of-159 case the feature exists for. Only entries that were already tracked are ever written back to .overlay.json. - overlay::remove_recipe_file() replaces the delete-file-then-clean-empty-dir block that was duplicated across gc_identical, cmd_remove_xpkg, and cmd_clear_xpkg. It takes a resolved path rather than (dir, name), since a legacy untracked recipe's on-disk location cannot be safely recomputed from its name via recipe_path(). - cmd_add_xpkg's identical-refusal path no longer leaves an empty letter directory behind; --list-xpkg output is sorted by name.
…sability-2026.9.12 # Conflicts: # tests/e2e/run_all.sh
…, fix the subos-scope restore order, one parser for store paths Critical: `self doctor --fix`'s repair ladder (repair_one, R3) shelled out `xlings remove <coord> -y` without --force and treated a non-zero exit as "could not be removed", returning before the compensating reinstall ran. Under remove's new withdraw-then-hook semantics a failing uninstall() hook exits 1 with the record already gone, so the ladder would drop a real entry, report a false negative, and never put the package back. Now passes --force (this remove is a means to a reinstall, not the user's own request) and no longer checks its exit code at all: the install always runs afterward, and the existing removalDone verifier alone decides both success and what gets reported. Important: cmd_remove's --subos/--all-subos loop restored Config::set_active_subos_override before resetting XLINGS_ACTIVE_SUBOS, so the override's own empty-override fallback read the env var one transition too early and left paths().activeSubos on the subos being left rather than the one being returned to -- and the enter/restore pair was not exception-safe. Extracted into ScopedSubosOverride, an RAII guard that restores in the same relative order it entered (env var, then override, then one reload), so a destructor -- not fall-through code -- runs the restore on every exit path. Minor: store_version_dir_from_recorded_path_ duplicated the forward-scan xvm::coordinate_from_payload_path already does right-to-left in five other call sites; now delegates to it and re-encodes the result with package_store_name, the same join the installer itself writes. tests/unit/test_self_repair.cpp: updated exact-command assertions for --force; replaced the "remove failing blocks reinstall" case (now the wrong behavior) with two verifier-driven cases covering a non-zero exit either way. tests/unit/test_xim_install.cpp: three ScopedSubosOverride tests against Config::paths().activeSubos directly (a unit-testable seam, chosen over an e2e assertion since the bug is about in-process Config state that a fresh-process-per-invocation e2e test cannot observe).
…sability-2026.9.12 # Conflicts: # src/core/xim/commands.cpp # tests/e2e/run_all.sh
…oken-home fixture proves the blast radius is one package
load_subos_snapshots() used to swallow an unparsable or malformed
subos/<name>/.xlings.json with a bare `catch (...) {}`, dropping that subos
out of every cross-subos question with no signal it had happened. It now
takes an optional out-param collecting the unreadable names; `self doctor`
reports each as a new Warning-level FindingKind::SubosUnreadable, and
`list --all` emits a Note naming them so a hand-edit mistake in one subos
reads as "could not be read", not "nothing installed".
Adds an e2e fixture that injects four independent kinds of home damage at
once (an unreadable subos, a DB entry for a package never installed, a
deleted local-overlay recipe file, and a real package's own xvm record
pointed at a missing bin dir) and proves none of it moves a sibling
package's payload tree or this subos's shim table.
Verified: mcpp build && mcpp test (54/54 unit tests); the new
broken_home_isolation_test.sh and the existing doctor/list/overlay e2e
tests all green. self_doctor_multi_subos_test.sh's pre-existing S4 failure
was confirmed unrelated by reverting this diff and reproducing it against
HEAD unchanged.
…d is pruned not downloaded, and --fix walks every subos D1: cmd_doctor now always builds the package catalog and always resolves a BrokenPayload's remedy, instead of gating both behind --deep/--fix. A plain `self doctor` used to say "no package in any index provides this entry" for releases the index plainly does provide -- measured on a real 111-subos home: 100 entries (fd@10.4.2, go@1.26.2, ...) that --deep alone resolved correctly, at zero extra network cost (the catalog is a local index read; only the payload walk stays --deep-gated). Scan::probeAvailable lets the report say "index unavailable" instead of "no package provides this" when the catalog itself could not be built. D2: Finding::unclaimed marks a BrokenPayload no subos anywhere references and that isn't this subos's active pick. repair_payloads_ now prunes such an entry's registration instead of reinstalling it, even when the index can still resolve it -- 65 of 107 broken payloads on the measured home were old mcpp versions nothing pointed at, and the ladder would have re-downloaded every one. prune_one_ is extracted so the payload ladder and the dead-registration rung share one victim-dropping body. `--subos <NAME>` anchors a doctor run to a specific subos (validated against Config::list_subos_names(), InvalidInput otherwise); `--show-ok` replaces `--all` as the documented flag for listing non-defect findings (`--all` keeps working, with a deprecation warning). `--fix` now walks every subos a ForeignPayload/OtherSubos finding names, in its own `self doctor --fix --subos <name>` subprocess, so the user no longer has to `xlings subos use <name>` by hand first -- the remedy 38 findings carried on the measured home. Guarded against recursion: a run invoked with --subos never walks further. Two new e2e (doctor_remedy_mode_parity_test.sh, doctor_cross_subos_fix_test.sh) and an extended doctor_fix_convergence_test.sh cover the new behaviour. self_doctor_test.sh and self_doctor_multi_subos_test.sh are updated where they encoded the old, now-reversed assumptions (quick doctor must not resolve a remedy; a foreign payload is never touched by this subos's --fix) and where a stale `.xlings.json["version"]` read predated the verifiedBy stamp Task 2 introduced.
…t up
Task 8 review found injection 3 (a config --add-xpkg recipe whose file is
deleted afterward) was never read by any command the test ran -- the
overlay's provenance is only reachable from `config --list-xpkg`/
`--remove-xpkg`/`--clear-xpkg` and `update`, while the catalog's normal
scan globs pkgs/*/*.lua directly and never notices the missing file.
Adds a second, untouched overlay entry (isolocalhealthy) alongside the one
whose file gets deleted, and runs `config --list-xpkg` against the wounded
home: asserts it survives (exit in {0,1,2}, no crash signature) and that
the untouched sibling entry still lists, without pinning what (if
anything) the broken row itself renders as -- this codebase's
overlay::Status has no "missing" variant, confirmed by tracing
load_with_files/status_of, so there is nothing there yet to assert on.
Also collapses profile.cpp's three repeated
`if (unreadable) unreadable->push_back(name);` into one mark_unreadable
lambda.
Verified: target/ was rebuilt from scratch (mcpp build && mcpp test,
54/54 unit tests); broken_home_isolation_test.sh, local_overlay_test.sh,
and doctor_fix_convergence_test.sh all green.
…sability-2026.9.12 # Conflicts: # tests/e2e/run_all.sh
`mcpp test --profile asan` builds the code under test and the gtest binaries with -fsanitize=address,undefined. Measured on this machine with the packaged gcc 16.1.0: build 1073 s, 54/54 suites pass, libasan.so.8 / libubsan.so.1 resolve from the toolchain payload. Issue #433 is why: a use-after-free reached three releases because nothing ever ran the tests instrumented.
…subos that pins it (#586) The effect-placement loop in process_xvm_operations_ only ever wrote into the current subos, so a registration rewrite (an install that re-runs a recipe's config() for an already-registered target@version and changes what it points at) left every other subos pinning that exact version with a sysroot link aimed at wherever the OLD registration named -- dangling once that location is gone. Measured on a real home: freetype's links were valid in 4 subos and dangling in 3. Extract the per-effect-kind placement loop into place_effects_into(), called once for the current subos exactly as before, then again for every other subos profile::find_subos_pinning_version() returns, reading each one's own workspace file directly rather than going through Config's active-subos override -- that override's own reload re-reads the version DB from disk, clobbering the in-memory batch this same install just registered, before it's saved. Adds E2E-105 (sysroot_refresh_pinning_subos_test.sh): a registration rewrite in one subos refreshes another pinning subos's sysroot with no dangling symlink, while a subos that never had the package gets nothing pushed to it.
Add a hard-gated `unit-asan` job to xlings-ci-linux.yml that runs `mcpp build --profile asan` / `mcpp test --profile asan` against the [profile.asan] toolchain (measured locally: 54/54 green, ~18 min build), with its own mcpp cache key so the instrumented BMI set can never mix with the dev profile's. Add a "Platform core contract" step to xlings-ci-macos.yml after the SubOS sandbox contract step, running notice_once_test.sh, remove_force_contract_test.sh, doctor_cross_subos_fix_test.sh, and broken_home_isolation_test.sh in order. doctor_cross_subos_fix_test.sh ships on a sibling branch of this same round not yet merged here; it is referenced by name on purpose so the step starts exercising it the moment that branch lands. Windows gets no new bash e2e (documented decision, ps1 port deferred).
docs/quick-start/multi-version.md: new "本地包索引(--add-xpkg)" section covering --list-xpkg / --remove-xpkg <NAME> / --clear-xpkg <all|stale> and that `xlings update` auto-removes overlay copies byte-identical to the freshly synced index; new "跨 SubOS 卸载" section documenting `remove`'s --all / --all-subos / --subos <NAME>. docs/quick-start/self-management.md: note `self doctor --subos <NAME>` and `--show-ok` (not yet on every client -- `--help` confirms support). docs/generated/command-reference.md regeneration is deferred to the controller: this worktree is missing flags that only exist on branches of this round not yet merged here. .agents/docs/2026-09-12-robustness-and-usability-optimization-plan.md: §6 gains an "实施" column recording where each phase landed; §8 gains two implementation-time findings -- the multi-subos doctor e2e's stamp helper was pinning a writer (it read the old `version` key) rather than an invariant, and doctor's default report gated its remedy probe on --deep for a 2s saving and paid for it with ~100 false "no remedy" lines on a real home.
Review finding 1: self-management.md hedged `self doctor --subos <NAME>` and `--show-ok` as only present on "some newer clients" when both ship in this same 2026.9.12.1 release (Task 6, sibling branch). Reworded to "自 2026.9.12.1 起", kept both flags documented, and documented `--all` as the deprecated spelling of `--show-ok` so the deprecation is discoverable from the doc itself. Review finding 2: the unit-asan job's comment claimed to mirror build-and-test's steps exactly but omitted "Record the restored BMI set (diagnostic for #56)" and "Diagnose a failed mcpp build" -- so a cache/index failure in the asan job had less visibility than the same failure in the dev job. Added both verbatim; neither references a cache key literal (only steps.mcpp-cache, which resolves to this job's own cache step, and generic ~/.mcpp paths), so nothing needed adapting for the asan cache key beyond what unit-asan's own cache step already sets. Re-validated: both workflow YAMLs parse via `ruby -ryaml`; rebuilt the worktree binary (2026.9.12.1) and reran test_docs_examples.py, test_generated_command_reference.py, and test_cli_spec_parity.py, all green.
Review follow-up on the cross-subos --fix walk (2026.9.12.1): - RepairReport::failedSubos (name, exit code) records a cross-subos child (`self doctor --fix --subos <name>`) that exits non-zero. Folded into the existing `outstanding` gate at the end of cmd_doctor, so a failed child now makes the parent return 1 and withholds `record_verified_version` exactly like an outstanding failedEntries victim does -- previously the failure was recorded as a note only, with no effect on the exit code or the stamp. - The interpolated subos name is checked with is_shell_safe_token before the walk builds a command from it, matching every other shelled-out value in this file. A name that fails the check is never executed -- recorded as a failed subos with a hand-run remedy instead. The same check already refuses a leading '-', which is also what would otherwise make the child's own --subos parser misread the name as another flag. - Extracted the inline walk_other_subos lambda into a top-level repair_other_subos_walk_(), following this file's repair_*_() convention; the !subos recursion guard moved to the two call sites. - Dropped a redundant `&& !activeHere` term from Finding::unclaimed: owner.ownedHere's first disjunct (subos_claims) is activeHere verbatim, so !owner.ownedHere already implies !activeHere. New e2e scenario (I5, doctor_cross_subos_fix_test.sh): a read-only subos/other/ directory makes the child's own reinstall attempt genuinely fail (config() cannot write) without going through paths that would reclassify the finding away from BrokenPayload, so the child's own re-detect still finds it broken and both layers correctly withhold the stamp. Asserts exit 1, verifiedBy absent, and the report naming the failed subos and the exact child command.
…ead (review round 1) The cross-subos refresh loop from the previous commit used the existing, forgiving load_workspace_file_, which returns an empty SubosWorkspace for both "genuinely has nothing" and "could not be read" -- so a pinning subos whose .xlings.json is unreadable at the moment of the refresh would be silently skipped, keeping stale (and per #586, possibly dangling) links with nothing said about it. Add load_workspace_file_checked_, which distinguishes the two via std::optional, and warn-and-continue on nullopt: names the subos, the target@version, the file, and the runnable repair (`xlings self doctor --subos <name>`). One subos's broken file no longer blocks or silently swallows the others' refresh. Also drops the always-discarded installed[] parameter from place_effects_into (no effect kind placed there reads it). Adds e2e Scenario 5. Note left in both the test and the task report: find_subos_pinning_version's own scan (load_subos_snapshots) already skips a subos whose file fails to parse -- by explicit, documented design, so an unrelated `remove` isn't taken down by one hand-edited file -- so it never names a subos in that state, and the new warn line is reachable only by a same-process TOCTOU race a black-box e2e test can't arrange. Scenario 5 instead asserts the part that IS testable and is the substance of the ask: a corrupted pinning subos must not block, crash, or widen the blast radius of a registration rewrite happening for the others.
…sability-2026.9.12 # Conflicts: # tests/e2e/run_all.sh
…sability-2026.9.12 # Conflicts: # tests/e2e/run_all.sh
…ithout a file is "missing", and a scope restore never escapes a destructor Three small, reviewed-and-deferred findings: - `cmd_remove_xpkg` resolved a name only through `recipe_path(dir, name)`, so it could not delete an untracked overlay recipe (no `.overlay.json` entry) that `--list-xpkg`/`--clear-xpkg` already reach via `load_with_files`. It now looks the name up through a new `overlay::find_entry(load_with_files(dir), name)` seam and only drops the provenance entry when the found entry was tracked. - `overlay::Status::Kind` had no way to say "tracked entry whose file is gone" -- `status_of` degraded to Unique off an empty sha. Added `Missing`, rendered as "missing" by `--list-xpkg`; `gc_identical` and `--clear-xpkg stale` now treat Missing the same as provenance garbage and drop the `.overlay.json` record (no file to delete). - `ScopedSubosOverride::~ScopedSubosOverride()` is implicitly noexcept but calls three functions that can throw during restore; wrapped the body in try/catch so a restore failure during unwinding logs instead of calling std::terminate. Unit tests: OverlayStatusTest.Missing*, OverlayFindEntry.*, OverlayGc.RemovesAMissingTrackedEntry (tests/unit/test_xim_overlay.cpp). E2E: local_overlay_test.sh S5c (remove-xpkg on an untracked recipe). broken_home_isolation_test.sh's tracked-entry-with-deleted-file scenario stays green unchanged.
…sability-2026.9.12
…-a37e2fa8340c40f3d
…e payload Completes the change started in the prior commit (fc6b9a8): adds the e2e regression scenario (remove_force_contract_test.sh S8) proving that a sibling subos whose .xlings.json cannot be read blocks a full removal (detach-only instead, payload retained), names the unreadable subos in the report, and is NOT overridden by --force -- deleting a payload a different, unrelated subos may still need is not what --force on this removal means. S8: two subos both install `plain`; `other2`'s workspace file is corrupted; `remove plain -y` and `remove plain --force -y` in `default` both exit 0, keep the payload on disk, and name `other2`; fixing the file lets `remove plain -y` in `other2` clean everything up.
`cmd_doctor`'s outstanding-count (`stillFound`, ~doctor.cpp:5161) only matched BrokenPayload/ForeignPayload findings when deciding whether a ladder failure still counts. An IncompletePayload finding that survives its own repair attempt (`repair_incomplete_` re-runs install(), the failure marker stays on disk, re-detected on the very same re-scan) fell through that gate: a home whose only remaining defect was a failed reinstall could still stamp `verifiedBy` and report success in the summary while `self doctor --fix` returned 1 -- a straight contradiction between the stamp and the exit code. Fix: IncompletePayload now also counts toward `stillFound`. Six-line diff. Verified by reverting it, rebuilding, and confirming the new e2e scenario failed exactly as expected (verifiedBy WAS written despite the failed reinstall); restored, reran, passes. e2e: install_silent_failure_test.sh gained S4, reusing its existing `brokenpkg` fixture (install() always returns false) -- after the CLI install fails and leaves an `incomplete: true` stamp, `self doctor --fix -y` re-attempts the same failing install and must exit non-zero without stamping verifiedBy. Also rebuilds doctor_cross_subos_fix_test.sh's I5 (E2E-104), whose read-only-subos-directory mechanism stopped being a deterministic child failure once a concurrently-merged repair ladder (Task 5: remove --force then reinstall) started withdrawing the broken registration before the read-only directory could block anything -- re-detection then found nothing left to be broken, and a pruned/unclaimed entry with nothing left to find is doctor's pre-existing, intentional "not outstanding" case, kept unchanged here. I5 now uses a subos directory created by hand whose NAME fails `is_shell_safe_token` (leading '-'), with a workspace claiming the broken payload's exact target@version the same way a real owner's file would: `repair_other_subos_walk_`'s existing shell-safety guard puts it straight into failedSubos without ever invoking a subprocess, which is deterministic by construction. Asserts: parent exit 1, verifiedBy not written, report names the offending subos and says it was never run (not a safe shell token) rather than that it failed. The separate, non-rc-gated smoke that a real cross-subos child subprocess still runs lives in I3 (asserts the "repairing subos other" announce line).
…isk payload Task 7's report (task-7-report.md, "surfaced defect") described `xlings install <target>@<version>` in a SECOND subos, for a payload some other subos already has on disk, taking a shortcut straight to `xvm cmd_use` for activation -- refused because the shortcut never wrote the target into that subos's own installed[] first. Reproduced the exact two-subos shape against current HEAD (2026.9.12.1) twice, once with a trivial fixture and once with a bindir+lib+header package (the same shape sysroot_refresh_pinning_subos_test.sh uses): both times it does NOT reproduce. `install`'s plan resolution is per-subos-aware -- a target this subos does not yet have is still a pending node even when the shared payload exists on disk -- so the executed node skips only the now-redundant install() hook and re-runs config() for the current subos, registering it into that subos's own active/installed[] and materializing its shim + sysroot links exactly as a fresh install would. This appears to be an incidental side effect of other work merged into this same round after the task-7 report was written; not bisected since no further action was needed. No fix commit accompanies this one because there is nothing to fix. Adds a permanent regression guard instead: second_subos_install_test.sh (E2E-107 in run_all.sh) -- install libfixture@1.0.0 in default, `subos new other`, install the same coordinate in other; asserts exit 0, other's own workspace has it active+installed, and other's shim/lib symlink/header all exist.
…, never to an alias (#590) Two layers, one symptom. `semver::parse` rejected '+' as a field character, so Temurin's real key `25.0.4+7` satisfied no range while its alias `25.0.4` (`{ ref = "25.0.4+7" }`) did -- and `select_version_` handed every key to the range matcher, so `xim:jdk-temurin@>=11` resolved to the alias string, `dep_install_dir` joined it to the store root, and the consumer's wrappers baked a JAVA_HOME with no bin/java under it. Build metadata now carries no precedence (SemVer 10), and range / no-hint selection considers only concrete versions with aliases followed to what they name; the exact-match branch already did. Unit tests pin both halves. The removal half of the issue is covered by 2026.9.12.1's DB-first remove resolution, which no longer asks the index which version to remove.
macOS CI (clang 20 / libc++) rejected three spellings gcc accepted: `std::chrono::clock_cast` (not shipped by libc++), an `it != end` comparison of recursive_directory_iterator from a module implementation unit (the range form fails the same way; profile.cpp's `std::default_sentinel` does not), and a `const char*` format argument that clang resolved to the wide formatter. Verified with `mcpp build --toolchain llvm@20.1.7` and the full unit suite under that toolchain (54/54), then gcc again.
…thholds the stamp R3's terminal outcome -- `remove --force` genuinely drops a broken registration and the reinstall meant to put it back also fails -- was folded into `failedEntries`, which only counts against `outstanding` when re-detection still finds the (target, version) as a finding. A fully removed registration is never a finding again, so `outstanding` read 0, `--fix` stamped `verifiedBy`, and exited 0 having taken a package out and left it out. `RepairReport::removedNotReinstalled` records this outcome unconditionally (mirroring `failedSubos`) and folds into `outstanding` the same way, gating both the exit code and the stamp (F1, 2026.9.12). D2's unclaimed check also now treats any unreadable subos as a possible claimant -- the same controller ruling `remove` and the installer refresh already apply -- so nothing is routed to D2's prune while a sibling subos cannot be read; the prune note says why when this applies (F2). Building F1's e2e surfaced a real, separate bug: `cmd_remove_resolved_`'s DB-record fallback derived its "is there still a record" check by stripping only a namespace from the ORIGINAL argument, never a version -- so a bare `remove <name>` worked by accident (that argument never had a version to leave attached) while the repair ladder's own commands (always the full `ns:name@version` coordinate) looked up a DB target literally named `"name@version"`, found nothing, and treated a genuinely broken, still-registered package as "not installed" -- a silent no-op on `remove --force`, the one case it exists to clean up. Fixed to strip version then namespace from the resolved coordinate, matching the pattern used elsewhere in this file. New e2e: tests/e2e/doctor_removed_not_reinstalled_test.sh (E2E-108) -- a package that installs once for real then fails forever; deleting its whole payload directory forces a genuine reinstall attempt on every retry. Extends doctor_cross_subos_fix_test.sh (I9b) for F2, and adds a regression scenario (S4b) to remove_force_contract_test.sh for the namespace+version bug.
… may have broken
`remove --force` in R3 bypasses `cmd_remove`'s own reverse-dependency
guard by design -- the ladder already decided the removal has to
happen -- but bypassing the question is not the same as it having no
answer. `direct_dependents_of` (extracted from `cmd_remove`'s own guard
into a shared function, xim/commands.cppm) is now asked once, only for
R3's terminal "REMOVED but could not reinstall" outcome, via an
injected `DependentsProvider` (repair.cppm/repair.cpp) so the decision
table stays unit-testable without a catalog. `RepairResult::dependents`
carries the answer to doctor's report, which lists them as "may be
broken" (F7, 2026.9.12).
This also carries the commands.cpp fix from the previous commit: the
same one-line `bareTargetName` bug that made a forced removal on a
namespaced, versioned coordinate silently no-op is what R3 shells out
with, so both are exercised by the same e2e.
Unit tests in tests/unit/test_self_repair.cpp cover the provider being
asked only for that one outcome, never for a healthy re-register, a
successful reinstall, or the OTHER "!healed" shape from R3 ("remove
--force did not drop it").
sysroot_refresh_pinning_subos_test.sh landed on this branch; the macOS platform-core-contract step referenced it only by name, with a comment saying it lived on a sibling branch not yet merged. It is merged now -- add it to the step and drop the stale comment (F3, 2026.9.12).
…ile are all written safely Three read-modify-write helpers on `~/.xlings.json` (mark_hint_seen, record_client_version, record_verified_version) now go through a shared, lock-protected RMW under a short (~2s), XLINGS_LOCK_TIMEOUT- independent timeout: these fire on nearly every command now (the upgrade notice, doctor's stamp), and none of them is worth making a `list` wait behind an install for. A lock that cannot be taken in time means the write is skipped, not retried and not escalated -- a repeated note is acceptable, a lost version write is not. Re-entrant via `acquire_state_lock`'s own env-var marker, so a caller that already holds the lock (doctor stamping at the end of its own repair pass) never double-waits (F4, 2026.9.12). `Config::save_workspace()` -- the writer of the CURRENT subos's own `.xlings.json`, reached from ordinary install/remove/use -- used to read a file that failed to parse and silently fall back to a blank `nlohmann::json` object, then write that object back: a corrupted-but- otherwise-intact manifest (subos_info, envs, anything else) was blanked by the very first write after the corruption. `profile:: save_subos_workspace` already refuses this for every OTHER subos's file (doctor's cross-subos repairs write through it); this makes the CURRENT subos's own writer keep the same promise (F10, 2026.9.12).
The loop that assigns `cmd` only ever picks a non-flag argument (one that does not start with `-`), so `cmd == "-h"`, `cmd == "--help"`, and `cmd == "--version"` could never be true -- dead code on every input. Removed; the reachable case (`cmd.empty()`, only flags typed) already scans `fargv` itself for `-h`/`--help`/`--version` and is unchanged (F11, 2026.9.12).
`is_version_referenced_anywhere_` returned on the FIRST subos whose workspace file could not be read, warning about only that one name -- while the separate `pinned_by` report (find_subos_pinning_version, cmd_remove_resolved_) already lists every unreadable subos it saw. The decision-time diagnostic and the follow-up report disagreed about how many subos were even in question. Now collects every unreadable subos across the whole scan before deciding (a real match anywhere still short-circuits), and the one warning it prints names all of them, each with its own runnable `xlings self doctor --subos <name>` (F9, 2026.9.12). Extended remove_force_contract_test.sh's S8 with a second corrupted sibling subos, asserting both names appear.
`concrete_versions_` followed exactly one `ref` hop: an alias chain
("8" -> ref "8.0" -> ref "8.0.1", the real key) left ">=8" resolving to
"8.0" -- itself just another name, never a directory the store
created. Now follows the chain to a fixed point, bounded at 8 hops and
cycle-safe (a hand-edited or malformed index that points two aliases at
each other fails closed instead of looping) (F6, 2026.9.12).
Unit test in tests/unit/test_xim_catalog.cpp: a three-link chain
resolves `>=8` to the real, concrete end.
Companion unit test for the concrete_versions_ fix: "8" -> "8.0" -> "8.0.1" (real) resolves `>=8` to `8.0.1`, not the middle alias link.
Companion unit tests for F7's dependents provider: names the injected
dependents on the "REMOVED but could not reinstall" outcome, and
confirms it is NOT consulted for a healthy re-register, a successful
reinstall, or the other "!healed" shape ("remove --force did not drop
it") -- and that a caller supplying no provider gets the pre-existing
behaviour (no dependents reported).
…is pruned past it I9b in doctor_cross_subos_fix_test.sh (F2): with the same otherwise- unclaimed entry I9 already builds, adding an unreadable sibling subos must make D2 route the entry to reinstall instead of prune. Also fixes the fixture's own xpm table to actually declare the version I9/I9b inject by hand, so the coordinate genuinely resolves through the catalog -- without it, the entry took the separate "unowned" prune path regardless of unclaimed, and neither scenario exercised D2 at all.
…ys fast Scenario 6 in home_config_lock_test.sh (F4): holds the state lock via flock while running `list` on a real pty (the upgrade notice is TTY- gated); asserts `list` exits promptly (well under the 10-minute XLINGS_LOCK_TIMEOUT default, proving the short, independent budget took effect) and the config file is byte-identical, since the notice's own lock-protected write must have been skipped rather than waited on.
S9 in remove_force_contract_test.sh (F5): a package only in `other`; from `default`, `remove plain` (no -y) stays put and offers `--all-subos`, `remove plain -y` implicitly reaches `other` and removes it there. Also in this file: S4b, a regression guard for the `bareTargetName` namespace+version bug fixed alongside F1 (remove by full `ns:name@version` coordinate after the payload is gone must actually remove, not silently no-op as "not installed"); and S8's second corrupted sibling subos for F9 (every unreadable subos is named, not just the first).
… state S6 asserted the shared DB entry survives the FULL --fix run, including the cross-subos walk into `other`. That was only ever true because two bugs combined to prevent `other`'s own, later, independent repair attempt from ever completing (the F1 bareTargetName fix, and a second bug found alongside it: `repair_incomplete_` shelled a bare `xlings` rather than the running client's own absolute path, so its reinstall attempt silently failed to resolve in a home with no shim on PATH). With both fixed, `other` -- correctly, once it is the release's last remaining claimant -- can finish the removal `default`'s own ladder rightly declined to claim. S6's actual property (default's own R3 must not claim "REMOVED" for a detach) is unchanged and still asserted from the message text; the DB-survives check is replaced with checking default's own workspace shows the detach it printed. S8b/S8c (a separate, unrelated finding in `other`) are green again now that `other`'s own --fix can actually run its reinstall.
E2E-108 doctor_removed_not_reinstalled_test.sh (F1): a package that installs once for real then fails forever; deleting its whole payload directory forces a genuine reinstall attempt on every retry, reaching R3's "REMOVED but could not reinstall" through the real CLI end to end. E2E-109 current_subos_unreadable_test.sh (F10): the CURRENT subos itself corrupted (not a sibling) -- list/install/remove/doctor survive, a sibling subos's file is untouched, and doctor reports it unreadable. E2E-110 doctor_cross_subos_child_timeout_test.sh (F8): a cross-subos `--fix` child whose install() hook hangs is killed within the configured XLINGS_DOCTOR_CHILD_TIMEOUT bound and reported as a timeout, not waited on.
…ts, and the table the real machine fills
This was referenced Sep 12, 2026
Sunrisepeak
added a commit
to openxlings/xim-pkgindex
that referenced
this pull request
Sep 12, 2026
…local is an overlay) (#830) Applied by .github/scripts/version-check.py --apply --only xlings against the released assets; all three platform tables gain the entry and `latest` moves. Release: openxlings/xlings#591. Co-authored-by: sunrisepeak <x.d2learn.org@gmail.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.
一句话
四条痛点(提示太吵、
local:总和发布包冲突、删不掉/doctor 要先进 subos、一个坏包拖垮别的)是三个结构性原因的四张脸,这个 PR 按.agents/docs/2026-09-12-robustness-and-usability-optimization-plan.md的 P0–P5 一次落地,外加 #590。版本 2026.9.12.1。改了什么(按用户能看到的)
use/install/list/self update不再每条命令都印 "run xlings self doctor --fix";升级后新客户端提醒一次(落盘 memo);.xlings.json新增verifiedBy,由doctor --fix写;doctor 报告只保留一行事实。命名空间降级(local输给xim)在两者版本相同时不说,不同时说一次。local变成带来源的覆盖层。config --add-xpkg记 provenance,与已同步索引字节相同的副本不入库;xlings update自动清掉字节相同的旧副本;新增--list-xpkg/--remove-xpkg <NAME>/--clear-xpkg <all|stale>,老版本加进来的无 provenance 文件同样可见可清。真机 159 个里 157 个会消失。remove无论如何都能删干净。 按已装记录解析而不是索引 latest(remove: a versionless removal with exactly one stored version never resolves, and the name resolves to the index's latest rather than the installed version #578);recipe 缺失跳过 hook;hook 失败先撤状态再报错(--force下退出 0);新增--all、--all-subos、--subos <NAME>;一个不可读的 subos 被视为"可能在用",只 detach 不删载荷,--force不越权。--fix不用进 subos。 默认模式也做索引探针(D1:真机 100 条假的 "no package in any index provides" 归零,+2 s);没人引用的坏载荷 prune 而不是重装(D2:真机 65 个旧 mcpp 不再下载);--fix对每个有 finding 的 subos 起子进程--subos <name>,子进程失败父进程退 1 且不打戳;--all→--show-ok(保留别名);IncompletePayload 也阻止打戳。list/install/use/shim/doctor不变。dep_install_dirbakes a dependency's alias key instead of its dereferenced version #590:semver::parse接受 build metadata(25.0.4+7),范围解析只在具体版本里选、别名一律解引用。unit-asan硬门禁([profile.asan],本地 54/54)。Windows 本轮不加 bash e2e(需要 ps1 port,记入下一轮)。不变量(测试锁的)
I1 同一 home 任何命令跑两遍第二遍不重复 Notice;I2
--fix退出 0 后不再提示;I3--fix两遍第二遍 planned 为空(多 subos);I4remove X --force -y后 DB/workspace/shim/store 四处不见,与 recipe/hook/载荷状态无关;I5 一个坏条目不改变其它包的结果;I6 同版本本地副本不触发警告;I8 默认与--deep的 remedy 相同;I9 未引用坏载荷不发起下载。新增 e2e:E2E-100 … E2E-107;单测 54/54;mcpp test --profile asan54/54。真机数字(
~/.xlings,111 subos,只读跑)self doctor说"没有索引提供"的条目--deep一致)--fix计划下载subos use X我替维护者做的判断(需要 review 的)
--force(它的 remove 是为了重装;由 verifier 决定报告)。remove --force容忍 hook 失败;但不越权删除一个不可读 subos 可能在用的载荷。--fix失败 ⇒ 父进程退 1 + 不打戳(计划原文只说"记 notes",按 spec 的 silent-success 规则改)。配套:openxlings/xim-pkgindex#826(测试循环文档加第 8 步)。
整分支 review 之后追加的一波(同一 PR)
--fix退 1 且不打戳,并列出可能被弄坏的直接依赖者。hintsSeen、version、verifiedBy)以及当前 subos 的 workspace 写入都在状态锁下进行(2 s 超时,拿不到就跳过);不可解析的状态文件永远不被覆盖。XLINGS_DOCTOR_CHILD_TIMEOUT覆盖;POSIXtimeout(1),没有时告警并按旧行为不限时)。remove对每个不可读 subos 都点名;隐式-y跨 subos 扩展有了契约测试(E2E-108..110 新增)。cmd_remove的命名空间+版本剥离让--force删除静默 no-op;repair_incomplete_用的是 PATH 上的xlings而不是当前客户端。cli.cpp两处死比较删除;macOS 平台契约补跑 sysroot 刷新测试。本地全量 e2e(release tarball):114/115,唯一一条(E2E-48 N8,pty 确认)用上一版发布二进制同样失败,是本机环境,CI 同一清单为绿。
发布后另开 issue(不在本 PR)
Windows 上新 bash e2e 的 ps1 移植;
interface/agent 协议对新动词(--all-subos、--list-xpkg等)的对等支持;XLINGS_LOCK_TIMEOUT恢复时置空而非取消设置(无可观察影响)。