Skip to content

chore: sync SpecLedger templates to constitution v2.1.0#2

Closed
so0k wants to merge 1 commit into
mainfrom
worktree-sync-templates-constitution
Closed

chore: sync SpecLedger templates to constitution v2.1.0#2
so0k wants to merge 1 commit into
mainfrom
worktree-sync-templates-constitution

Conversation

@so0k

@so0k so0k commented May 24, 2026

Copy link
Copy Markdown
Contributor

Why

The constitution was amended to v2.1.0 (nine principles): added III. Ground-Truth Anchoring and IX. Skill–CLI Co-Evolution, folded output-shape assertions into II, and split the old Simplicity/YAGNI principle into VI–VIII. The .specledger/templates/ still reflected the old five-principle structure, so the Constitution Check gate and task scaffolding were stale and partly Python-flavored.

Changes

plan-template.md

  • Constitution Check rewritten to all nine principles — renumber Agent-First CLI Design (III→IV) and Code Quality (IV→V); add III. Ground-Truth Anchoring and IX. Skill–CLI Co-Evolution; output-shape assertions in II; split V → VI–VIII.
  • Testing line cites output-shape assertions + ground-truth fixtures/tiers.
  • Source Code layout replaced with skillrig's Go structure (cmd/skillrig, internal/skillcore|config|lock|index|client, testdata/cassettes) instead of the src/models/services Python placeholder (Constitution V).

spec-template.md

  • Acceptance-scenario output-shape guidance so scenarios seed shape-asserting tests, not strings.Contains checks (Constitution II).

tasks-template.md

  • Foundational ground-truth fixture task (III).
  • Output-shape assertions noted in every user-story test section.
  • Polish tasks for pattern-gate classification (IV) and skill co-evolution trigger evals (IX) + design-doc sync.
  • Implementation tasks use Go paths (internal/, cmd/skillrig), not .py.
  • Definition of Done extended: shape assertions · ground-truth fixtures · pattern classification · skill coverage.

Deferred (not in this PR)

  • Checklist rename — there are two same-named checklist-template.md files (.specledger/templates/ generic vs docs/design/ pattern-gate). Renaming the design-doc one (+ updating constitution/cli.md references) was flagged optional; left as a follow-up to avoid touching files already on main.

🤖 Generated with Claude Code

@so0k @claude
chore: sync SpecLedger templates to constitution v2.1.0
e033622 
Re-align the plan/spec/tasks templates with the nine-principle constitution
(v2.1.0); they still reflected the old five-principle structure.

plan-template.md
- Constitution Check gate rewritten to all nine principles: renumber
  Agent-First CLI Design (III→IV) and Code Quality (IV→V); add the new
  III. Ground-Truth Anchoring and IX. Skill–CLI Co-Evolution gates; fold
  output-shape assertions into II; split V into VI–VIII (Simplicity).
- Testing line cites output-shape assertions + ground-truth fixtures/tiers.
- Source Code layout replaced with skillrig's Go structure (cmd/skillrig,
  internal/skillcore|config|lock|index|client, testdata/cassettes) instead
  of the Python-flavored src/models/services placeholder (Constitution V).

spec-template.md
- Acceptance scenarios get output-shape guidance so they seed shape-asserting
  tests, not content-only checks (Constitution II).

tasks-template.md
- Foundational ground-truth fixture task (Constitution III).
- Output-shape assertions noted in every US test section.
- Polish tasks for pattern-gate classification (IV) and skill co-evolution
  trigger evals (IX); design-doc sync note.
- Implementation tasks use Go paths (internal/, cmd/skillrig) not .py.
- Definition of Done extended to shape assertions, ground-truth fixtures,
  pattern classification, and skill coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
so0k added a commit that referenced this pull request May 26, 2026
@so0k @claude
fix(001): surface resolver source diagnostics (FR-004) + close review…
3910538 
… gaps

Resolve the post-implementation checkpoint findings on the origin resolver so a
future --verbose caller can never mistake a silent skip for intended behavior.

- config.MalformedError: typed, unwrappable. Load returns it for parse failures
  and a plain wrapped error for genuine I/O failures, so callers distinguish a
  skippable malformed file from a fatal read error (FR-004 / contract resolve.md).
- ResolveOrigin: add ResolutionResult.Diagnostics ([]SourceDiagnostic). New
  originFromFile classifies each source into four honest outcomes — usable /
  skippable-with-diagnostic (malformed or invalid origin) / quiet fall-through
  (absent or origin-less) / fatal I/O — replacing usableOrigin which collapsed
  everything (including I/O errors) to (zero, false). Diagnostics accumulate
  regardless of the final Source.
- Tests: malformed-project, invalid-shape, origin-less, unreadable-fatal,
  unreadable-project-fatal-despite-global (A3), and malformed-SKILLRIG_ORIGIN
  hard-error (A1); TestLoadMalformedErrors asserts errors.As(*MalformedError).
- Remove dead Origin.IsZero() (A4 — no callers).
- Reconcile research.md D9 to the shipped 0o600/0o750 perms (self-review #2).
- contracts/resolve.md + data-model.md document Diagnostics and the
  malformed-vs-fatal split, with a "never re-introduce a silent skip" note.

Gate green: go test ./... · gofmt · go vet · golangci-lint (0 issues).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
so0k added a commit that referenced this pull request May 26, 2026
@so0k @claude
fix(001): address Qodo review — surface swallowed errors, fix zero-or…
d95b830 
…igin String, scope todo rule

Resolve the three real bugs from the Qodo PR review (#2/#3/#4); #1 dismissed as
a false positive with an AGENTS.md clarification.

- #2 FindProjectConfig now returns (string, bool, error): a non-fs.ErrNotExist
  stat failure (e.g. permission denied on an ancestor .skillrig dir) is surfaced
  as fatal instead of masked as "not found"; ResolveOrigin fails fast. Closes
  the discovery-stage gap symmetric to Load's I/O-fatal path.
- #3 Origin.String() returns "" for the zero Origin (the SourceNone sentinel)
  instead of a misleading "/"; precedence test compares directly.
- #4 gitRoot returns (string, error); ProjectWriteTarget falls back to cwd ONLY
  for expected cases (git absent / not a repo) and propagates unexpected errors
  (context cancellation/timeout, exec failures), so init never writes config to
  the wrong directory.
- #1 (markdown checkbox rule violation): false positive — the cited checkboxes
  are /specledger in-document spec/plan checklists, not work tracking. AGENTS.md
  reworded to scope the rule to work-item tracking and explicitly allow them.

Tests (TDD, real fixtures): zero-origin String, permission-denied walk-up fatal,
cancelled-ctx write-target fatal, non-repo cwd fallback preserved. Gate green:
go test ./... · gofmt · go vet · golangci-lint (0 issues).

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

so0k commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

NOTE: it seems these templates are not used by the sl binary as it bootstraps with embedded templates, closing for now pending upstream bug report

@so0k so0k closed this May 29, 2026
@so0k so0k deleted the worktree-sync-templates-constitution branch May 29, 2026 13:39
so0k added a commit that referenced this pull request May 30, 2026
@so0k @claude
fix(002): resolve adversarial review #2 findings
4b0e33a 
Address the independent post-implementation review (reviews/002-review.md):
correctness + harness bugs, error-navigation gaps, missing unit tiers, and the
local-origin resolution footgun.

Correctness / harness:
- R2-H1: add resolvePlacement looked up the lock by the directory arg while
  writeLockEntry keys it by the manifest name — an identical re-add of a skill
  whose manifest name != its dir was wrongly refused (FR-003). Look up by name;
  regression test for name!=dir.
- R2-H2: the re-exec'd git stub now sets GOCOVERDIR, so `go test -cover
  ./pkg/skillcore/...` no longer leaks a warning into captured git stderr.
- GAP-C: `make test-unit` now runs ./internal/... AND ./pkg/... (skillcore's
  Constitution III tests were silently skipped by the documented unit tier).
- GAP-D: ship an executable check.sh (0o755) in the sample skill so the
  mode-preservation assertion actually exercises the exec bit.

Error navigation (cli.md Principle 1/2):
- R2-M3: custom Args validators on add/verify return what/why/fix + an example
  instead of cobra's terse "accepts 1 arg(s)" / "unknown command".
- R2-M4/AR-2: new *OriginNotFoundError distinguishes a missing local origin
  checkout from a wrong skill name (was conflated into "check the skill name").

Local-origin resolution (AR-1/R2-L6):
- Anchor the origin source to the repo root (filepath.Join(repoRoot, originDir)),
  matching the destination — `add` now works from any subdirectory (was
  CWD-relative and failed from nested dirs). Verified live.

Tests (Constitution III):
- GAP-A: pkg/skillcore/verify_test.go — taxonomy (ok/mismatch/orphan/missing/
  dirty), counts, aggregate-all, dirty-masks-mismatch precedence, empty-repo,
  unsupported lockfileVersion -> *LockError.
- GAP-B: internal/cli/addverify_test.go — exitCodeFor (incl. wrapped
  *VerifyFailure), mapAddError classes, originDirRef, arg validators, renderers.

Docs / tooling:
- R2-L5: cli.md add synopsis corrected to the shipped surface; --origin/--pin
  marked dropped/planned.
- AR-4: add --help, contracts/add.md, skillrig-init + skillrig-add-verify skills
  synced to repo-root resolution + the distinct missing-checkout error.
- checkpoint-workflow review-agent template now loads agentic-go-cli-design +
  golang-code-style/testing/lint, and requires a clean tree before launching.

Gate: make check (0 lint) + go test -cover ./... green (skillcore 79.5%,
internal/cli 51.2%). Deferred: AR-5 (cosmetic stale data-model sample SHA).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced May 30, 2026
Merged
Merged
@skillrig-release-bot skillrig-release-bot Bot mentioned this pull request Jun 1, 2026
Merged
so0k pushed a commit that referenced this pull request Jun 5, 2026
fix(004): address PR #19 review on skillrig show
d97495b 
Resolves the actionable Qodo findings on the show command:

- getwd error now carries a fix: line. Extracted a single
  usageCannotGetwd helper (repo.go) and routed every command's
  os.Getwd failure through it (show/search/add/verify/index/init), so
  the what/why/fix cannot drift per command (AP-06 discipline).
- "search failed" misattribution: generalized mapSearchError into
  mapCatalogError(cmd, origin, err); search passes "search", show
  passes "show", so an unclassified failure names the real command.
- Terminal-injection hardening: catalog text is fetched and untrusted,
  so human output now strips control bytes / neutralizes ANSI escapes
  via sanitizeTerminal (single-line fields drop newlines; show's
  description body keeps them). Applied to both show and search;
  --json is never sanitized. Added unit coverage.
- show unit test now asserts output SHAPE (exact header/field lines +
  footer-as-final-line), not only substrings.
- cli.md: scoped the 80-char/newline truncation rule to compact LIST
  output and documented show as the deliberate full-detail exception
  (issue #17) — show intentionally prints the complete description.

The full-description behavior (Qodo #2) and the runnable footer (Qodo
#3) are intentional: show exists to print the untruncated description,
and per the agentskills.io spec a skill `name` is a lowercase
alphanumeric-hyphen slug (no leading/trailing hyphen, no spaces), so
the footer command is always shell/cobra-safe for conformant origins.

https://claude.ai/code/session_01SAczAuBfsd2hTEGrFBsmQZ
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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@so0k