Skip to content

fix(lock): reject unreadable resolved schemas before overwriting existing pins #5

Closed

Description

@showxu

Summary

skill-cli 0.2.1 treats an existing skills.resolved file that cannot be decoded as empty state. A subsequent successful add overwrites it with the new dependency, losing the previous source, skill, and installation records.

This concerns decoding failures in a readable file. Data(contentsOf:) read errors already propagate.

Code evidence at b742a99:

  • Sources/Core/InstallLock.swift:131–147 suppresses both current and legacy decoding errors and returns LockFile().
  • Sources/Core/InstallLock.swift:241–245 also permits fallback to a legacy file after a malformed current file.
  • Sources/Core/InstallLock.swift:170–198 merges into that result and saves it.
  • Sources/Core/InstallLock.swift:150–163 replaces the resolved file.
  • Sources/Core/Installer.swift:167–195 materializes installation files before the lock merge at lines 236–241.

Impact: recoverable registry metadata can be silently discarded. Installed directories may remain, but their management records are lost.

Reproduction

The following is a proposed reproduction derived from source inspection; it has not been executed for this report.

Use a fresh temporary directory. All skill commands below explicitly use project scope.

repro_root="$(mktemp -d)"
mkdir -p "$repro_root/project" "$repro_root/source/a" "$repro_root/source/b"

for skill_name in a b; do
  printf -- '---\nname: %s\ndescription: Reproduction fixture\n---\n' \
    "$skill_name" > "$repro_root/source/$skill_name/SKILL.md"
done

cd "$repro_root/project"
skill add "$repro_root/source/a" --scope project --agent codex --mode copy
cp .agent/skills.resolved "$repro_root/valid.resolved"

# Make JSON invalid while retaining all original records.
printf '!' >> .agent/skills.resolved
cp .agent/skills.resolved "$repro_root/malformed.resolved"

skill add "$repro_root/source/b" --scope project --agent codex --mode copy
cat .agent/skills.resolved

No legacy lock should exist in this fresh project.

Expected Behavior

Adding b fails with an actionable decoding error identifying skills.resolved. The existing file remains byte-for-byte unchanged.

A missing file may initialize empty state; a malformed existing file must not be treated as missing. Validate existing state before materializing installation changes.

Completion checks:

  • Invalid JSON and structurally invalid current-schema JSON fail without overwriting the file.
  • A malformed current file does not silently fall back to an older legacy file.
  • A missing current file can still use supported legacy migration.
  • Valid current files continue to merge normally.
  • Run swift test after implementing the fix.

Actual Behavior

By the current control flow, the invalid file is treated as empty. The second add succeeds and saves only b's records. The records for a disappear without a decoding diagnostic.

Duplicate search found no matching issue. Issue #4 concerns source membership reconciliation and is unrelated.

Environment

  • skill version: 0.2.1.
  • Source inspected: b742a99; Sources and Tests are unchanged from v0.2.1.
  • Swift version: not obtained; the version query was blocked by sandbox cache permissions.
  • OS: macOS 27.0.1 (26A434).
  • Architecture: arm64.
  • Validation: static inspection only; reproduction and tests not executed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions