Skip to content

install_skills.py: scope the failed-hash rollback to the installed agents (#405) - #407

Merged
seathatflowsinourveins merged 2 commits into
mainfrom
claude/skills-rollback-agent-scope-20260927
Sep 27, 2026
Merged

install_skills.py: scope the failed-hash rollback to the installed agents (#405)#407
seathatflowsinourveins merged 2 commits into
mainfrom
claude/skills-rollback-agent-scope-20260927

Conversation

@seathatflowsinourveins

@seathatflowsinourveins seathatflowsinourveins commented Sep 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Fixes #405.

tools/adoption/install_skills.py installs each pinned skill for two agents (add ... -a claude-code codex). Its failed-hash rollback ran skills remove <name> -g -y with no -a. In skills v1.7.0, remove without -a selects every known agent (vercel-labs/skills src/remove.ts#L209), so a rollback could delete a same-named skill that another agent owns. adoption/lifecycle.md:100: "Never uninstall all user tools to roll back one package."

Change:

  • add and the rollback remove now take their agents from one SKILL_AGENTS = ("claude-code", "codex") tuple, with -a last, because the CLI reads trailing words as agent names.
  • tests/test_install_skills.py: the mismatch-rollback test asserts the remove call's exact arguments and that its agent list equals add's. With the old unscoped call the test fails (checked by reverting the one line); with this change it passes.
  • manifests/evidence.json: the two files are re-registered.

Checks:

  • python3 -m unittest tests.test_install_skills: 19 OK.
  • tests.test_skills_manifest and tests.test_skills_status: OK.
  • validate.py: passed (7323 hashed files).
  • git diff --check: clean.

Found by the GPT-6 cross-family review of #391, whose rollback text already uses the scoped form.

SOTA sources

🤖 Generated with Claude Code

https://claude.ai/code/session_017d27CMzkbLezdLv7vAStPu

@seathatflowsinourveins seathatflowsinourveins added the lane:foundation Foundation lane: Claude/Codex setup, hosts, memory, RAG, research, workers label Sep 27, 2026
@seathatflowsinourveins seathatflowsinourveins mentioned this pull request Sep 27, 2026
Merged
6 of 8 tasks
@seathatflowsinourveins

seathatflowsinourveins commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator Author

GPT-6 cross-family review of 09c77463: approve. (codex exec -s read-only -m gpt-6-astra, reasoning effort max)

  • P3, nonblocking wording — install_skills.py:62: The rationale implies -a must come last. Both upstream parsers stop collecting agents at the next flag: remove, add. Executing the installed v1.7.0 parsers confirmed both orders work. Describe final placement as a convention; the generated commands are valid.

  • P3, nonblocking documentation — install_skills.py:21: The module docstring still describes the old unscoped rollback, contradicting the executable call at line 201. Update the example to skills remove <name> -g -y -a claude-code codex.

Verification: both commands use the same SKILL_AGENTS tuple. No other executable unscoped skills-removal path was found in tools/adoption/. Upstream confirms explicit agent selection and retention of canonical content and its lock entry while another detected agent uses it. Removal implementation

All 83 requested tests passed. In a memory-backed scratch copy outside the checkout, restoring only the old rollback line failed precisely at the new assertion on line 317; the fixed copy passed. Only the two expected evidence entries changed, with matching hashes and sizes. validate.py passed with 7,323 hashed files; the checkout remains clean.

The two P3 items are nonblocking wording.

🤖 Generated with Claude Code

@seathatflowsinourveins @claude Scout
install_skills.py: scope the failed-hash rollback to the installed ag…
b5884be 
…ents (#405)

The installer adds each pinned skill with `-a claude-code codex`, but its
rollback ran `skills remove <name> -g -y` with no `-a`. In skills v1.7.0
that selects every known agent (vercel-labs/skills src/remove.ts#L209), so
a rollback could delete a same-named skill another agent owns. Both calls
now take the agents from one SKILL_AGENTS tuple, `-a` last because the CLI
reads trailing words as agent names.

The mismatch-rollback test now asserts the remove call's exact arguments
and that its agent list equals add's; it fails with the unscoped call
(checked) and passes with this change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017d27CMzkbLezdLv7vAStPu
@seathatflowsinourveins @claude Scout
#405: re-register the two changed files in manifests/evidence.json
c170296 
tools/adoption/install_skills.py and tests/test_install_skills.py are
hash-registered; validate.py passes (7323 hashed files).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017d27CMzkbLezdLv7vAStPu
@seathatflowsinourveins
seathatflowsinourveins force-pushed the claude/skills-rollback-agent-scope-20260927 branch from 09c7746 to c170296 Compare September 27, 2026 19:14
@seathatflowsinourveins
seathatflowsinourveins merged commit 0f76651 into main Sep 27, 2026
24 checks passed
@seathatflowsinourveins
seathatflowsinourveins deleted the claude/skills-rollback-agent-scope-20260927 branch September 27, 2026 19:39
@seathatflowsinourveins seathatflowsinourveins mentioned this pull request Sep 27, 2026
Merged
seathatflowsinourveins pushed a commit that referenced this pull request Sep 27, 2026
Scout @claude
Runtime-worker skills: repair round for the independent Claude review
f2037dc 
GPT-6 (astra, max) repair of all seven findings; the blocker (#407's scoped
global rollback) was already resolved at the rebase and is kept byte for
byte. Project rollback now removes exactly the add targets and checks the
canonical folder, project lock entry and Claude link independently,
reporting a retained copy as an error instead of widening the deletion
(skills@7407f389 src/remove.ts:209-333). A project .claude/skills/<name>
that is not a symlink to the canonical folder is local-modified unless
--force (installer.ts:254-264). Every selected source/ref is fetched before
any add; an unavailable gh is reported as unverified, not as a mismatch.
The pruning rule records Claude Code's skill-listing budget and treats
dropped descriptions as unknown. 52 tests pass; five fail-first runs are
recorded in validation.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
seathatflowsinourveins pushed a commit that referenced this pull request Sep 27, 2026
Scout @claude
Runtime-worker skills: repair round for the independent Claude review
d109d0e 
GPT-6 (astra, max) repair of all seven findings; the blocker (#407's scoped
global rollback) was already resolved at the rebase and is kept byte for
byte. Project rollback now removes exactly the add targets and checks the
canonical folder, project lock entry and Claude link independently,
reporting a retained copy as an error instead of widening the deletion
(skills@7407f389 src/remove.ts:209-333). A project .claude/skills/<name>
that is not a symlink to the canonical folder is local-modified unless
--force (installer.ts:254-264). Every selected source/ref is fetched before
any add; an unavailable gh is reported as unverified, not as a mismatch.
The pruning rule records Claude Code's skill-listing budget and treats
dropped descriptions as unknown. 52 tests pass; five fail-first runs are
recorded in validation.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
seathatflowsinourveins pushed a commit that referenced this pull request Sep 28, 2026
Scout @claude
Runtime-worker skills: repair round for the independent Claude review
f7c7791 
GPT-6 (astra, max) repair of all seven findings; the blocker (#407's scoped
global rollback) was already resolved at the rebase and is kept byte for
byte. Project rollback now removes exactly the add targets and checks the
canonical folder, project lock entry and Claude link independently,
reporting a retained copy as an error instead of widening the deletion
(skills@7407f389 src/remove.ts:209-333). A project .claude/skills/<name>
that is not a symlink to the canonical folder is local-modified unless
--force (installer.ts:254-264). Every selected source/ref is fetched before
any add; an unavailable gh is reported as unverified, not as a mismatch.
The pruning rule records Claude Code's skill-listing budget and treats
dropped descriptions as unknown. 52 tests pass; five fail-first runs are
recorded in validation.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
seathatflowsinourveins pushed a commit that referenced this pull request Sep 28, 2026
Scout @claude
Runtime-worker skills: repair round for the independent Claude review
a583613 
GPT-6 (astra, max) repair of all seven findings; the blocker (#407's scoped
global rollback) was already resolved at the rebase and is kept byte for
byte. Project rollback now removes exactly the add targets and checks the
canonical folder, project lock entry and Claude link independently,
reporting a retained copy as an error instead of widening the deletion
(skills@7407f389 src/remove.ts:209-333). A project .claude/skills/<name>
that is not a symlink to the canonical folder is local-modified unless
--force (installer.ts:254-264). Every selected source/ref is fetched before
any add; an unavailable gh is reported as unverified, not as a mismatch.
The pruning rule records Claude Code's skill-listing budget and treats
dropped descriptions as unknown. 52 tests pass; five fail-first runs are
recorded in validation.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
seathatflowsinourveins pushed a commit that referenced this pull request Sep 28, 2026
Scout @claude
Runtime-worker skills: repair round for the independent Claude review
fd2f0f2 
GPT-6 (astra, max) repair of all seven findings; the blocker (#407's scoped
global rollback) was already resolved at the rebase and is kept byte for
byte. Project rollback now removes exactly the add targets and checks the
canonical folder, project lock entry and Claude link independently,
reporting a retained copy as an error instead of widening the deletion
(skills@7407f389 src/remove.ts:209-333). A project .claude/skills/<name>
that is not a symlink to the canonical folder is local-modified unless
--force (installer.ts:254-264). Every selected source/ref is fetched before
any add; an unavailable gh is reported as unverified, not as a mismatch.
The pruning rule records Claude Code's skill-listing budget and treats
dropped descriptions as unknown. 52 tests pass; five fail-first runs are
recorded in validation.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
seathatflowsinourveins pushed a commit that referenced this pull request Sep 28, 2026
Scout @claude
Runtime-worker skills: repair round for the independent Claude review
aa0d1ee 
GPT-6 (astra, max) repair of all seven findings; the blocker (#407's scoped
global rollback) was already resolved at the rebase and is kept byte for
byte. Project rollback now removes exactly the add targets and checks the
canonical folder, project lock entry and Claude link independently,
reporting a retained copy as an error instead of widening the deletion
(skills@7407f389 src/remove.ts:209-333). A project .claude/skills/<name>
that is not a symlink to the canonical folder is local-modified unless
--force (installer.ts:254-264). Every selected source/ref is fetched before
any add; an unavailable gh is reported as unverified, not as a mismatch.
The pruning rule records Claude Code's skill-listing budget and treats
dropped descriptions as unknown. 52 tests pass; five fail-first runs are
recorded in validation.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
seathatflowsinourveins pushed a commit that referenced this pull request Sep 28, 2026
Scout @claude
Runtime-worker skills: repair round for the independent Claude review
1413553 
GPT-6 (astra, max) repair of all seven findings; the blocker (#407's scoped
global rollback) was already resolved at the rebase and is kept byte for
byte. Project rollback now removes exactly the add targets and checks the
canonical folder, project lock entry and Claude link independently,
reporting a retained copy as an error instead of widening the deletion
(skills@7407f389 src/remove.ts:209-333). A project .claude/skills/<name>
that is not a symlink to the canonical folder is local-modified unless
--force (installer.ts:254-264). Every selected source/ref is fetched before
any add; an unavailable gh is reported as unverified, not as a mismatch.
The pruning rule records Claude Code's skill-listing budget and treats
dropped descriptions as unknown. 52 tests pass; five fail-first runs are
recorded in validation.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
seathatflowsinourveins added a commit that referenced this pull request Sep 28, 2026
@seathatflowsinourveins Scout @claude
Runtime-worker skills program: project-scoped installs and a 137-skil…
bdf25d2 
…l manifest (#429)

* Runtime-worker skills program: project-scoped installs and a 138-skill pinned manifest

GPT-6 (astra, max) build. install_skills.py gains --project-dir and --agent
targets with the default global behaviour unchanged, project lock identity and
pinned source-tree checks, and scoped rollback. The runtime-worker manifest pins
138 skills (77 OpenHands extensions v0.25.0, all 15 superpowers lifecycle
skills, 28 existing adoption entries by reference) with a report-only freshness
checker and scheduled workflow. Not installed into any worker yet; invocation
and pruning are measured after install.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Runtime-worker skills: repair round for the independent Claude review

GPT-6 (astra, max) repair of all seven findings; the blocker (#407's scoped
global rollback) was already resolved at the rebase and is kept byte for
byte. Project rollback now removes exactly the add targets and checks the
canonical folder, project lock entry and Claude link independently,
reporting a retained copy as an error instead of widening the deletion
(skills@7407f389 src/remove.ts:209-333). A project .claude/skills/<name>
that is not a symlink to the canonical folder is local-modified unless
--force (installer.ts:254-264). Every selected source/ref is fetched before
any add; an unavailable gh is reported as unverified, not as a mismatch.
The pruning rule records Claude Code's skill-listing budget and treats
dropped descriptions as unknown. 52 tests pass; five fail-first runs are
recorded in validation.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* CI fix: cover the skills freshness workflow and classify the repair values

Found and fixed through the upstream gh-fix-ci skill (openai/skills@49f948fa)
on the GPT-6 lane, then reviewed by the coordinator:
- tests/test_workflow_security_coverage.py lists
  runtime-worker-skills-freshness.yml; the workflow itself already passes the
  hardening, pinning, permissions and zizmor checks.
- blind_checkout: the review disposition "fixed" joins the closed-vocabulary
  labels (as in #416), and the superseded rollback assertion's disposition is
  data.
- harness-defaults anti-pattern row: run acceptance with TMPDIR outside every
  repository.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* test_install_skills: compare resolved paths on both sides for macOS temp dirs

On macOS the temporary directory sits under /var, a symlink to /private/var,
so local.resolve() returned the /private/var form while the expected path kept
the /var form. validate-macos failed 8 subtests of the two new project-install
tests (run 36349458060). Resolve the expected path too, as
tests/adaptive_paper_hermetic.py already does with os.path.realpath.

Reproduced on Linux with TMPDIR set to a symlinked directory: 8 failures
before, OK after; the module passes 38 tests with a plain or symlinked TMPDIR.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Runtime-worker skills repair r2: exclude main's gated security-audit, take reused gates from main

Coordinator direction on the #429 blocker (option b): runtime workers must
not bypass a gate set on main.

- Contract: every skill in adoption/skills/manifest.json is either reused
  by reuse_ref (pins equal to main's, gates not restated) or listed in
  `excluded` with an adoption_ref, a reason and an overturn condition.
  security-audit (#448, 8315274: Codex off, name-only pending the M5c
  bake-off against /security-review) is excluded, and a test fails as soon
  as main's gate on it changes. A synthetic future main skill in neither
  set fails the contract (negative control).
- Gates: install_skills.py resolves each reuse_ref when it reads a
  manifest, refuses an entry whose pin drifted or that restates
  codex_enabled/claude_listing, and takes both gates from main.
  --print-codex-config on the runtime manifest now disables the 16 reused
  skills main keeps off for Codex; before, it printed none. The README
  states that claude_listing is a Claude Code skillOverrides setting that
  the OpenHands SDK does not read (OpenHands/software-agent-sdk@fcc102a6
  skill.py:1123-1130, 182-185, 278-284, 528-538; same blob at da28c773).
- --only naming a pruned project skill fails as pruned, not unknown.
- The README documents that --check-only needs the pinned skills binary,
  and gh sign-in and network in project mode.
- Counts: 138 selected skills from 12 sources (unchanged); 28 of main's 29
  adoption skills reused; 8 exclusions (7 OpenHands, 1 adoption).
- validation.json records the round (fail-first: 9 tests, failures=7,
  errors=2; after the fix: 9 OK) and refreshes every input hash; the
  anti-pattern log gains the dropped-gates row; the 8 changed files are
  re-registered in manifests/evidence.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Runtime-worker skills: narrow the OpenHands loader claim and the security-audit overturn (R2-6)

The README said OpenHands withholds a skill from the model only through
SKILL.md disable-model-invocation, and research.md said the SDK reads only
SKILL.md frontmatter. load_user_skills also loads OpenHands' own enabled
installed skills (skill.py:936-973 at fcc102a6). Both now say the loader
reads skill directories, SKILL.md frontmatter and its installed-skill enable
state, never Claude Code settings or a skillOverrides key. The
available_skills citation moves from lines 182-185 to 183-186.

The security-audit overturn now names a promotion as codex_enabled true or
claude_listing on; any other change to main's gate still fails the
exclusion test and needs a new review. The test message says so.

validation.json records R2-6, the corrected sources, the red excerpt in log
order, refreshed input hashes and the rerun checks; the five changed files
are re-registered in manifests/evidence.json.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Runtime-worker skills records: note the rebase onto main 8d8f79c

research.md and validation.json record the second rebase, onto 8d8f79c
(#460), the merged docs/harness-defaults.md input hash and the checks rerun
on that tree. The manifests/evidence.json re-registration that was part of
this commit was dropped when the branch was rebased onto c1581fa, and is
redone at the tip.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Runtime-worker skills repair r3: containment and pin checks before add, project scope, name-keyed reuse

Two independent reviews of 8efdc35, a GPT-6 cross-family review
(codex_call.sh, gpt-6-astra max) and a headless Claude session review,
returned changes-needed with no blocker or high finding. Each code item
failed first against the unchanged code:

- GPT6-C1: refuse a project whose .agents/skills, .claude/skills or
  skills-lock.json passes through a symlink, whose selected skill paths
  resolve outside it, or that is --home, before any skills or gh call.
- CL-D1: compare every selected pinned tree with its tree_sha in the
  preflight and refuse before any add; the native in-use guard keeps a
  rolled-back copy while a detected agent such as Codex reads it.
- CL-D3: the runtime manifest carries "scope": "project" and is refused
  without --project-dir; pruned entries are never installed in either mode.
- CL-D6: reuse_ref and adoption_ref name the adoption manifest and match
  by skill name.
- CL-D7: runtime_skill_freshness.py reads the CLI version and ref from
  the manifest's cli pin.
- GPT6-C5: offline zizmor analysis of the freshness workflow, with
  floating-tag, persist-credentials and permissions controls.

Docs: the rendered OpenHands skill list omits <location>; disabled_skills
and the loading flags are its controls; third-party instruction files load
from the work directory and Git root, so the example worker projects move
outside the checkout; rollback retention is certain when Codex is detected;
skills-review-r1.json is labelled unpublished.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Runtime-worker skills repair r4: exclude verification-before-completion with main (M4, #464)

Main c0966da (#464) moved verification-before-completion from the
adoption skills to its excluded list under the skills-trial conflict
rule M4. On the rebased tree the unchanged coverage contract and
installer refused the runtime manifest's reuse_ref for it (67 tests,
6 failures, 2 errors).

- Exclude the skill in the runtime manifest with its reviewed pin, the
  M4 and AGENTS.md:16 reason, and the overturn "main re-admits it"; it
  carries no adoption_ref because main no longer lists it in skills.
- New test_verification_before_completion_stays_excluded_until_main_readmits_it
  and the revised superpowers lifecycle test failed first; a source-count
  test ties sources and the research table to the selected skills.
- Recount: 137 skills from 12 sources (superpowers 14), 27 reused
  (16 Codex-disabled on main), 9 exclusions, 15 empty cells (new
  github-ci-fix/extraction-caller gap), 1,020,955 bytes and 36,739
  description characters.
- validation.json: repair_r3.not_relayed becomes the five confirmed
  CONFIRM ids; repair_r4 records the rebase, sources, fail-first and checks.
- State Codex detection precisely ($CODEX_HOME, ~/.codex when unset, or
  /etc/codex) in research.md, the harness-defaults row and a code comment.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Rebase onto main c0966da: re-register the PR's files in manifests/evidence.json

Main's evidence.json with the 13 PR files registered at their current
sha256 and byte size (8 added, 5 updated); no other entry changed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Scout <scout@local>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

lane:foundation Foundation lane: Claude/Codex setup, hosts, memory, RAG, research, workers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

install_skills.py: the failed-hash rollback runs skills remove without -a, so it can delete another agent's same-named skill

1 participant

@seathatflowsinourveins