feat(sdk): expose installed skill metadata and lifecycle commands - #30
feat(sdk): expose installed skill metadata and lifecycle commands#30spencercjh wants to merge 3 commits into
Conversation
Signed-off-by: Spencer Cai <jiahao.cai@dynamia.ai>
Signed-off-by: Spencer Cai <jiahao.cai@dynamia.ai>
Signed-off-by: Spencer Cai <jiahao.cai@dynamia.ai>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3967bdc83a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| normalized, metadata, err := resolveGitHubBundle(bundle.github) | ||
| return normalized, mergeBundledMetadata(metadata, bundle.meta), err |
There was a problem hiding this comment.
When WithBundleMetadata wraps a GitHubBundle, this branch overlays the bundled metadata onto the resolved GitHub metadata, so a supplied SourceID or Provenance replaces the canonical github:owner/repo/path identity and resolved-commit provenance. docs/API.md limits these custom fields to bundled/embedded inputs, and TypeScript and Python do not allow attaching them to GitHub bundles; Go and Rust should reject this combination or preserve the GitHub-owned fields rather than recording inaccurate and cross-language-inconsistent source data.
AGENTS.md reference: AGENTS.md:L110-L114
Useful? React with 👍 / 👎.
| if not is_owned_metadata(payload): | ||
| return None | ||
| return payload | ||
| raise KitupError("invalid installed metadata") | ||
| return InstalledMetadata( | ||
| schema_version=1, |
There was a problem hiding this comment.
When .kitup.json contains "schemaVersion": true, is_owned_metadata accepts it because Python considers True == 1, and this new public reader returns an InstalledMetadata object with schema_version=1 instead of raising KitupError. The TypeScript, Go, and Rust readers reject the same malformed metadata, so explicitly require an integer value that is not a boolean before exposing the record.
Useful? React with 👍 / 👎.
| var meta InstalledMetadata | ||
| if err := json.Unmarshal(data, &meta); err != nil || !isOwnedMetadata(meta) { | ||
| return InstalledMetadata{}, ErrInvalidInstalledMetadata |
There was a problem hiding this comment.
For metadata containing only differently cased required keys such as "APPID" and "SKILLNAME", encoding/json matches the struct tags case-insensitively and this reader returns a managed InstalledMetadata value. TypeScript, Rust, and Python require the documented exact keys, so malformed metadata can be considered owned only by Go; validate the required keys and their types in the raw object before unmarshalling into the struct.
AGENTS.md reference: AGENTS.md:L110-L114
Useful? React with 👍 / 👎.
| INSTALL["Installer\nplan · conflict policy · copy · update · uninstall"]:::execution | ||
| REPORT["Reports\nInstallReport · UninstallReport"]:::execution | ||
| INSTALL["Lifecycle\nplan · copy · update · status · uninstall"]:::execution | ||
| REPORT["Reports\nInstallReport · StatusReport · UninstallReport"]:::execution |
There was a problem hiding this comment.
The diagram places StatusReport inside the shared SDK (ts / go / rust / python) boundary, but a repo-wide search shows that only Go defines StatusReport or a status operation, and the shared case schema has no status operation. Since this file is the canonical architecture map, it currently documents a cross-language report boundary that is neither implemented nor fixture-covered; label status as Go-only or add the missing SDK behavior and golden cases.
AGENTS.md reference: AGENTS.md:L134-L134
Useful? React with 👍 / 👎.
1 participant
Summary
skill statusandskill uninstallcommands with optional JSON outputProblem
Callers could not reliably inspect an installed skill through a public contract, and the Cobra adapter lacked lifecycle commands suitable for either users or automation. Consumers such as hami-cli otherwise had to parse
.kitup.jsondirectly or scrape human-readable terminal output.The metadata schema remains backward compatible. Uninstall only removes kitup-owned directories whose
AppIDmatches, revalidates quarantined metadata before deletion, and does not imply force behavior.Validation
node scripts/check.mjsStack
This is PR 3 of 3 and is stacked on #29, which is stacked on #28. While all three target
mainbecause the branches live in a fork, the review-specific commit is3967bdc.