From 1b47f6eb16ffc277f2dec0bf274533ceb9279e04 Mon Sep 17 00:00:00 2001 From: Michael Magan Date: Sun, 23 Aug 2026 11:45:48 -0700 Subject: [PATCH] Fix issues found by an Opus review pass across workflows, CLI source, and the skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical: bump-homebrew.yml and clawhub-publish.yml never actually ran on a real release. Both trigger on release: published, but release.yml creates that release with the default GITHUB_TOKEN, and GitHub does not let events from that token trigger other workflows. Both are now called directly as jobs (workflow_call) right after npm publish, instead of relying on the release event. Other fixes: - bump-homebrew.yml: curl now fails loudly on a bad download instead of hashing an error body as a fake sha256; version is validated as bare semver and passed through env: instead of interpolated into the shell; the formula sed rewrite is now asserted to have actually taken. - src/commands.ts: CLI_VERSION was a hardcoded literal that was already stale one release behind, mislabeling agent-context output and every pairing token. Now read from package.json like --version already is. - src/argv.ts: --timeout was missing from the recognized global flags, so it errored as an unknown command when placed before the subcommand, unlike --base-url and --token. - README.md: still said "Not published yet" on the actual npm package page for a package that's been live for two releases. - skills/charming/templates/crud moved from the repo root into the skill folder — ClawHub and Hermes only sync skills//, so the template the skill tells agents to always copy was invisible to both. - templates/crud/ui.js: the 4s poll did a full #app innerHTML replace, wiping half-typed input and cutting the undo toast's display window short every time it fired. #toast and the list are now separate, independently-updated containers, and in-progress input in #add-input is preserved across a background refresh. item.id/item.text are now HTML-escaped before interpolation (was unescaped, stored XSS in every app copied from the template). Verified: bun run check passes clean (112 tests). Rebuilt, packed, and globally installed the tarball fresh — confirmed --version and agent-context both report 0.1.1, --timeout works before the subcommand, and the template ships under skills/charming/templates/crud. Re-ran a clawhub sync dry-run against the moved skill folder — still resolves correctly (6 files, status: update). Not fixed (cosmetic, deferred): stale @usecharming/cli name in bun.lock, workflow step ordering in release.yml, no concurrency group on the release workflows. --- .github/workflows/bump-homebrew.yml | 87 ++++++---- .github/workflows/clawhub-publish.yml | 24 ++- .github/workflows/release.yml | 25 +++ README.md | 4 +- package.json | 1 - skills/charming/SKILL.md | 2 +- .../charming/templates}/crud/module.js | 0 .../charming/templates}/crud/styles.css | 0 skills/charming/templates/crud/ui.js | 151 ++++++++++++++++++ src/argv.test.ts | 9 ++ src/argv.ts | 2 +- src/commands.ts | 11 +- templates/crud/ui.js | 113 ------------- 13 files changed, 270 insertions(+), 159 deletions(-) rename {templates => skills/charming/templates}/crud/module.js (100%) rename {templates => skills/charming/templates}/crud/styles.css (100%) create mode 100644 skills/charming/templates/crud/ui.js delete mode 100644 templates/crud/ui.js diff --git a/.github/workflows/bump-homebrew.yml b/.github/workflows/bump-homebrew.yml index f97f4ed..d34b6a0 100644 --- a/.github/workflows/bump-homebrew.yml +++ b/.github/workflows/bump-homebrew.yml @@ -1,9 +1,11 @@ name: Bump Homebrew formula # Keeps tambo-labs/homebrew-tap's charming.rb pointed at the latest -# published npm tarball. Triggers off this repo's own GitHub Release -# (created by release.yml), so it only ever points at a version that -# already published successfully. +# published npm tarball. Called directly by release.yml as a job +# (workflow_call) right after a successful npm publish, not left to +# trigger off `release: published` — GitHub doesn't let events from +# the default GITHUB_TOKEN (which creates that release) trigger other +# workflows, so that path silently never ran. # # Uses HOMEBREW_TAP_TOKEN, a fine-grained PAT scoped to just # tambo-labs/homebrew-tap with Contents: read/write — not the @@ -12,13 +14,21 @@ name: Bump Homebrew formula # disabled and has never run). on: - release: - types: [published] + workflow_call: + inputs: + version: + description: Version to bump the formula to (no leading v, e.g. 0.1.2) + required: true + type: string + secrets: + HOMEBREW_TAP_TOKEN: + required: true workflow_dispatch: inputs: version: description: Version to bump the formula to (no leading v, e.g. 0.1.2) required: true + type: string permissions: contents: read @@ -27,21 +37,27 @@ jobs: bump: runs-on: ubuntu-latest steps: - - name: Resolve version - id: version + - name: Validate version + env: + VERSION: ${{ inputs.version }} run: | - version="${{ inputs.version }}" - if [ -z "$version" ]; then - version="${GITHUB_REF_NAME#v}" + if ! [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "::error::version must be a bare semver like 0.1.2, got: $VERSION" >&2 + exit 1 fi - echo "version=$version" >> "$GITHUB_OUTPUT" + echo "version=$VERSION" >> "$GITHUB_ENV" - name: Download tarball and compute sha256 id: tarball + env: + VERSION: ${{ env.version }} run: | - version="${{ steps.version.outputs.version }}" - url="https://registry.npmjs.org/usecharming/-/usecharming-${version}.tgz" - curl -sL -o package.tgz "$url" + url="https://registry.npmjs.org/usecharming/-/usecharming-${VERSION}.tgz" + curl -fsSL -o package.tgz "$url" + if ! tar -tzf package.tgz >/dev/null; then + echo "::error::downloaded file is not a valid tarball — $url likely 404'd" >&2 + exit 1 + fi sha256=$(shasum -a 256 package.tgz | cut -d' ' -f1) echo "url=$url" >> "$GITHUB_OUTPUT" echo "sha256=$sha256" >> "$GITHUB_OUTPUT" @@ -54,12 +70,19 @@ jobs: - name: Update formula working-directory: homebrew-tap + env: + URL: ${{ steps.tarball.outputs.url }} + SHA256: ${{ steps.tarball.outputs.sha256 }} run: | - version="${{ steps.version.outputs.version }}" - url="${{ steps.tarball.outputs.url }}" - sha256="${{ steps.tarball.outputs.sha256 }}" - sed -i "s#^ url \".*\"# url \"${url}\"#" Formula/charming.rb - sed -i "s#^ sha256 \".*\"# sha256 \"${sha256}\"#" Formula/charming.rb + sed -i "s#^ url \".*\"# url \"${URL}\"#" Formula/charming.rb + sed -i "s#^ sha256 \".*\"# sha256 \"${SHA256}\"#" Formula/charming.rb + # Assert the rewrite actually took, not just that *something* changed — + # a formatting drift that makes the sed anchors stop matching would + # otherwise leave the formula untouched while still reporting success. + if ! grep -qF "$URL" Formula/charming.rb || ! grep -qF "$SHA256" Formula/charming.rb; then + echo "::error::sed rewrite didn't take — check Formula/charming.rb's url/sha256 line format" >&2 + exit 1 + fi if git diff --quiet; then echo "changed=false" >> "$GITHUB_ENV" else @@ -69,29 +92,29 @@ jobs: - if: env.changed == 'true' name: Commit and push working-directory: homebrew-tap + env: + VERSION: ${{ env.version }} + GH_TOKEN: ${{ secrets.HOMEBREW_TAP_TOKEN }} run: | - version="${{ steps.version.outputs.version }}" git config user.name "charming-cli-release[bot]" git config user.email "charming-cli-release[bot]@users.noreply.github.com" - git switch -C "bump/charming-${version}" + git switch -C "bump/charming-${VERSION}" git add Formula/charming.rb - git commit -m "charming ${version}" - git push --force origin "bump/charming-${version}" - env: - GH_TOKEN: ${{ secrets.HOMEBREW_TAP_TOKEN }} + git commit -m "charming ${VERSION}" + git push --force origin "bump/charming-${VERSION}" - if: env.changed == 'true' name: Open PR working-directory: homebrew-tap + env: + VERSION: ${{ env.version }} + GH_TOKEN: ${{ secrets.HOMEBREW_TAP_TOKEN }} run: | - version="${{ steps.version.outputs.version }}" - count=$(gh pr list --repo tambo-labs/homebrew-tap --head "bump/charming-${version}" --state open --json number --jq length) + count=$(gh pr list --repo tambo-labs/homebrew-tap --head "bump/charming-${VERSION}" --state open --json number --jq length) if [ "$count" -eq 0 ]; then gh pr create --repo tambo-labs/homebrew-tap \ --base main \ - --head "bump/charming-${version}" \ - --title "charming ${version}" \ - --body "Bumps the charming formula to ${version}, matching the npm release." + --head "bump/charming-${VERSION}" \ + --title "charming ${VERSION}" \ + --body "Bumps the charming formula to ${VERSION}, matching the npm release." fi - env: - GH_TOKEN: ${{ secrets.HOMEBREW_TAP_TOKEN }} diff --git a/.github/workflows/clawhub-publish.yml b/.github/workflows/clawhub-publish.yml index 146e303..f4469c7 100644 --- a/.github/workflows/clawhub-publish.yml +++ b/.github/workflows/clawhub-publish.yml @@ -1,10 +1,15 @@ name: ClawHub publish -# Keeps clawhub.ai's charming skill listing pointed at the version in -# skills/charming/SKILL.md. `clawhub sync` fingerprints skill folders -# (deriving the slug from the folder name) and only publishes new or -# changed ones, so it's safe to run on every release even if the skill -# itself didn't change. +# Keeps clawhub.ai's charming skill listing in sync with skills/charming/. +# `clawhub sync` fingerprints the skill folder's actual content and only +# publishes when it changed, bumping the registry's own patch version — +# there's no version field in SKILL.md itself to point at. That makes it +# safe to call on every release even if the skill didn't change. +# +# Called directly by release.yml as a job (workflow_call) rather than +# relying on `release: published` — GitHub doesn't let events from the +# default GITHUB_TOKEN (which creates that release) trigger other +# workflows, so that path silently never ran. # # This repo requires every `uses:` action pinned to a full commit SHA # (org policy: sha_pinning_required). ClawHub's own reusable workflow @@ -15,7 +20,10 @@ name: ClawHub publish # # Skills have no OIDC/trusted-publisher option yet, so this uses # CLAWHUB_TOKEN, a token from `clawhub login` + `clawhub token`, scoped to -# the charming publisher (clawhub.ai/settings?view=organizations). +# the charming publisher (clawhub.ai/settings?view=organizations). The PR +# dry-run below has no token, so it resolves the registry state +# unauthenticated — it verifies the sync command runs cleanly against the +# real skill content, not that the real publish's auth/ownership works. on: pull_request: @@ -24,6 +32,10 @@ on: - skills/** release: types: [published] + workflow_call: + secrets: + CLAWHUB_TOKEN: + required: true workflow_dispatch: permissions: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5116ec7..da8b2e2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,6 +9,13 @@ name: Release # # A tag alone doesn't publish: `bun run check` (the same gate ci.yml # runs) must pass, and the tag must match package.json's version. +# +# bump-homebrew and clawhub-publish are called directly as jobs +# (workflow_call), not left to trigger off `release: published`. +# GitHub does not let events created by the default GITHUB_TOKEN +# (like the `gh release create` below) trigger other workflows — +# that path silently never fired, and Homebrew/ClawHub stayed on the +# old version forever with everything upstream showing green. on: push: @@ -23,9 +30,15 @@ jobs: permissions: contents: write # create the GitHub Release id-token: write # npm trusted publishing (OIDC) + outputs: + version: ${{ steps.version.outputs.version }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Resolve version + id: version + run: echo "version=${GITHUB_REF_NAME#v}" >> "$GITHUB_OUTPUT" + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: bun-version: 1.3.14 @@ -65,3 +78,15 @@ jobs: - run: gh release create "$GITHUB_REF_NAME" --generate-notes --verify-tag env: GH_TOKEN: ${{ github.token }} + + bump-homebrew: + needs: release + uses: ./.github/workflows/bump-homebrew.yml + with: + version: ${{ needs.release.outputs.version }} + secrets: inherit + + clawhub-publish: + needs: release + uses: ./.github/workflows/clawhub-publish.yml + secrets: inherit diff --git a/README.md b/README.md index dc40559..937454a 100644 --- a/README.md +++ b/README.md @@ -4,15 +4,13 @@ Build and manage personal apps hosted by [Charming](https://charm.ing) from a te ## Install -Not published yet. Once it is, the plan is: - ```bash npm install -g usecharming # or brew install tambo-labs/tap/charming ``` -Until then, install from source: +Or install from source: ```bash git clone https://github.com/tambo-labs/charming-cli.git diff --git a/package.json b/package.json index 9a3ac3f..9a7051b 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,6 @@ "bin", "dist", "examples", - "templates", "provenance.json", "UPSTREAM.json", "skills" diff --git a/skills/charming/SKILL.md b/skills/charming/SKILL.md index 619b36b..6da1d60 100644 --- a/skills/charming/SKILL.md +++ b/skills/charming/SKILL.md @@ -32,7 +32,7 @@ Do not free-hand the skeleton. Copy `templates/crud`, then change the data model ## Start 1. `charming auth status`. If `"authenticated": false`, run `charming auth login --no-open` and give the user the approval URL and code. -2. Copy the CRUD template into a working directory: `cp -r "$(npm root -g)/usecharming/templates/crud" ./my-app` Change `manifest.id`, `manifest.meta.name`, the storage key, and the `window.charming.api("")` argument in `ui.js` to match. +2. Copy the CRUD template into a working directory: `cp -r "$(npm root -g)/usecharming/skills/charming/templates/crud" ./my-app` Change `manifest.id`, `manifest.meta.name`, the storage key, and the `window.charming.api("")` argument in `ui.js` to match. 3. `charming apps create ./my-app --description "" --dry-run` 4. `charming apps create ./my-app --description "" --yes` 5. Smoke-test the backend before you claim it works: `charming apps call list --input '{}'` diff --git a/templates/crud/module.js b/skills/charming/templates/crud/module.js similarity index 100% rename from templates/crud/module.js rename to skills/charming/templates/crud/module.js diff --git a/templates/crud/styles.css b/skills/charming/templates/crud/styles.css similarity index 100% rename from templates/crud/styles.css rename to skills/charming/templates/crud/styles.css diff --git a/skills/charming/templates/crud/ui.js b/skills/charming/templates/crud/ui.js new file mode 100644 index 0000000..3ad04f2 --- /dev/null +++ b/skills/charming/templates/crud/ui.js @@ -0,0 +1,151 @@ +// CRUD template UI. Rename the api() argument to match manifest.id, then +// adapt the markup and fields to the app's real data model. + +const api = window.charming.api('my-app'); + +let items = []; +let shellReady = false; + +function escapeHtml(value) { + return String(value).replace( + /[&<>"']/g, + (char) => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' })[char], + ); +} + +async function refresh() { + items = await api.list({}); + render(); +} + +// #toast and #app-content are built once. render() only ever touches +// #app-content — a full #app replacement on every refresh (including the +// 4s poll below) would otherwise wipe half-typed input and cut the undo +// toast's own display window short every time it fires. +function ensureShell() { + if (shellReady) return; + const app = document.querySelector('#app'); + app.innerHTML = ` +
+
+ +
+ `; + shellReady = true; +} + +function render() { + ensureShell(); + const content = document.querySelector('#app-content'); + + // A background refresh (onStateChange or the 4s poll) shouldn't wipe + // whatever the user is mid-typing into #add-input. + const input = content.querySelector('#add-input'); + const preserved = + document.activeElement === input + ? { value: input.value, selectionStart: input.selectionStart } + : null; + + content.innerHTML = ` +
+

