feat(cli): complete governed Skill lifecycle - #240
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe CLI now supports verified Skill installation management through ChangesSkill CLI lifecycle and release
Estimated code review effort: 5 (Critical) | ~90+ minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant CLI
participant Registry
participant InstallState
Operator->>CLI: run verify, update, or remove
CLI->>InstallState: inspect receipt and installation tree
InstallState-->>CLI: verification status
alt update
CLI->>Registry: authorize and download exact release
Registry-->>CLI: manifest and package bytes
CLI->>InstallState: stage and commit replacement
else remove
CLI->>InstallState: quarantine verified installation
end
InstallState-->>CLI: lifecycle result
CLI-->>Operator: table or JSON output
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TegamiThis repository uses Tegami to manage releases. When your changes affect published packages, add a changelog file under Create a changelog → · Changelog format Release preview
Changelogs in this PR
Run Managed by Tegami. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-cli.yml:
- Around line 32-35: Document each permission in the workflow’s permissions
block with an adjacent brief comment: identify actions: read as required by gh
run list, contents: read as required for checkout, and id-token: write as
required for OIDC Trusted Publishing.
- Around line 106-124: Update the “Verify registry release and provenance” step
to explicitly detect an empty published value after the retry loop and fail with
a clear registry propagation timeout message before invoking JSON.parse.
Consider extending or backing off the existing 12-attempt, 5-second retry window
while preserving the current validation for successfully retrieved package
metadata.
In `@apps/cli/package.json`:
- Around line 4-6: Update the package metadata for the public `@orgmemory/cli`
package by replacing the "UNLICENSED" value in package.json with the applicable
SPDX license identifier or a valid "SEE LICENSE IN <filename>" reference
matching the included license text.
In `@apps/cli/src/index.ts`:
- Around line 244-260: Update the skill verification flow around the
options.json output branch to compute whether any result entry has a status
other than "verified" before branching into JSON or table output, and set
process.exitCode to 2 for that failure condition in both formats. Preserve the
existing JSON serialization, empty-result handling, and table output behavior.
In `@apps/cli/src/install.test.ts`:
- Around line 157-178: Add two tests alongside the concurrent-install coverage:
seed .orgmemory/skills.lifecycle.lock/owner.json with a non-running PID and
verify installSkill reclaims the abandoned lock, then seed it with a live
foreign PID and verify the lock is not reclaimed and the documented
mutexIsAbandoned behavior is preserved. Use the existing temporary-directory
setup and installation helpers, and ensure each test exercises the
mutexIsAbandoned decision path.
- Around line 97-121: Update the test named “marks additions, links, and changed
bytes as modified” to actually cover the symlink handling in readInstalledTree
by importing and using symlink from node:fs/promises to create a link in the
installed tree, while retaining the existing addition and changed-bytes
coverage.
In `@apps/cli/src/install.ts`:
- Around line 736-791: Update mutexIsAbandoned to treat only an ESRCH failure
from process.kill as evidence that the owner process is gone; treat EPERM and
other errors as an active owner. In withScopeLock, replace unconditional mutex
removal with an owner-PID check so cleanup removes the lock only when it still
belongs to the current process and cannot delete a replacement owner’s lock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 18887f88-a622-4b93-9c57-112be1a779a8
⛔ Files ignored due to path filters (8)
docs/guidelines/cli-publication.mdis excluded by!docs/**docs/increments/active/2026-08-01-skill-cli-distribution-lifecycle/challenge-brief.mdis excluded by!docs/**docs/increments/active/2026-08-01-skill-cli-distribution-lifecycle/challenge-verdict.mdis excluded by!docs/**docs/increments/active/2026-08-01-skill-cli-distribution-lifecycle/design.mdis excluded by!docs/**docs/increments/active/2026-08-01-skill-cli-distribution-lifecycle/plan.mdis excluded by!docs/**docs/roadmap.mdis excluded by!docs/**docs/specs/domains/asset-registry.mdis excluded by!docs/**docs/tests/domains/asset-registry.mdis excluded by!docs/**
📒 Files selected for processing (14)
.github/workflows/ci.yml.github/workflows/publish-cli.yml.tegami/skill-cli-lifecycle.mdARCHITECTURE.mdapps/cli/README.mdapps/cli/package.jsonapps/cli/src/contracts.tsapps/cli/src/index.tsapps/cli/src/install.test.tsapps/cli/src/install.tsapps/docs/content/docs/product-guides/work-with-governed-assets.mdxapps/docs/content/docs/product-guides/work-with-governed-assets.vi.mdxpackage.jsonscripts/npm-publish-workflow-policy.test.mjs
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: Neo4j adapter
- GitHub Check: OpenSearch adapter
- GitHub Check: Web · Node 24
- GitHub Check: Public docs · Node 24
- GitHub Check: PostgreSQL GraphRAG
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Treat the repository and runtime evidence as the engineering system of record; do not treat chat or Northstar as authoritative.
Before changing a domain, read its specification, test-coverage document, and applicable decision filenames.
Material decisions about domain boundaries, authorization, persistence, publication, concurrency, cache isolation, parity scope, or deployment require an independent architecture challenge and documented alternatives before implementation.
Do not use completed increment documents as the source for current behavior; use them only for history or archaeology.
Before using unfamiliar Spring Boot, Spring Modulith, Spring AI, Gradle, React, Vite, Tailwind, TypeScript, Next.js, or Fumadocs APIs, consult current official documentation, Context7, and the relevant verification skill.
Readdocs/guidelines/agent-safety.mdbefore retrieval, AI, MCP, permission, upload, graph, or export work; never commit secrets or customer data.
Keepddl-auto=validateand pair every persisted-model change with a Flyway migration.
Use the testing harness; a terminating clean test is the JVM context gate, andbootRunis not verification.
Files:
package.jsonapps/docs/content/docs/product-guides/work-with-governed-assets.vi.mdxapps/cli/package.jsonapps/docs/content/docs/product-guides/work-with-governed-assets.mdxscripts/npm-publish-workflow-policy.test.mjsapps/cli/README.mdARCHITECTURE.mdapps/cli/src/contracts.tsapps/cli/src/install.test.tsapps/cli/src/index.tsapps/cli/src/install.ts
.github/**/*.{yml,yaml}
⚙️ CodeRabbit configuration file
.github/**/*.{yml,yaml}: Require least-privilege permissions, explicit release tags for actions,
bounded job timeouts, concurrency cancellation, frozen lockfiles, and no
secrets in pull-request workflows. GitHub Actions are intentionally not
pinned to commit SHAs; Dependabot owns their scheduled version updates.
Files:
.github/workflows/publish-cli.yml.github/workflows/ci.yml
apps/docs/content/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (apps/docs/CLAUDE.md)
apps/docs/content/docs/**/*.{md,mdx}: Public prose must live only inapps/docs/content/docs; internal engineering documents are source evidence, not publication input.
sourceRefsis build-time traceability metadata and must never be rendered in published content.
Draft pages must remain excluded unlessDOCS_INCLUDE_DRAFTS=trueis set for a local or controlled preview.
Files:
apps/docs/content/docs/product-guides/work-with-governed-assets.vi.mdxapps/docs/content/docs/product-guides/work-with-governed-assets.mdx
apps/docs/content/docs/**/*.mdx
📄 CodeRabbit inference engine (apps/docs/CLAUDE.md)
Use
index.mdxfor a section root and<slug>.mdxfor a named English page; add Vietnamese pages individually as adjacentindex.vi.mdxor<slug>.vi.mdxfiles, using the/vi/docs/...route.
Files:
apps/docs/content/docs/product-guides/work-with-governed-assets.vi.mdxapps/docs/content/docs/product-guides/work-with-governed-assets.mdx
apps/docs/content/docs/**/*.vi.mdx
📄 CodeRabbit inference engine (apps/docs/CLAUDE.md)
Vietnamese routes must visibly fall back to the reviewed English page until the matching
.vi.mdxexists; a fallback route is not a completed translation.
Files:
apps/docs/content/docs/product-guides/work-with-governed-assets.vi.mdx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend changes must pass lint, typecheck, tests, and a production build; perform browser verification when the flow warrants it.
Files:
apps/cli/src/contracts.tsapps/cli/src/install.test.tsapps/cli/src/index.tsapps/cli/src/install.ts
🧠 Learnings (4)
📚 Learning: 2026-07-24T22:52:57.466Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 40
File: .github/workflows/ci.yml:126-126
Timestamp: 2026-07-24T22:52:57.466Z
Learning: In this repository’s GitHub Actions workflows, the `uses:` field may intentionally reference GitHub Actions by explicit release tags (not immutable commit SHAs) per the project’s OrgMemory policy. Do not flag tag-based `uses:` references as “unpinned” if they are release-tag-based (e.g., `owner/repovX.Y.Z`) and follow the repo’s Dependabot-owned scheduled updates approach.
Applied to files:
.github/workflows/publish-cli.yml.github/workflows/ci.yml
📚 Learning: 2026-07-28T20:06:14.930Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 112
File: apps/docs/content/docs/developers/api-reference/administration.mdx:11-11
Timestamp: 2026-07-28T20:06:14.930Z
Learning: In OrgMemory public documentation MDX files, ensure any `lastReviewed` dates are set using the repository project timezone `Asia/Bangkok`, not the reviewer’s local timezone or the runtime/build timezone. When generating or updating API-reference pages, intentionally use the exact same `lastReviewed` date convention as the authored public documentation pages to keep timestamps consistent across both sources.
Applied to files:
apps/docs/content/docs/product-guides/work-with-governed-assets.vi.mdxapps/docs/content/docs/product-guides/work-with-governed-assets.mdx
📚 Learning: 2026-07-28T20:06:15.064Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 112
File: apps/docs/content/docs/developers/api-reference/authentication.mdx:12-12
Timestamp: 2026-07-28T20:06:15.064Z
Learning: In OrgMemory docs/tests that describe date-based review checks, treat the project timezone as Asia/Bangkok for all “date” comparisons/validations. Do not assume UTC when interpreting or validating fields like `lastReviewed`. For example, `lastReviewed: '2026-07-29'` should be considered valid when the effective “current date” in Asia/Bangkok is July 29, 2026 (not merely when the UTC date matches).
Applied to files:
apps/docs/content/docs/product-guides/work-with-governed-assets.vi.mdxapps/docs/content/docs/product-guides/work-with-governed-assets.mdx
📚 Learning: 2026-07-30T06:46:08.031Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 144
File: apps/docs/public-content.manifest.json:13-67
Timestamp: 2026-07-30T06:46:08.031Z
Learning: For the Fumadocs docs portal under apps/docs, follow the expected MDX filename conventions that drive section-root vs named-page routing. Use `index.mdx` for English section-root pages and `<slug>.mdx` for named pages. For Vietnamese, use adjacent locale-suffixed files: `index.vi.mdx` for section roots and `<slug>.vi.mdx` for named pages. Do not require or enforce a `page.mdx` filename; doing so would change/break the intended section-root route structure.
Applied to files:
apps/docs/content/docs/product-guides/work-with-governed-assets.vi.mdxapps/docs/content/docs/product-guides/work-with-governed-assets.mdx
🪛 markdownlint-cli2 (0.23.1)
.tegami/skill-cli-lifecycle.md
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🪛 OpenGrep (1.26.0)
apps/cli/src/install.ts
[WARNING] 63-63: Sequelize.literal() with dynamic input can lead to SQL injection. Use parameterized queries or model methods instead.
(coderabbit.sql-injection.sequelize-literal)
[WARNING] 67-67: Sequelize.literal() with dynamic input can lead to SQL injection. Use parameterized queries or model methods instead.
(coderabbit.sql-injection.sequelize-literal)
[WARNING] 77-77: Sequelize.literal() with dynamic input can lead to SQL injection. Use parameterized queries or model methods instead.
(coderabbit.sql-injection.sequelize-literal)
[WARNING] 82-82: Sequelize.literal() with dynamic input can lead to SQL injection. Use parameterized queries or model methods instead.
(coderabbit.sql-injection.sequelize-literal)
[WARNING] 105-105: Sequelize.literal() with dynamic input can lead to SQL injection. Use parameterized queries or model methods instead.
(coderabbit.sql-injection.sequelize-literal)
🪛 zizmor (1.28.0)
.github/workflows/publish-cli.yml
[error] 38-38: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 67-67: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 72-72: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 33-33: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🔇 Additional comments (16)
apps/cli/src/contracts.ts (1)
123-158: LGTM!apps/cli/src/index.ts (2)
199-201: LGTM!Also applies to: 264-326, 423-447
4-4: 🩺 Stability & AvailabilityNo issue.
tsconfig.build.jsonemits source files fromsrc/todist/, so../package.jsonresolves next todist/index.jsas the package root.apps/cli/src/install.ts (1)
44-123: LGTM!Also applies to: 136-163, 165-196, 198-253, 255-307, 538-634, 636-727, 793-909, 911-992
apps/cli/src/install.test.ts (1)
60-68: LGTM!Also applies to: 77-95, 123-155, 180-320, 401-423, 443-469, 485-536, 549-560
.github/workflows/publish-cli.yml (2)
38-38: Unpinned action references are intentional.zizmor flags
actions/checkout@v7.0.1,pnpm/action-setup@v6.0.9, andactions/setup-node@v7.0.0as unpinned. This repository intentionally references actions by explicit release tags, not commit SHAs, and Dependabot owns their scheduled updates.Based on learnings, "the
uses:field may intentionally reference GitHub Actions by explicit release tags (not immutable commit SHAs) per the project's OrgMemory policy" and "Dependabot-owned scheduled updates approach" .Also applies to: 67-67, 72-72
Source: Learnings
1-124: LGTM!.tegami/skill-cli-lifecycle.md (1)
1-13: MD041 warning is a false positive for this file type.markdownlint-cli2 flags the missing top-level heading, but this file follows the standard changeset entry format: YAML frontmatter followed by an
## Improvementssection. That pattern doesn't call for an H1 as the literal first line.Source: Linters/SAST tools
apps/cli/package.json (1)
17-20: LGTM!scripts/npm-publish-workflow-policy.test.mjs (1)
1-37: LGTM!.github/workflows/ci.yml (1)
174-174: LGTM!package.json (1)
17-17: LGTM!ARCHITECTURE.md (1)
45-48: LGTM!Also applies to: 105-111
apps/cli/README.md (1)
22-24: LGTM!Also applies to: 34-35, 48-56, 67-76
apps/docs/content/docs/product-guides/work-with-governed-assets.mdx (1)
100-113: LGTM!apps/docs/content/docs/product-guides/work-with-governed-assets.vi.mdx (1)
102-115: LGTM!
1 participant
Summary
@orgmemory/clifor independent public npm distribution through a protected manual OIDC Trusted Publishing workflowArchitecture challenge
gpt-5.6-sol ultraverdict: accept with changes--forcenpxactivation until registry proofVerification
pnpm check:cli(49 tests)0.1.0)pnpm check:web(56 tests + production build)pnpm release:checkgit diff --checkActivation boundary
This PR does not render an npm command in the product. The first public publish still requires verified owner control of the npm
orgmemoryorganization and the documented short-lived bootstrap. The exact pinnednpxhandoff will be a second PR only after registry/provenance verification.Summary by CodeRabbit
New Features
skill verify,skill update, andskill removecommands, with JSON output support.Documentation
Release