Skip to content

fix(doctor): report foreign-installation skill packages as info, not update-nag#411

Merged
ScriptedAlchemy merged 1 commit into
masterfrom
codex/doctor-foreign-skills
Jul 10, 2026
Merged

fix(doctor): report foreign-installation skill packages as info, not update-nag#411
ScriptedAlchemy merged 1 commit into
masterfrom
codex/doctor-foreign-skills

Conversation

@ScriptedAlchemy

Copy link
Copy Markdown
Owner

Problem

tracedecay doctor's managed-skill materialization check classified every inactive materialized package in a project scope as Orphan and told the user to run tracedecay update to remove it. But the update/remove path (remove_materialized_skillmay_remove_owned) already refuses to delete a project-scope package this installation did not author — a package committed by another installation (manifest materialized_by differs) or a legacy manifest with no materialized_by at all. So doctor emitted a permanent warning nag prescribing a remediation that never runs.

Concretely, in this repo the committed .claude/.codex skills automation-run-review and scheduler-review (manifests with materialized_by: null) were reported as two warnings each cycle, telling the user to run an update that would skip them as foreign.

Fix

Route doctor and the remove path through one shared predicate, package_is_foreign_to_installation, so they classify identically by construction and doctor can never again prescribe an action update refuses to perform. may_remove_owned is now defined as its negation.

  • New SkillDrift::ForeignOrphan variant, rendered at info severity (DoctorCounters::info) — indented, no ! marker, does not increment the warning count, never names tracedecay update. Message: "'' project skill from another installation; leave in place, or delete the directory manually if unwanted ()".
  • Self-authored inactive packages still surface as plain Orphan with the update remediation (correct — update removes them).
  • Global (home) packages are never foreign.
  • The doctor render logic is extracted into a pure skill_drift_report(scope_desc, finding) -> (DriftLevel, String) classifier so severity/message is unit-testable.

Behavior of reconcile_scope / remove_materialized_skill is unchanged; this only aligns doctor's reporting with their existing protection.

Tests

  • doctor_reports_foreign_installation_package_as_foreign_orphan — committed package authored by another installation → exactly one ForeignOrphan, no Orphan, and remove_materialized_skill agrees with SkippedForeign.
  • doctor_reports_legacy_manifestless_author_as_foreign_orphan — legacy manifest with materialized_by stripped, and missing-manifest case → ForeignOrphan.
  • doctor_reports_own_inactive_skill_as_plain_orphan — self-authored inactive package stays Orphan.
  • Lib unit tests foreign_orphan_renders_as_info_without_update_remediation and plain_orphan_still_warns_with_update_remediation on the pure classifier.
  • Existing doctor_scope call sites updated with the installation-id arg.

Verification

  • cargo fmt --check, cargo clippy --workspace --all-targets --locked -- -D warnings — clean.
  • cargo nextest run --test agent_suite --no-fail-fast — 501 passed.
  • Skill-materialization + doctor lib tests — all pass.
  • Live: built binary's tracedecay doctor against this repo now emits the four packages ({automation-run-review,scheduler-review} × .claude/.codex) as info-level lines, no warning-count increment, no update prescription.

🤖 Generated with Claude Code

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 3535097

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Doctor's orphan classifier prescribed `tracedecay update` for every
inactive materialized skill package, but update/remove refuses to delete
a project-scope package this installation did not author (a committed
manifest with a different `materialized_by`, or a legacy manifest with
none). The result was a permanent warning nag for a remediation that
never runs.

Route doctor and the remove path through one shared predicate,
`package_is_foreign_to_installation`, so they classify identically by
construction. Foreign packages now surface as a new `SkillDrift::
ForeignOrphan` rendered at info severity ("project skill from another
installation; leave in place, or delete the directory manually"),
never incrementing the warning count and never naming `update`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ScriptedAlchemy ScriptedAlchemy force-pushed the codex/doctor-foreign-skills branch from 717b856 to 3535097 Compare July 10, 2026 00:45
@ScriptedAlchemy ScriptedAlchemy merged commit e0b3cc3 into master Jul 10, 2026
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant