Skip to content

feat(#577): skill ownership + lifecycle model (P1) - #767

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

feat(#577): skill ownership + lifecycle model (P1)#767
Weegy merged 2 commits into
mainfrom
feat/577-skill-artifacts

Conversation

@Weegy

@Weegy Weegy commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase 1 of #577 (skills as scope-owned, shareable artifacts). Adds the model layer only: ownership, a lifecycle state machine, and HMAC tamper-evidence over a canonical manifest. No resolution/shadowing (P2), no sharing/promotion/cron-guard (P3), no admin UI (P4) — those are separate, stacked phase PRs per the phase cut in docs/plans/phase4a-577-skills-prompt-2026-08-20.md.

Base: origin/main @ 28be3dde (merged in before this commit). This PR's own base branch: main.

What's in this PR

  • Migration middleware/migrations/0040_skill_ownership_lifecycle.sql (next free number — 0038 is reserved for the Satellites epic Epic: Satellites — outbound-only edge nodes (Raspberry Pi class) that pair with a device code, run selected agents, and bridge internal-only systems securely into the main omadia #746, 0039 is turn_receipts, both already on main). Adds four nullable/defaulted columns to the existing skills table (from 0003_agent_builder_graph.sql, the table skillImport.ts/skillLoader.ts/skillGuard.ts already operate on):
    • owner_scope TEXT — the skill's home as a ScopeId wire string (personal:<userId> / group:<groupRef> / org:<orgId>). Nullable: pre-existing imported rows predate ownership and stay unowned until assigned, same posture as content_hash in 0004.
    • lifecycle_status TEXT NOT NULL DEFAULT 'draft' with a CHECK constraint for draft/reviewed/published/archived.
    • manifest_signature TEXT, manifest_signed_at TIMESTAMPTZ — HMAC tamper-evidence, NULL until first signed transition.
  • src/services/skillLifecycle.ts — pure, synchronous model:
    • SkillOwnerScope narrows ScopeId (@omadia/channel-sdk, Scope model with audience-floor permission intersection for shared rooms #575) to personal | group | org — a skill cannot be owned by a conversation/system/unscoped scope.
    • SKILL_LIFECYCLE_EDGES — explicit allowlist of legal transitions (draft→reviewed, reviewed→draft, reviewed→published, published→archived). archived is terminal; every other pair (including same-status) is illegal.
    • requiredCapabilitiesFromFrontmatter — reads frontmatter.requiredCapabilities. Absent → []. Malformed (not an array, or a non-string/blank entry) → throws SkillManifestError naming the exact field/index/value. Never silently drops — the fix(skills): surface frontmatter the SKILL.md import silently drops #690 lesson (frontmatter parser silently dropped data instead of erroring).
    • canonicalSkillManifest — the byte-exact serialization that gets signed: fixed field order (an array of pairs, not an object — immune to key-insertion-order), requiredCapabilities deduped + sorted by plain codepoint comparison (order-independent) but never case-folded (mirrors the feat(#333): Principal — the platform's typed answer to "who is this?" (phase 1) #724 precedent: role keys aren't lowercased because case is semantically significant; same rule applied to capability identifiers here).
    • signSkillManifest / verifySkillManifestSignature — HMAC-SHA256, constant-time compare, never throws on a malformed/wrong-length signature.
    • missingRequiredCapabilities / canPublishSkill — pure set-difference against an already-resolved granted set. Deliberately has no dependency on GrantStore — P3 (sharing + promotion) is the one place that resolves how a capability got granted; this module only checks whether it's present.
    • transitionSkillLifecycle — combines all three P1 invariants (legal move, valid owner scope, publish capability gate) into one pure decision, re-signing the manifest at the new status on success.
  • src/services/skillLifecycleStore.ts — thin Postgres store over the new columns (getSkill, assignPersonalOwner, transition), using raw pool.query rather than a new AgentGraphStore method — keeps this phase's surface to src/services/skill* per the binding surface separation with the parallel Credential keychain with grants and a broker that keeps secrets away from the agent #578 session. assignPersonalOwner is the only direct-assignment path (personal only): Skills as scope-owned, shareable artifacts (grants, org promotion, git skill packs) #577's Kernkonzept explicitly forbids creating a skill directly in team/org scope — those are reached only through admin-gated promotion (P3).
  • Tests: test/skillLifecycle.test.ts (30 cases, pure) + test/skillOwnershipLifecycleStore.pg.test.ts (6 cases, pg-gated, new file — kept separate from the pre-existing skillLifecycleStore.pg.test.ts which covers unrelated Wave-0 content_hash/forkedFrom coverage on AgentGraphStore, so fixtures never collide).

Blast radius

  • New columns only, all nullable/defaulted — no existing read path touches them, no existing write path is required to set them. Zero behavior change for the current skill-import/guard/loader pipeline.
  • No existing file touched — this PR is 5 new files, 0 diffs to existing code. skillImport.ts, skillGuard.ts, skillLoader.ts, agentGraphStore.ts, grants.ts, admin.ts are all untouched, consistent with the binding surface separation.
  • @omadia/channel-sdk consumed only (ScopeId, parseSessionScope, formatSessionScope) — no edits to that package.
  • Full middleware suite: 6915 tests / 6903 pass / 0 fail / 12 pre-existing skips (unrelated) after this change, both before and after origin/main merge + rebuild.

Test + mutation evidence

Ran the full non-pg suite (npm test) and the pg-gated suite for skills against an ephemeral local Postgres — all green, no regressions in the pre-existing skillLifecycleStore.pg.test.ts / skillImport.test.ts / skillHash.test.ts / skillGuard.test.ts.

Manual mutation checks against skillLifecycle.ts (rebuilding nothing extra was needed — these are pure functions run directly via tsx, no dist/ involved; the pg-gated store test was separately verified to exercise the compiled migration against real Postgres):

# Mutation Result
1 Added archived → draft as a legal edge (reopen the terminal state) 2 tests fail: the full 4×4 matrix test and the explicit "archived is terminal" test
2 Lowercased capabilities before sorting in canonicalSkillManifest (case-fold, breaking the #724 precedent) 1 test fails: "does NOT case-fold capabilities"
3 Made requiredCapabilitiesFromFrontmatter silently skip invalid entries instead of throwing (reintroducing the #690 defect shape) 2 tests fail: the index-naming and blank-string error-message tests
4 Disabled the publish capability gate entirely (if (false && target === 'published')) 1 test fails: "blocks publish when a required capability is not granted"

All four mutations were reverted after confirming failure; the working tree is clean (git diff --stat on the mutated file shows no residue).

Not in this PR (later #577 phases)

  • Wiring owner_scope assignment into the existing importSkillMarkdown create path (P2/P3 — a scope-aware import flow).
  • Scope-ordered resolution + shadowing in skillLoader.ts (P2).
  • GrantStore-backed capability resolution, sharing grants, admin-gated org/team promotion, cron write-guard (P3).
  • Any HTTP route or web-ui surface (P4).

Migration number

0040. Verified against origin/main @ merge time: 0038 reserved (#746 Satellites), 0039 is turn_receipts (#757), nothing else claims 0040. The parallel #578 (Keychain) session works in credentials//vault surfaces per the binding separation and should not need this series.


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

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.
@Weegy
Weegy merged commit 18d41b7 into main Aug 20, 2026
9 checks passed
@Weegy
Weegy deleted the feat/577-skill-artifacts branch August 20, 2026 12:39
Weegy added a commit that referenced this pull request Aug 20, 2026
* feat(#577): skill ownership + lifecycle model (P1)

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.

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

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 added a commit that referenced this pull request Aug 20, 2026
Add/add conflicts in three P1-origin files: main carries P1's squash
(#767), this branch carries P1 plus P3's deliberate evolution of the same
files (transition options gained the actor guard, assignPersonalOwner was
reshaped). Verified before resolving: git log on main shows ONLY the #767
squash ever touched these paths, so every main-side-only line is a line P3
changed on purpose — resolved with the branch side.
Weegy added a commit that referenced this pull request Aug 20, 2026
…te-guard (P3) (#771)

* feat(#577): skill ownership + lifecycle model (P1)

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.

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

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.

* feat(#577): sharing via GrantStore + admin-gated promotion + cron write-guard (P3)

Adds the sharing/promotion/write-guard layer on top of P1 (skill
ownership/lifecycle) and P2 (scope-ordered resolution):

- src/services/skillLifecycle.ts: assertHumanActor / SkillAutomationWriteBlocked
  -- the enforced cron write-guard (#577 Kernkonzept #6). Checks
  ScopeId.kind === 'system' (scopeId.ts's own documented boundary: "no
  human is present in any of them" -- routine/schedule/conductor/
  conductor-builder), so it needs no new taxonomy. Threaded as the first
  check in every mutating store method -- a blocked write never reaches
  the database (asserted in the pg tests, not just at the pure-function
  level).

- src/services/skillLifecycleStore.ts:
  - assignPersonalOwner and transition now take an actorScope and call
    assertHumanActor before any query.
  - New promoteSkillOwnerScope(skillId, targetScope, opts): the ONLY way
    a skill reaches team/org ownership (#577 Kernkonzept #5 -- no direct
    creation there). Requires the skill be already 'published', re-signs
    the manifest at the NEW ownerScope + SAME status (promotion is a
    signature-changing event, since ownerScope is a signed field).
    Admin-gating itself (an authenticated-session check) is left to the
    route layer -- this method enforces the cron guard and the
    published-only invariant, nothing about roles.

- src/services/skillSharing.ts: sharing = a grant over GrantStore
  (#575), not a parallel ACL (#577 Kernkonzept #5). Encodes "skill X is
  shared with principal Y" as a Capability string (skill:read:<id>) and
  resolveSharedSkillIds(principal, roles, grants) turns a resolved
  capability set back into the ReadonlySet<string> P2's
  resolveSkillByName needs for its 'shared' bucket. Denials subtract
  from grants (same rule as the audience floor). Deliberately does NOT
  collapse "unresolved" (partial role lookup) to empty at this layer --
  SharedSkillIdsResult keeps the fact visible; toSharedSkillIdsSet is
  the explicit fail-closed adapter for callers who just want the
  resolver input. Consumes GrantStore/resolveCapabilities only -- no
  edits to grants.ts.

Tests: 15 new pure tests (skillSharing.test.ts) covering direct grants,
role-grant union, denial subtraction, unresolved-vs-empty, and the
fail-closed adapter; 2 new pure tests for assertHumanActor (all 4 system
origins blocked, every other ScopeId kind passes); 6 new pg-gated tests
covering the cron guard on all three mutating methods and the full
promotion flow (org target, team target, draft-refusal).

Mutation-tested: disabling assertHumanActor fails 4 tests across both
the pure and pg-gated suites; dropping the published-only gate on
promoteSkillOwnerScope fails 1 pg-gated test. Both reverted; working
tree clean afterward.

Not in this PR: the admin-gated HTTP route itself. Wiring a
session-authenticated Express route touches src/index.ts (the shared
app-bootstrap file, ~3800 lines, high concurrent-edit traffic across
this repo's parallel issue-harness sessions) and needs to correctly
replicate the existing session/auth middleware chain -- a promotion
endpoint with a subtly wrong auth check is a real security regression,
not a place to move fast. The service-layer method
(promoteSkillOwnerScope) is complete and fully tested; mounting it
behind route + session auth is left as a follow-up (naturally lands
with P4's admin UI, which needs a concrete endpoint contract anyway).
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