Skip to content

Phase 0 — canonical Origin model (collapse the six provenance representations) #207

Description

@aroff

Problem Statement

Provenance — "where an installed skill came from" — is currently modelled six different ways across the codebase: two distinct types both named SkillSource (one in core::manifest, one in the CLI add command), a SourceType enum, a SourceSpecificFields struct, the flat source_* fields on the lock entries, and nine flat source_* fields on SkillDefinition. This drift is a comprehension tax and a latent bug surface (the two same-named types can be confused; branch/tag Options allow illegal combinations; the lock duplicates data already in its embedded source). It also blocks the browser skill-management work (spec 003): the HTTP handlers need a single, canonical, serializable provenance model to build an install(origin, mode) seam on, and the current tangle has no clean shape to hand across the crate boundary.

Solution

Introduce one canonical Origin type in fastskill-core that captures install intent (what the user asked for), and collapse all six existing representations into it. Separate intent from resolved facts: the exact commit SHA, resolved version, checksum, and timestamps move to a lock-only Resolved record, so Origin stays a clean description of intent. Rewrite the Manifest and Lock persisted formats around origin + resolved, and replace the CLI's string-to-source inference so it returns an Origin. This is a behavior-preserving refactor for the CLI (no user-facing command changes) that lands independently and becomes the foundation for spec 003 Phases 1–3. Per ADR-0005 it is a greenfield break: no back-compat shim and no migrator, but old on-disk data is rejected with a clear, actionable message rather than a cryptic parse failure.

User Stories

  1. As a maintainer, I want a single Origin type for provenance, so that I stop reasoning about six overlapping representations.
  2. As a maintainer, I want the two colliding SkillSource types removed, so that a symbol name no longer means two different things across crates.
  3. As a maintainer, I want Origin to be intent-only, so that "what the user asked for" is not smeared together with "what it resolved to."
  4. As a maintainer, I want resolved facts (commit SHA, resolved version, checksum, timestamps) to live only in the Lock's Resolved record, so that the reproducibility data has one home.
  5. As a maintainer, I want the Lock to embed a copy of the skill's Origin, so that the lock records exactly what was resolved from, independent of later manifest edits.
  6. As a maintainer, I want GitRef modelled as a sum type (Default | Branch | Tag | Commit), so that illegal states like "branch and tag at once" are unrepresentable.
  7. As a maintainer, I want commit-pinning support via GitRef::Commit, so that a user can pin a git origin to an exact SHA.
  8. As a maintainer, I want Origin::repository's version to be an Option<VersionConstraint> (the ADR-0004 normalizing type), so that versioning logic lives in exactly one place and a bare version normalizes to an exact pin.
  9. As a maintainer, I want the four Origin variants (git, local, zip-url, repository) to cover all five real fetch mechanisms, so that no install path is left unmodelled.
  10. As a maintainer, I want local to cover both a directory and a local .zip file, so that installing a local archive does not need a fifth variant.
  11. As a maintainer, I want editable validated as directory-only, so that an editable local-zip is rejected with a clear error instead of silently misbehaving.
  12. As a maintainer, I want Origin::repository to always store a concrete Repository name, so that provenance survives a change to the default repository and multiple registries are addressable.
  13. As a user installing from a registry without naming a repo, I want the default resolved and recorded at install time, so that my one-word input still produces unambiguous provenance.
  14. As a user, I want to paste a single string (URL, path, or scope/id) and have the correct Origin inferred with a sensible "latest" default, so that the common case needs no flags.
  15. As a user installing from a URL that ends in .zip, I want it treated as a zip-url origin, so that a remote archive is no longer mis-detected as a git repository.
  16. As a user, I want a default-branch git install to serialize to a minimal {"type":"git","url":…} form, so that the manifest/lock stay readable for the 95% case.
  17. As a maintainer, I want the Manifest SkillEntry reduced to { id, origin, groups }, so that the redundant top-level version and editable fields (now folded into Origin) are gone.
  18. As a maintainer, I want the Lock entry reshaped to { id, name, origin, resolved, installed_at, deps, depth, parent, groups }, so that its flat source_*/commit_hash/checksum fields collapse into origin + resolved.
  19. As a maintainer, I want SkillDefinition's nine source_* fields replaced by a single origin, so that the runtime skill record matches the canonical model.
  20. As a user upgrading the tool, I want an old lock (version below 3.0) to be rejected with a message telling me to delete the lock and re-run install, so that I am never left debugging a cryptic serde error.
  21. As a user upgrading the tool, I want a pre-Origin manifest to fail with a message telling me to replace source = … with origin = …, so that I know exactly how to fix it.
  22. As a maintainer, I want the CLI's existing commands (add, update, remove, list) to behave identically after the refactor, so that Phase 0 ships with zero user-facing behavior change.
  23. As a maintainer, I want the lock metadata.version bumped to 3.0, so that the format change is detectable going forward.
  24. As a maintainer, I want Origin to serialize with an internally-tagged type discriminator, so that the persisted form is self-describing and stable.
  25. As a maintainer, I want SourceSpecificFields and SourceType deleted, so that no vestigial provenance types remain after the collapse.