My App

+

${items.length} item${items.length === 1 ? '' : 's'}

+
+
+
+ + +
+
    + ${items + .map( + (item) => ` +
  • + + + ${escapeHtml(item.text)} + + +
    • + `, + ) + .join('')} +
+
+ `; + bind(); + + if (preserved) { + const restored = content.querySelector('#add-input'); + restored.value = preserved.value; + restored.focus(); + restored.setSelectionRange(preserved.selectionStart, preserved.selectionStart); + } +} + +function bind() { + const content = document.querySelector('#app-content'); + + content.querySelector('#add-form').addEventListener('submit', async (event) => { + event.preventDefault(); + const input = content.querySelector('#add-input'); + const text = input.value.trim(); + if (!text) return; + input.value = ''; + await api.add({ text }); + await refresh(); + }); + + content.querySelectorAll('li').forEach((row) => { + const id = row.dataset.id; + + row.querySelector('.toggle').addEventListener('change', async () => { + await api.toggle({ id }); + await refresh(); + }); + + row.querySelector('.remove').addEventListener('click', async () => { + const removed = items.find((item) => item.id === id); + items = items.filter((item) => item.id !== id); + render(); + await api.remove({ id }); + showUndoToast(removed); + }); + }); +} + +function showUndoToast(removedItem) { + const toast = document.querySelector('#toast'); + toast.innerHTML = ` + Removed + + `; + toast.classList.remove('hidden'); + + toast.querySelector('#undo').addEventListener('click', async () => { + toast.classList.add('hidden'); + await api.restore({ item: removedItem }); + await refresh(); + }); + + setTimeout(() => toast.classList.add('hidden'), 6000); +} + +refresh(); + +// Live updates when another surface (or agent) changes the data. +window.charming.onStateChange?.(() => refresh()); +setInterval(refresh, 4000); diff --git a/src/argv.test.ts b/src/argv.test.ts index 675ff02..93646a8 100644 --- a/src/argv.test.ts +++ b/src/argv.test.ts @@ -22,4 +22,13 @@ describe('canonicalizeGlobalFlags', () => { '--token=chrm_user_test', ]); }); + + test('moves a prefix --timeout after the discovered command, like --base-url and --token', () => { + expect(canonicalizeGlobalFlags(['--timeout', '5000', 'apps', 'list'])).toEqual([ + 'apps', + 'list', + '--timeout', + '5000', + ]); + }); }); diff --git a/src/argv.ts b/src/argv.ts index 4c5f6f8..7bc5e88 100644 --- a/src/argv.ts +++ b/src/argv.ts @@ -1,4 +1,4 @@ -const globalFlags = new Set(['--base-url', '--token']); +const globalFlags = new Set(['--base-url', '--token', '--timeout']); export function canonicalizeGlobalFlags(argv: string[]): string[] { const command: string[] = []; diff --git a/src/commands.ts b/src/commands.ts index c3545b3..898452e 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -1,6 +1,8 @@ import { spawn } from 'node:child_process'; +import { readFileSync } from 'node:fs'; import { readFile } from 'node:fs/promises'; -import { basename, resolve } from 'node:path'; +import { basename, dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { type OptionValue, stringOption, stringOptions } from './args.js'; import { @@ -735,4 +737,9 @@ function isUserToken(token: string | undefined): boolean { return token?.startsWith('chrm_user_') === true || token?.startsWith('bld_user_') === true; } -export const CLI_VERSION = '0.1.0'; +// Read once from package.json rather than a hardcoded literal, which drifted +// stale (0.1.0) against the real published version within one release — +// this mislabeled agent-context output and every pairing token minted since. +export const CLI_VERSION: string = JSON.parse( + readFileSync(resolve(dirname(dirname(fileURLToPath(import.meta.url))), 'package.json'), 'utf8'), +).version; diff --git a/templates/crud/ui.js b/templates/crud/ui.js deleted file mode 100644 index f6377c2..0000000 --- a/templates/crud/ui.js +++ /dev/null @@ -1,113 +0,0 @@ -// CRUD template UI. Rename the api() argument to match manifest.id, then -// adapt the markup and fields to the app's real data model. - -const api = window.charming.api('my-app'); - -let items = []; - -async function refresh() { - items = await api.list({}); - render(); -} - -function render() { - const app = document.querySelector('#app'); - app.innerHTML = ` -
-
-

My App

-

${items.length} item${items.length === 1 ? '' : 's'}

-
-
-
- - -
-
    - ${items - .map( - (item) => ` -
  • - - - ${item.text} - - -
    • - `, - ) - .join('')} -
-
- -
- `; - bind(); -} - -function bind() { - const app = document.querySelector('#app'); - - app.querySelector('#add-form').addEventListener('submit', async (event) => { - event.preventDefault(); - const input = app.querySelector('#add-input'); - const text = input.value.trim(); - if (!text) return; - input.value = ''; - await api.add({ text }); - await refresh(); - }); - - app.querySelectorAll('li').forEach((row) => { - const id = row.dataset.id; - - row.querySelector('.toggle').addEventListener('change', async () => { - await api.toggle({ id }); - await refresh(); - }); - - row.querySelector('.remove').addEventListener('click', async () => { - const removed = items.find((item) => item.id === id); - items = items.filter((item) => item.id !== id); - render(); - await api.remove({ id }); - showUndoToast(removed); - }); - }); -} - -function showUndoToast(removedItem) { - const toast = document.querySelector('#toast'); - toast.innerHTML = ` - Removed - - `; - toast.classList.remove('hidden'); - - toast.querySelector('#undo').addEventListener('click', async () => { - toast.classList.add('hidden'); - await api.restore({ item: removedItem }); - await refresh(); - }); - - setTimeout(() => toast.classList.add('hidden'), 6000); -} - -refresh(); - -// Live updates when another surface (or agent) changes the data. -window.charming.onStateChange?.(() => refresh()); -setInterval(refresh, 4000);