feat(001): support optional branch/ref in origin reference (OWNER/REPO[@REF]) by so0k · Pull Request #4 · skillrig/cli

so0k · 2026-05-29T11:13:46Z

Summary

Origin references now accept an optional @REF suffix — OWNER/REPO[@REF] — to track a branch of the skills library. A bare OWNER/REPO is unchanged and tracks the default branch.
Validation stays shape-only and offline (no network/existence check, consistent with the rest of init). The ref is stored combined in the single origin key (origin = 'my-org/my-skills@staging') and round-trips through the resolver with no structural change (Origin.Ref + String()).
Adopts the project's R26 identity grammar (@ref, as used by gh skill / npx skills). For an origin @ref is a moving branch pointer; immutable skill pinning (add --pin) stays a separate, later concern.
Amendment to spec 001 (FR-018–FR-020): new addendum + checkpoint log, updated cli.md ("Origin Reference Grammar" section + rationale for @ref vs --branch/#), README, ARCHITECTURE §2d/R26, contracts, data-model, quickstart, and the skillrig-init skill + evals. Also bundles the CLAUDE.md pre-release marker and ROADMAP branch-aware/aws notes.

Test plan

make check → green: gofmt, go vet, golangci-lint (0 issues), go test ./....
New tests: TestQuickstart_BindWithRef (incl. JSON completeness + footer), TestSaveLoadRoundTripWithRef, TestResolveOrigin_MalformedRefRecordsDiagnostic, resolver precedence rows 8/9, ParseOrigin branch/tag/commit/slash + invalid-ref cases, TestParseOriginStringRoundTrip.
Manual e2e: init --origin my-org/my-skills@main --json writes the combined key; idempotent re-bind (written:false); @bad:ref exits 1 with OWNER/REPO[@REF] guidance; bare OWNER/REPO still works.

Notes for reviewers

An independent adversarial review ran on this branch; its 3 MEDIUM findings (contract drift, ground-truth matrix rows, untested ref-reject path) plus an assertion gap were fixed in this PR. Details in specledger/001-init-origin-resolution/sessions/001-origin-ref-support-checkpoint.md.
refPattern is intentionally permissive (shape-only): git-invalid-but-charset-clean refs (.., leading/trailing /) are accepted by design (decision D-A2) — existence/validity is deferred to a future command.
Skill trigger accuracy is argued, not measured: the description change is purely additive, but the eval harness is confounded by the already-installed skill (0/0 across queries). A clean run requires temporarily un-installing the skill.

🤖 Generated with Claude Code

@ref

Origin references now accept an optional @ref suffix (OWNER/REPO[@ref]) to track a branch of the skills library; a bare OWNER/REPO is unchanged and tracks the default branch. Validation stays shape-only and offline, and the ref is stored combined in the single `origin` config key, round-tripping through the resolver with no structural change (Origin.Ref + String()). Amendment to spec 001 (FR-018–FR-020): adds the addendum, updates the contracts/data-model/quickstart, cli.md "Origin Reference Grammar" section, README, ARCHITECTURE §2d/R26 notes, and the skillrig-init skill + evals. Also includes the CLAUDE.md pre-release marker and ROADMAP branch-aware/aws notes. All amendment issues closed; make check green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

qodo-code-review · 2026-05-29T11:14:25Z

Review Summary by Qodo

Support optional branch/ref in origin references (OWNER/REPO[@ref])

✨ Enhancement

Walkthroughs

Description

• Add optional @REF suffix to origin references (OWNER/REPO[@REF])
  - Allows repos to track specific branches of skills libraries
  - Stored combined in single origin config key with no structural change
• Implement shape-only, offline validation for branch/ref (consistent with existing design)
  - Permissive charset validation; existence/reachability deferred to future commands
  - Rejects malformed refs (empty after @, invalid characters) with actionable errors
• Extend Origin data model, ParseOrigin parser, and String() renderer
  - Add Ref field to Origin struct; split on @ during parsing
  - Update resolver precedence matrix to prove ref survives resolution unchanged
• Update CLI prompts, help text, contracts, and documentation
  - Align all user-facing text to OWNER/REPO[@REF] grammar
  - Add Origin Reference Grammar section to cli.md with rationale for @ref vs alternatives
• Expand test coverage and agent skill
  - Add unit tests for ref parsing, round-trip serialization, malformed refs
  - Add integration tests (TestQuickstart_BindWithRef, resolver rows 8–9)
  - Update skillrig-init skill with branch-tracking eval and trigger keywords

Diagram

flowchart LR
  A["Origin Reference<br/>OWNER/REPO[@REF]"] --> B["ParseOrigin<br/>Split on @"]
  B --> C["Validate OWNER/REPO<br/>+ optional REF"]
  C --> D["Origin Struct<br/>Owner, Repo, Ref"]
  D --> E["String() Render<br/>with @Ref suffix"]
  E --> F["Config Storage<br/>origin = 'org/repo@branch'"]
  F --> G["Resolver<br/>Ref passes through"]
  G --> H["CLI Output<br/>--json + human"]

File Changes

1. internal/cli/init.go 📝 Documentation +7/-3

Update prompts and help text for @ref syntax

internal/cli/init.go

2. internal/config/origin.go ✨ Enhancement +50/-17

Add Ref field and @-split parsing logic

internal/config/origin.go

3. internal/config/origin_test.go 🧪 Tests +49/-5

Expand tests for branch/tag/commit refs

internal/config/origin_test.go

View more (19)

4. internal/config/config_test.go 🧪 Tests +43/-0

Add round-trip test with ref in config

internal/config/config_test.go

5. internal/config/resolve_test.go 🧪 Tests +31/-1

Add resolver precedence rows for ref survival

internal/config/resolve_test.go

6. test/quickstart_test.go 🧪 Tests +54/-0

Add integration test for branch binding

test/quickstart_test.go

7. docs/design/cli.md 📝 Documentation +22/-1

Document origin grammar and @ref rationale

docs/design/cli.md

8. README.md 📝 Documentation +16/-1

Add branch-tracking example and explanation

README.md

9. docs/ARCHITECTURE-v0.md 📝 Documentation +3/-1

Note R26 identity grammar implementation

docs/ARCHITECTURE-v0.md

10. .agents/skills/skillrig-init/SKILL.md 📝 Documentation +18/-8

Update skill description for branch tracking

.agents/skills/skillrig-init/SKILL.md

11. .agents/skills/skillrig-init/evals/evals.json 🧪 Tests +12/-0

Add eval for branch-tracking use case

.agents/skills/skillrig-init/evals/evals.json

12. .agents/skills/skillrig-init/evals/trigger-eval-set.json 🧪 Tests +2/-0

Add trigger queries for branch scenarios

.agents/skills/skillrig-init/evals/trigger-eval-set.json

13. specledger/001-init-origin-resolution/amendments/001-origin-ref-support.md 📝 Documentation +95/-0

New amendment spec for optional @ref

specledger/001-init-origin-resolution/amendments/001-origin-ref-support.md

14. specledger/001-init-origin-resolution/contracts/init.md 📝 Documentation +6/-6

Update contract to reflect @ref syntax

specledger/001-init-origin-resolution/contracts/init.md

15. specledger/001-init-origin-resolution/contracts/resolve.md 📝 Documentation +1/-0

Document ref pass-through in resolver

specledger/001-init-origin-resolution/contracts/resolve.md

16. specledger/001-init-origin-resolution/data-model.md 📝 Documentation +8/-5

Add Ref field and update ParseOrigin spec

specledger/001-init-origin-resolution/data-model.md

17. specledger/001-init-origin-resolution/quickstart.md 📝 Documentation +14/-0

Add BindWithRef scenario and resolver rows

specledger/001-init-origin-resolution/quickstart.md

18. specledger/001-init-origin-resolution/spec.md 📝 Documentation +5/-3

Add amendment reference and update assumptions

specledger/001-init-origin-resolution/spec.md

19. specledger/001-init-origin-resolution/sessions/001-origin-ref-support-checkpoint.md 📝 Documentation +69/-0

Record divergence review and resolution

specledger/001-init-origin-resolution/sessions/001-origin-ref-support-checkpoint.md

20. specledger/001-init-origin-resolution/issues.jsonl 📝 Documentation +6/-0

Add amendment feature and task issues

specledger/001-init-origin-resolution/issues.jsonl

21. CLAUDE.md 📝 Documentation +3/-1

Add pre-release marker and update description

CLAUDE.md

22. docs/ROADMAP.md Additional files +2/-1

...

docs/ROADMAP.md

qodo-code-review · 2026-05-29T11:14:27Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. ParseOrigin uses user-facing string ✓ Resolved 📘 Rule violation ⌂ Architecture

Description

internal/config.ParseOrigin() returns a formatted, user-facing usage message via originErrFmt,
embedding presentation text in the config layer. This violates the requirement that
internal/config remain presentation-free and makes future error UX changes harder to manage.

Code

internal/config/origin.go[R28-32]

Evidence
The checklist forbids human-oriented formatting in internal/config. The PR adds originErrFmt
containing a full end-user usage message (including examples) and uses it in ParseOrigin() to
build returned errors, placing presentation text in internal/config.
Rule 783432: No human-oriented formatting or printing in internal/config package
internal/config/origin.go[28-32]
internal/config/origin.go[70-76]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`internal/config.ParseOrigin()` constructs user-facing copy (e.g., expected shape + examples) inside `internal/config` via `originErrFmt` and `fmt.Errorf(...)`. Per compliance, `internal/config` should return typed/structured errors (or sentinel errors) and let `internal/cli` render the user-facing message.

## Issue Context
The CLI currently wraps `ParseOrigin` errors into `UsageError` by reusing `err.Error()`, which means config-layer strings become the end-user UX. The desired direction is: config returns a machine-readable error type describing what failed; CLI formats the final `invalid origin ... expected OWNER/REPO[@REF] ...` message.

## Fix Focus Areas
- internal/config/origin.go[28-32]
- internal/config/origin.go[67-77]
- internal/cli/init.go[95-98]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@ref

… cli Addresses Qodo rule 783432 (internal/config must stay presentation-free). ParseOrigin now returns a typed *InvalidOriginError carrying only the offending value; internal/cli renders the OWNER/REPO[@ref] what/why/fix message. Updates the config + cli unit tests and the data-model/contract/amendment wording. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…FIG env Qodo code review (3 bugs + 6 rule violations). Fixed the 4 real/actionable; declined 4 (documented decisions / false positive) — see PR reply. - #8 (security): token no longer passed via argv. authConfigArgs (-c http.extraHeader=…) → authConfigEnv injecting GIT_CONFIG_COUNT/KEY_0/VALUE_0 (git >=2.31) via cmd.Env, so the base64 credential is in the process environ, not ps-visible argv. Matches gh-cli's "token out of argv" posture (gh uses a credential helper; we use GIT_CONFIG env since we resolve the token ourselves). runEnv() added; Clone/fetchSparseInto/FetchFile use it. F2 tests rewritten: TestAuthConfigEnv + TestClone_TokenInjectionViaEnv (assert no token in argv, credential in GIT_CONFIG_VALUE_0 env). - #7: `search` no longer requires a git repo — repoRoot is optional; a non-git cwd / checkout-less remote falls through to FetchCatalog. New TestQuickstart_SearchRemoteFromNonGitDir (file:// origin from a plain tempdir). - #5: search declares Args: cobra.ArbitraryArgs (rule 782342). - #9: human description width 60 → 80 (rule 783450). - #4: investigated — no genuine ignored error (errcheck clean); no change. Gate: make check 0 lint; go test -count=1 all pass; new tests run+pass; only the two git-on-PATH t.Skip guards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

## What Adds a **research spike** and an **RFC** for a `skillrig` **mise backend plugin** that co-installs multiple backing CLIs from one origin monorepo, plus a correction to `ARCHITECTURE-v0` §8b. Resolves the investigation in #23. The RFC is written to **bootstrap a new, separate repo** (`skillrig/mise-skillrig` — the plugin is Lua, not Go, so it does not belong in `skillrig/cli`). ## The key finding (why this RFC matters) #23 proposed the plugin to fix a multi-binary install collision. While researching open question #4, the premise shifted — and then a reviewer pushback sharpened it into the real justification: - **Layer A — install scheduler.** The collision (two aliases → same `github:org/repo@version` deduped to one install) was a scheduler bug, **fixed in mise 2026.4.12 (PR #9093)**. We require it, but it only fixes Layer A. - **Layer B — version resolution.** PR #9093 explicitly *does not touch version listing/resolution*. The origin's **strict-semver tag policy** forbids prefix tags (`iii-v0.1.0`) and forces **build-metadata streams** (`v0.5.0+iii`). Per SemVer 2.0.0, build metadata is **ignored for precedence**, so those streams collapse to one version, and `version_prefix` (a leading-prefix stripper; there is **no** `tag_regex`) cannot select a `+` suffix. | Option | Indep. versions | Co-location | Strict-semver | Native mise | |---|:---:|:---:|:---:|:---:| | prefix streams + `version_prefix` | ✅ | ✅ | ❌ forbidden | ✅ | | build-metadata streams | ✅ | ✅ | ✅ | ❌ **broken (Layer B)** | | one release, all binaries | ❌ | ✅ | ✅ | ✅ (post-#9093) | | separate repo per CLI | ✅ | ❌ | ✅ | ✅ | | **`skillrig` backend plugin** | ✅ | ✅ | ✅ | ✅ (owns listing) | Only the plugin satisfies *independent versioning + co-location + strict-semver* together — because its `BackendListVersions` hook **owns version listing**. That is a **capability** justification, not just ergonomics. ## Open questions answered (from #23) - **OQ4 (native coverage):** partially — #9093 is scheduler-only; build-metadata streams stay native-unresolvable. Plugin justified. - **OQ1 (home/cadence):** own repo `skillrig/mise-skillrig`, backend name `skillrig`, independent release cadence. - **OQ2 (origin metadata):** new convention-versioned `[[binaries]]` block in `.skillrig-origin.toml`, mirrored into `index.json` (stream selector + asset template + checksums). Shared by plugin and CLI (AP-04). - **OQ3 (verification):** checksum-only for v1 (origin already ships `checksums.txt`); treeSha/provenance parity is v2. ## Contents - `specledger/013-mise-backend/research/2026-06-02-mise-backend-plugin.md` — the spike (corrected after review). - `docs/rfcs/0001-mise-skillrig-backend.md` — the RFC: how the **origin template**, **mise + plugin**, and **`skillrig` CLI** fit together; the `[[binaries]]` contract; the three Lua hooks (with snippets); auth (incl. the keyring-404 gotcha); verification depth; alternatives; phasing. - `docs/ARCHITECTURE-v0.md` §8b — correction callout pointing to the RFC. ## Notes for reviewers - The RFC documents the **native fallbacks honestly** — an org that can relax its tag policy to prefix streams doesn't need the plugin. - Docs-only; no code or CLI behavior change. The plugin itself is **not** built here. https://claude.ai/code/session_01Nr5ZxzqTGefujhtEVPzMeV --- _Generated by [Claude Code](https://claude.ai/code/session_01Nr5ZxzqTGefujhtEVPzMeV)_ --------- Co-authored-by: Claude <noreply@anthropic.com>

qodo-code-review Bot reviewed May 29, 2026

View reviewed changes

Comment thread internal/config/origin.go Outdated

so0k merged commit dc7924c into main May 29, 2026

so0k deleted the 001-init-origin-resolution branch May 29, 2026 13:36

so0k mentioned this pull request May 30, 2026

feat(002): vendor & verify skills — skillcore SDK + add/verify #5

Merged

This was referenced May 31, 2026

feat(003): Discover & Acquire — search + remote add + index (manifest→frontmatter) #8

Merged

docs(013): add RFC 0001 for the skillrig mise backend plugin #27

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(001): support optional branch/ref in origin reference (OWNER/REPO[@REF])#4

feat(001): support optional branch/ref in origin reference (OWNER/REPO[@REF])#4
so0k merged 2 commits into
mainfrom
001-init-origin-resolution

so0k commented May 29, 2026

Uh oh!

qodo-code-review Bot commented May 29, 2026

Uh oh!

qodo-code-review Bot commented May 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

so0k commented May 29, 2026

Summary

Test plan

Notes for reviewers

Uh oh!

qodo-code-review Bot commented May 29, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-code-review Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-code-review Bot commented May 29, 2026 •

edited

Loading