Implementation Decisions

  • New module Origin in fastskill-core (a new origin submodule under core), re-exported alongside the other core types. It is the single canonical provenance type; ADR-0005 governs it and CONTEXT.md defines it.

  • Origin is intent-only. Variants: Git { url, ref: GitRef, subdir? }, Local { path, editable } (path is a directory or a .zip; editable is dir-only), ZipUrl { url } (remote), Repository { repo, skill, version: Option<VersionConstraint> } (repo always concrete). GitRef is a sum type Default | Branch | Tag | Commit with Default meaning the repository's default branch.

  • Serialization: internally-tagged with a type discriminator (git | local | zip-url | repository); unset optional fields (ref when Default, subdir, version) are omitted so the "latest" case is minimal. From a prototype, the intended type shape:

    #[serde(tag = "type", rename_all = "kebab-case")]
    pub enum Origin {
        Git { url: String, r#ref: GitRef, subdir: Option<PathBuf> },
        Local { path: PathBuf, editable: bool },
        ZipUrl { url: String },
        Repository { repo: String, skill: String, version: Option<VersionConstraint> },
    }
    #[serde(rename_all = "kebab-case")]
    pub enum GitRef { Default, Branch(String), Tag(String), Commit(String) }
  • Resolved state is lock-only. A Resolved { version, commit_hash?, checksum? } record is added to the Lock entry. Origin carries none of these.

  • Manifest SkillEntry becomes { id, origin, groups }. Its former top-level version folds into Origin::repository's constraint; editable folds into Origin::local.

  • Lock entry becomes { id, name, origin, resolved, installed_at, dependencies, depth, parent_skill, groups } for both project and global locks. The flat source_name/source_url/source_branch/commit_hash/checksum fields are removed. The lock metadata.version is bumped to 3.0.

  • SkillDefinition's nine source_* fields collapse into a single origin: Origin.

  • String inference (detect_skill_source and callers) returns an Origin. Rules: scope/id (optionally @version) → Repository; a URL whose path ends in .zipZipUrl; other http(s)/git/.git URLs → Git (default branch); a local path ending in .zipLocal; any other existing/relative path → Local. This fixes the current defect where a .zip URL is classified as git.

  • Repository resolution: when the user's input omits a repository, resolve the default at install time and record its concrete name in Origin::repository.repo.

  • Old-data handling: no shim, no migrator. On lock read, a metadata.version below 3.0 is rejected with an actionable error ("delete the lock and re-run install"). A pre-Origin manifest that fails to deserialize is surfaced with a clear "replace source = … with origin = …" message rather than a raw serde error.

  • Deletions: the CLI SkillSource, the manifest SkillSource, SourceType, and SourceSpecificFields are removed once all call sites route through Origin.

  • Scope: this PRD is the model + persistence + inference refactor only. The install(origin, mode) core seam, the reindex seam, endpoints, and UI are later phases of spec 003.

Testing Decisions

  • Good tests exercise external behavior, not internals: the persisted on-disk shape and the parse/round-trip result — not the private field layout of the new types.
  • Seam 1 — persistence boundary (Manifest load/save, Lock save/load). Tests: (a) round-trip every Origin variant (and the SkillEntry/lock-entry that embed it) serialize→deserialize losslessly; (b) golden on-disk shape assertions, including the terse "latest" forms (a default-branch git origin serializes without ref/subdir; a repository origin without version); (c) old-data rejection — a lock with metadata.version below 3.0, and a manifest using the pre-Origin source = … shape, each produce the actionable error. Prior art: skill_project_toml_test.rs, integration_skill_project_toml_test.rs, integration_unified_format_test.rs, core_project_tests.rs.
  • Seam 2 — detect_skill_sourceOrigin. Table-driven test: each input string maps to the expected Origin variant, explicitly covering the .zip-URL → ZipUrl fix, scope/id[@version]Repository, git URLs → Git default branch, and local dir/zip → Local. Prior art: the existing mod tests in crates/fastskill-cli/src/utils.rs.
  • Behavior-preserving safety net: the existing CLI/integration tests (skill_project_toml_test, integration_unified_format_test, core_project_tests) must stay green with only the mechanical type rename — they are the guard that Phase 0 changed no CLI behavior.
  • No new seams are introduced; both test seams already exist.

Out of Scope

  • The FastSkillService::install(origin, mode) core seam and the reindex seam (spec 003 Phase 1).
  • Any HTTP endpoint, /status capability, or web-UI change (spec 003 Phases 2–3).
  • A migrator or back-compat shim for pre-Origin on-disk data (ADR-0005: greenfield break).
  • Changing the CLI's user-facing commands, flags, or output.
  • The registry version picker and sanitized Markdown preview (spec 003 v2).

Further Notes

  • Depends on and is governed by ADR-0005 (install seam + Origin model) and the Origin glossary entry in CONTEXT.md; respects ADR-0004 (bare version = exact pin, registry-scoped) via VersionConstraint, and ADR-0002 (reindex conditional) is untouched here.
  • Full design detail (the grilled Q1–Q5 decisions) lives in specs/003-browser-skill-management.md under "Phase 0 — the Origin model (detailed)". Note that specs/ is gitignored (local working artifact), so the ADR + this PRD are the durable record.
  • This is a large mechanical change touching every provenance construction/read site; landing it behind green existing integration tests is the correctness anchor.

Metadata

Metadata

Assignees

No one assigned

    Labels

    ready-for-agentPRD triaged and ready for an agent to implement

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions