Skip to content

feat(#577): scope-ordered skill resolution with shadowing (P2) - #768

Merged
Weegy merged 3 commits into
mainfrom
feat/577-skill-artifacts-p2
Aug 20, 2026
Merged

feat(#577): scope-ordered skill resolution with shadowing (P2)#768
Weegy merged 3 commits into
mainfrom
feat/577-skill-artifacts-p2

Conversation

@Weegy

@Weegy Weegy commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase 2 of #577 (skills as scope-owned, shareable artifacts). Adds the scope-ordered resolver with shadowing — no sharing/promotion/cron-guard (P3), no admin UI (P4).

Base: feat/577-skill-artifacts (P1, #767), NOT main. Stacked per the phase-cut plan — this PR should be merged after #767, or #767's diff will show up here until then. GitHub will resolve the base automatically once #767 merges into main and this branch is rebased/retargeted, but for review right now the "Files changed" tab includes P1's 5 files plus this PR's 2 new ones.

What's in this PR

  • src/services/skillResolver.tsresolveSkillByName(name, candidates, ctx), pure and synchronous:
    • Resolves by name, not slugskills.slug is globally UNIQUE (migration 0003), so no schema change was needed: two scopes can already own same-named skills as long as their slugs differ (which the existing uniqueSlug disambiguation in skillImport.ts already guarantees on insert).
    • Strict precedence: personal → shared → team → org (Skills as scope-owned, shareable artifacts (grants, org promotion, git skill packs) #577 Kernkonzept chore(deps,docker,web-dev): Bump node from 20-slim to 26-slim in /web-dev #4), evaluated in that order, stopping at the first non-empty bucket.
    • Membership/sharing are pre-resolved inputs (SkillResolutionContext.memberTeams / .orgId / .sharedSkillIds) — this module never talks to GrantStore directly, same seam P1's missingRequiredCapabilities used for capability grants. P3 is the phase that wires GrantStore to produce sharedSkillIds.
    • Only lifecycleStatus === 'published' candidates are eligible, filtered before bucketing.
    • A same-bucket tie returns { ok: false, reason: 'ambiguous', level, candidates } rather than an arbitrary pick.

The two dangerous cases (both explicitly named in the phase-cut plan, both separately tested + mutated)

  1. Wrong precedence — a personal skill must beat an org skill of the same name, not the reverse. Test: personal beats org even at equal names. Mutated by reversing the bucket evaluation order → 3 tests fail (including the ambiguity-short-circuit test, which also depends on precedence order).
  2. Wrong lifecycle status — an unpublished skill at a higher-precedence level must never outrank a published skill lower down; it must be excluded outright, not merely deprioritized. Test: a draft at a higher-precedence level does NOT win over a published skill at a lower level. Mutated by dropping the lifecycleStatus === 'published' filter → 3 tests fail.

A third, related failure mode — silent tie-breaking within one bucket — is guarded structurally (ambiguous result) and covered by 2 tests (two org skills with the same name are ambiguous, ambiguity at personal short-circuits).

Test coverage (15 tests, test/skillResolver.test.ts, all pure — no pg gate needed)

  • Precedence: personal > shared > team > org, including partial-absence fallthrough at every level.
  • Lifecycle gate: draft / reviewed / archived candidates never resolve, even as the sole candidate.
  • Ownership: unowned (ownerScope: null) never resolves; a personal skill owned by a different user is not auto-matched; team membership in a different team than the owner doesn't match; org candidate doesn't match when the requester has no org.
  • Ambiguity: same-bucket ties are reported, not silently resolved.
  • requesterScope.kind !== 'personal' (e.g. a system/routine scope) correctly leaves the personal bucket empty without erroring.
  • Name matching is case-sensitive (same philosophy as P1's capability identifiers — never fold case on a value this module didn't mint).

Mutation evidence

# Mutation Result
A Reversed bucket evaluation order (org checked before personal) 3 tests fail: personal beats org…, shared beats team beats org, ambiguity at personal short-circuits…
B Dropped the lifecycleStatus === 'published' filter 3 tests fail: the draft-vs-published test, the archived-never-resolves test, the reviewed-never-resolves test

Both mutations reverted after confirming failure; git diff --stat on the mutated file shows no residue afterward.

Blast radius

  • 2 new files, 0 diffs to existing code — same posture as P1. skillLoader.ts (the on-disk SKILL.md parser) is untouched; this resolver is a new, independent module operating over DB-backed candidate rows a caller supplies (P3/P4 wire the actual AgentGraphStore query — out of scope here, kept pure and testable in isolation).
  • Full middleware suite after this change: 6949 tests / 6937 pass / 0 fail / 12 pre-existing skips.

Not in this PR (later #577 phases)


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Weegy added 2 commits August 20, 2026 14:07
Adds the model layer for scope-owned, tamper-evident skills:

- migrations/0040_skill_ownership_lifecycle.sql: owner_scope,
  lifecycle_status (draft/reviewed/published/archived), manifest_signature,
  manifest_signed_at columns on the existing skills table (0003).
- src/services/skillLifecycle.ts: pure model — SkillOwnerScope (ScopeId
  restricted to personal/group/org), the lifecycle transition matrix,
  requiredCapabilities parsing from frontmatter (throws SkillManifestError
  with a field-naming message on malformed input, never silently drops —
  #690 guard), canonical manifest serialization + HMAC-SHA256 sign/verify.
- src/services/skillLifecycleStore.ts: thin Postgres store over the new
  columns (assignPersonalOwner, transition), standalone from
  AgentGraphStore to keep this phase's surface to src/services/skill*.
- test/skillLifecycle.test.ts (30 tests): exhaustive 4x4 transition matrix,
  byte-exact canonical manifest lock, capability-order independence,
  capability case-sensitivity (mirrors #724's role-key precedent), tamper
  detection, malformed-signature handling.
- test/skillOwnershipLifecycleStore.pg.test.ts (6 tests, pg-gated): full
  draft->reviewed->published->archived flow against real Postgres,
  publish-gate rejection, re-signing on every transition.
Adds resolveSkillByName in src/services/skillResolver.ts: given a skill
name and a requester's scope, pick the single winning skill row across
personal -> shared -> team -> org, in that strict order (#577 Kernkonzept
#4). Pure and synchronous, layered on P1's ScopeId/lifecycle model
(#767) without touching GrantStore directly — membership/sharing are
passed in pre-resolved (SkillResolutionContext), same seam P1 used for
capability grants.

Resolves by `name`, not `slug`: skills.slug is globally UNIQUE (0003), so
no schema change is needed for two scopes to own same-named skills.

Two invariants get explicit, separately-mutation-tested coverage:
- precedence order (personal beats org even at equal names -- the
  'non-empty result from the wrong level' danger named in the phase
  spec, parallel to the quorum='all' fail-open lesson from #726)
- the lifecycle gate: only 'published' skills are eligible candidates,
  filtered BEFORE bucketing, so a draft at a higher-precedence level can
  never outrank a published skill lower down.

A third case is guarded structurally: two candidates tying within one
bucket return an explicit 'ambiguous' result (level + full candidate
list) rather than an arbitrary pick -- same 'absence/uncertainty is a
type' posture as resolveCapabilities (#575) and RoleSourceRegistry
(#333).

15 tests in test/skillResolver.test.ts, all pure (no pg gate needed).
Mutation-tested: reversing bucket precedence order fails 3 tests;
dropping the published-status filter fails 3 tests.
@Weegy
Weegy deleted the branch main August 20, 2026 12:39
@Weegy Weegy closed this Aug 20, 2026
@Weegy Weegy reopened this Aug 20, 2026
@Weegy
Weegy changed the base branch from feat/577-skill-artifacts to main August 20, 2026 12:41
@Weegy
Weegy merged commit 1fc0421 into main Aug 20, 2026
9 checks passed
@Weegy
Weegy deleted the feat/577-skill-artifacts-p2 branch August 20, 2026 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant