Skip to content

fix: skill catalog loses owner identity in API v1 - #3580

Open
goutamadwant wants to merge 6 commits into
openclaw:mainfrom
goutamadwant:fix/skill-list-owner-contract
Open

fix: skill catalog loses owner identity in API v1#3580
goutamadwant wants to merge 6 commits into
openclaw:mainfrom
goutamadwant:fix/skill-list-owner-contract

Conversation

@goutamadwant

@goutamadwant goutamadwant commented Sep 3, 2026

Copy link
Copy Markdown

Closes #3575

What Problem This Solves

Fixes an issue where API v1 skill-catalog consumers could not distinguish skills with the same slug when they belonged to different owners. It also resolves response validation failures for catalog entries that do not have a public latest version.

Why This Change Was Made

The canonical list response now includes the resolved ownerHandle, represents an unavailable latest version as null, and keeps the shared schema, generated schema package, and OpenAPI contract aligned. CLI explore output uses ownerHandle/slug so current catalog identity remains unambiguous.

The CLI supports configured custom registries that may still return the previous list shape. Its compatibility decoder therefore accepts omitted ownerHandle and latestVersion fields and falls back to slug-only and unknown-version display, while the canonical server and published API schemas continue to require owner-qualified identity and an explicit nullable version.

User Impact

API and CLI consumers can identify every current-registry skill by its owner-qualified name, including across cursor pages and when different owners publish the same slug. Entries without a public version satisfy the documented response contract. Existing custom registries remain usable during a rolling upgrade instead of failing CLI schema validation.

Evidence

Before the fix, focused regressions showed that the HTTP list handler omitted ownerHandle and the strict schema rejected an explicit latestVersion: null. After the fix, a deterministic four-page catalog fixture validates every response against ApiV1SkillListResponseSchema, preserves two distinct owners sharing one slug, retains an exact SemVer build-metadata value, and accepts the final entry with no public version.

A CLI compatibility regression parses the previous registry response shape without ownerHandle or latestVersion; a formatter regression proves explore output falls back to the original slug-only form. The canonical package schema remains strict and continues to require ownerHandle and latestVersion.

Validation after merging current main at 8c2de6c5 into signed head 904a50622f3b9c081eaa3f4cec3cee081fee4e85:

bun run --cwd packages/clawhub test -- src/schema/schemas.test.ts src/cli/commands/skills.test.ts
bunx vitest run convex/httpApiV1.handlers.test.ts convex/skills.publicListCursor.test.ts packages/schema/src/schemas.test.ts
CLAWHUB_LOCAL_CHECK_MODE=full bun run ci:pr
git diff --check upstream/main...HEAD

Results: focused CLI/schema tests passed (101/101), focused server/shared-schema tests passed (525/525), and the aggregate PR gate passed with the static audit, formatting, lint, dead-code analysis, 6,665 unit tests (3 skipped), package verification, production type/build checks, and secretless HTTP/CLI E2E coverage. The refreshed aggregate run used Bun 1.4.0; repository CI remains pinned to Bun 1.3.10.

The behavior proof uses deterministic handler and CLI fixtures. A live after-fix HTTP trace is unavailable because there is no configured local Convex deployment for this branch and the fix is not deployed to the public endpoint. Convex code generation was not required because no generated API signatures changed. The Vercel preview requires OpenClaw Foundation authorization, which is separate from the patch.

Disclosure: AI was used to understand the codebase and review the fix.

@clawsweeper

clawsweeper Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@vercel

vercel Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@goutamadwant is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 3, 2026
@clawsweeper

clawsweeper Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed September 11, 2026, 9:32 PM ET / September 12, 2026, 01:32 UTC (Revision 21).

ClawSweeper review

What this changes

Adds publisher identity to skill catalog responses, aligns nullable version schemas, and displays owner-qualified CLI results while accepting older registry responses.

Merge readiness

Blocked before merge - 3 items remain

This PR remains necessary: current main still has the catalog identity and nullability defects. No blocking code findings remain, including the previously reported legacy-registry compatibility issue, but real behavior proof is still required.

Priority: P2
Reviewed head: 904a50622f3b9c081eaa3f4cec3cee081fee4e85

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The focused patch resolves the previous compatibility finding, but fixture-only evidence leaves the runtime proof gate open.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: Needs real behavior proof before merge: the captured body supplies mocked handler and CLI fixtures, but no after-fix run through the real Convex catalog query, HTTP endpoint, and CLI. The prior request for cursor-page identity, exact version, null-version, and legacy-registry evidence remains unmet. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: Needs real behavior proof before merge: the captured body supplies mocked handler and CLI fixtures, but no after-fix run through the real Convex catalog query, HTTP endpoint, and CLI. The prior request for cursor-page identity, exact version, null-version, and legacy-registry evidence remains unmet. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 9 items Current main still needs the fix: The current-main HTTP projection emits slug without ownerHandle and explicitly emits null when no public version exists.
Current schema mismatch: The shared schema on main permits an omitted latestVersion but rejects explicit null, confirming the reported response-contract defect.
Latest release retains the old decoder: The v0.23.3 CLI schema also lacks ownerHandle and accepts optional, non-null latestVersion; the requested decoder correction is not present in that release.
Findings None None.
Security None None.

How this fits together

ClawHub’s catalog API turns public skill records from Convex into paginated JSON consumed by registry clients. The CLI validates those responses and displays skill identities and versions.

flowchart LR
  A[Public skill records] --> B[Convex catalog query]
  B --> C[Owner and public version projection]
  C --> D[Paginated HTTP response]
  D --> E[Client schema validation]
  E --> F[CLI skill listing]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: Needs real behavior proof before merge: the captured body supplies mocked handler and CLI fixtures, but no after-fix run through the real Convex catalog query, HTTP endpoint, and CLI. The prior request for cursor-page identity, exact version, null-version, and legacy-registry evidence remains unmet. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - Fixture tests do not establish actual Convex cursor traversal or end-to-end HTTP and CLI behavior after the fix.
  • Complete next step (P2) - Add after-fix real Convex HTTP and CLI evidence for the retained proof request. Terminal screenshots or recordings are welcome; copied output and logs also count. Redact private IPs, keys, phone numbers, and endpoints. Updating the PR body should trigger another review; otherwise ask a maintainer to comment @clawsweeper re-review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Diff size +208/-10 across 16 files The change spans backend projection, client contracts, generated schemas, documentation, and regression coverage.
Production and test LOC Production source +13/-4; tests +178/-1 Production growth is limited to the stated identity and nullability repair; generated artifacts and documentation are excluded.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3575
Summary: This PR is the explicit candidate fix for the open catalog-contract report.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Keep owner resolution in the existing digest projection, publish a consistent nullable-version contract, and retain the CLI’s tolerant decoder for older registries.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Keep owner resolution in the existing digest projection, publish a consistent nullable-version contract, and retain the CLI’s tolerant decoder for older registries.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes both defects: main omits owner identity and emits null versions that its shared schema rejects. No live reproduction was executed in this read-only review.

Is this the best way to solve the issue?

Yes, the patch uses the appropriate existing projection and decoder boundaries. Replacing the catalog endpoint or filtering every versionless skill would be broader and would change existing catalog behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 8c2de6c506bb.

Labels

Label justifications:

  • P2: This repairs a bounded catalog identity and response-validation defect without evidence of an urgent core outage.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Needs real behavior proof before merge: the captured body supplies mocked handler and CLI fixtures, but no after-fix run through the real Convex catalog query, HTTP endpoint, and CLI. The prior request for cursor-page identity, exact version, null-version, and legacy-registry evidence remains unmet. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Current main still needs the fix: The current-main HTTP projection emits slug without ownerHandle and explicitly emits null when no public version exists. (convex/httpApiV1/skillsV1.ts:1502, 8c2de6c506bb)
  • Current schema mismatch: The shared schema on main permits an omitted latestVersion but rejects explicit null, confirming the reported response-contract defect. (packages/schema/src/schemas.ts:347, 8c2de6c506bb)
  • Latest release retains the old decoder: The v0.23.3 CLI schema also lacks ownerHandle and accepts optional, non-null latestVersion; the requested decoder correction is not present in that release. (packages/clawhub/src/schema/schemas.ts, 87ca030c30f3)
  • Existing owner resolution reused: The patch carries the existing digest-resolved handle through the lean list projection. digestToOwnerInfo supplies a fallback handle for visible handle-less owners; trending already carries that same owner information. Public visibility and version-selection checks remain intact. (convex/skills.ts:6133, 904a50622f3b)
  • Prior compatibility finding resolved: The CLI decoder now accepts omitted ownerHandle and omitted or null latestVersion, with regression coverage and slug-only display fallback. Its schema file is unchanged from the previous reviewed head; the earlier blocker should not be raised again. (packages/clawhub/src/schema/schemas.ts:508, 904a50622f3b)
  • Captured proof remains fixture-only: The complete supplied PR body, captured with sourceRevision f3dd98d1b56346556bf0db947058bb1fa32624887213ea0fa823561281fd0022, explicitly says there is no live after-fix HTTP trace. The four-page handler test mocks runQuery, and the backend cursor test mocks getPage. Reported aggregate checks are supplemental and were not rerun during this read-only review. (convex/httpApiV1.handlers.test.ts:2982, 904a50622f3b)

Likely related people:

  • Patrick-Erichsen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Provide redacted real HTTP and CLI output showing owner collisions across tied-sort cursor pages, exact SemVer text, null versions, and compatibility with an older registry response.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (20 earlier review cycles; latest 8 shown)
  • reviewed 2026-09-03T23:42:36.727Z sha 6e886fd :: needs real behavior proof before merge. :: [P1] Keep latestVersion optional in the legacy CLI decoder
  • reviewed 2026-09-04T00:29:43.125Z sha 6e886fd :: needs real behavior proof before merge. :: [P1] Keep latestVersion optional in the legacy registry decoder
  • reviewed 2026-09-04T01:39:08.375Z sha 6e886fd :: needs real behavior proof before merge. :: [P1] Accept omitted latestVersion in legacy registry responses
  • reviewed 2026-09-04T17:01:46.061Z sha 6e886fd :: needs real behavior proof before merge. :: [P1] Keep latestVersion optional in the legacy CLI decoder
  • reviewed 2026-09-04T18:56:39.898Z sha 6e886fd :: needs real behavior proof before merge. :: [P1] Accept omitted latestVersion in the legacy decoder
  • reviewed 2026-09-05T01:05:48.458Z sha 6e886fd :: needs real behavior proof before merge. :: [P1] Keep latestVersion optional in the legacy CLI decoder
  • reviewed 2026-09-05T03:40:45.395Z sha 9df0733 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-10T09:45:08.949Z sha 01bc62c :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 High-priority user-facing bug, regression, or broken workflow. P2 Normal backlog priority with limited blast radius. and removed P2 Normal backlog priority with limited blast radius. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Sep 3, 2026
@goutamadwant
goutamadwant force-pushed the fix/skill-list-owner-contract branch from 9df0733 to 01bc62c Compare September 10, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API v1 skill catalog loses owner identity and has latestVersion schema drift

1 participant