feat(sdk): fold bitbadges-builder-mcp into the SDK#144
Merged
Conversation
Migrates the standalone bitbadges-builder-mcp package into bitbadgesjs-sdk/src/mcp/. Removes the file:-dep circular between SDK and MCP, collapses the two packages into one install, and exposes the full tool+resource surface as subpath exports (./mcp, ./mcp/tools, ./mcp/resources, ./mcp/skills, ./mcp/session, ./mcp/registry). The stdio MCP server keeps working through a new bin entry (bitbadges-builder-mcp -> dist/cjs/mcp/index.js) so existing Claude Desktop configs continue to resolve by installing bitbadgesjs-sdk. Adds `bitbadges-cli mcp` subcommand group (list / call / session / resources) that dispatches directly against the in-process registry — no subprocess, no protocol. File-backed sessions live under ~/.bitbadges/sessions/<id>.json so incremental builders compose across invocations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers isValidListId, checkNumbersAreStrings, checkUintRangeFormat, findAndValidateUintRanges, validateOrderCalculationMethod, validateApprovals, validateTokenMetadata, validatePermissions, validateApprovalCriteria, validateMsgConstructorFields, validateTransaction, validateSubscriptionApproval. ~90 cases. Landed alongside the MCP fold-in since both touch the SDK working tree; split into its own commit for reviewability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merged
3 tasks
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Review feedback on cli/commands/mcp.ts — `fs`/`os`/`path` and the SESSIONS_DIR constant were domain logic leaking into a CLI command file. Any other consumer (indexer, a future stdio bridge, tests) that wanted file-backed sessions would have duplicated the same 40 lines. Moves the persistence layer into src/mcp/session/fileStore.ts beside the in-memory store, exported through the existing ./mcp/session subpath: - DEFAULT_SESSIONS_DIR (overridable per call) - sessionFilePath(id, dir?) - loadSessionFromDisk(id, dir?) - saveSessionToDisk(id, dir?) - listSessionFilesOnDisk(dir?) - readSessionFileRaw(id, dir?) - resetSessionFile(id, dir?) cli/commands/mcp.ts now imports these helpers. The file keeps `fs` only for the genuinely CLI-local concern of reading --args-file. No behavior change — `~/.bitbadges/sessions/<id>.json` stays the default location, same format, same fall-through semantics on missing files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CodeQL flagged four issues on the regex-based HTML sanitizer used by fetch_docs: 1. `<script[^>]*>...</script>` doesn't match `</script >` (trailing whitespace before the closing `>`) — script bypass. 2. A single `.replace()` pass over `<script...>` leaves nested forms like `<scr<script>ipt>` as `<script>` after removing the inner match — partial sanitization. 3. Same two issues for `<style>`. 4. Sequential `.replace()` calls for HTML entities double-unescape — `&lt;` goes to `<` to `<` instead of staying as `<`. Replaces the ad-hoc regexes with: - `stripTagBlock(html, tag)` — tempered-greedy regex with permissive `</tag\s*>` closing match, run in a loop until the output is stable so overlapping bypasses are fully eliminated. - `decodeHtmlEntities(input)` — single-pass lookup table over `&(nbsp|amp|lt|gt|quot|apos);` so no decoded character can be reinterpreted as part of another entity. Verified against CodeQL's four cases plus the nested-prefix and `&lt;` double-unescape cases. Behavior for normal input (the only path this tool actually hits — bitbadges.io docs pages) is unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ecks Adds a single SDK entry point so CLI, MCP, indexer, and frontend all run the same deterministic review set. Ports ~40 frontend UX checks into src/core/review-ux/ and exposes reviewCollection() + Finding type with stable machine codes plus English fallback messages (localization is handled by callers via the finding code). CLI: sdk review now supports --json / --human / --strict with exit 0/1/2. MCP: new review_collection tool; audit_collection / verify_standards marked deprecated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds optional localeKey (+ per-slot overrides) to Finding so frontend adapters can prefer the richly-edited review_*_title/detail/fix strings in en/common.json over the SDK's terser messageEn/recommendationEn. Every ported UX check now points at its legacy key base. Also ports the last orphaned checks that had locale entries but no SDK coverage: - forceful_invariant_not_set (pre-fix critical for invariant-only case) - credit_token_transfers_allowed (credit-token skill) - addresslist_transfers_allowed (address-list skill) And fixes a small firing-condition bug in forceful_transfers_allowed so the invariant-only branch actually triggers, switching localeKeyDetail conditionally across the 3 state branches. +4 tests, 1010/1010 pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
7 tasks
…nd-aids Routes every indexer auto-fix repair to its correct home: - Wrong-shape hallucinations (PathMetadata.image at wrong level, array collectionMetadata, partial approvalAmounts/maxNumTransfers, empty amountTrackerId with non-zero limits) now throw clearly in validate.ts instead of being silently coerced - amountTrackerId is auto-filled in sessionState.addApproval from the approvalId when a non-zero limit exists, preventing silent tracker cross-contamination across approvals - Opinionated semantic guesses (predetermined_order unset/conflict) surface as new review items review.ux.predetermined_order_unset and review.ux.predetermined_order_conflict instead of silent defaults Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Removes the ensureAmountTrackerIds helper from sessionState.addApproval. The validator in validate.ts already throws clearly when a non-zero limit is set without an amountTrackerId — silent auto-fill even through the MCP tool path was still a band-aid over a producer bug. Pure throw-only posture: the AI agent sees the real error and learns to set amountTrackerId explicitly instead of relying on an implicit default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
"MCP" is the wire protocol of the stdio transport — one presentation layer among several. Calling the CLI subcommand, the source directory, and the library subpath exports "mcp" overstates the protocol's relevance and confuses consumers that reach the handlers in-process (CLI, indexer library use, chain binary delegation). The consistent brand elsewhere is "BitBadges Builder", so drop the `-mcp` suffix. Scope: - `src/mcp/` → `src/builder/` (78 files moved via git mv) - `src/cli/commands/mcp.ts` → `src/cli/commands/builder.ts` - Identifier: `mcpCommand` → `builderCommand` - Commander root: `bitbadges-cli mcp ...` → `bitbadges-cli builder ...` - package.json subpath exports and typesVersions: `./mcp/*` → `./builder/*` - package.json bin: `bitbadges-builder-mcp` → `bitbadges-builder` - `sdk skills` alias slug: `mcp-builder-skills` → `builder-skills` (coordinated with the bitbadges-docs rename on `feat/mcp-in-sdk`) - fetchDocs topic map: 'builder'/'builder tools' added, URLs pointed at /builder-tools; old 'mcp'/'mcp tools' keys retained for back-compat lookups until docs deploys - LLM-facing resource strings (recipes.ts, frontendDocs.ts, workflows.ts, registry.ts, tools/builders/*, core/validate.ts) rewritten "MCP tool" → "builder tool", "MCP server" → "builder", etc. Kept intentionally (protocol-true references): - `@modelcontextprotocol/sdk` imports - File headers in `src/builder/server.ts` and `src/builder/index.ts` clarified as "Model Context Protocol (MCP) stdio transport/server" — the directory is named for purpose, the file explains the transport - `src/builder/tools/registry.ts` uses "MCP" only where describing the content block wire shape returned over the stdio transport Verified: - `npm run build` clean (dual ESM+CJS, tsc-alias, madge no circulars) - `bitbadges-cli builder list --names | wc -l` → 51 - `bitbadges-cli builder resources list --uris | wc -l` → 11 - `bitbadges-cli builder call get_current_timestamp` round-trips - `node dist/cjs/builder/index.js` stdio JSON-RPC tools/list OK - `bitbadges-cli mcp ...` now fails with "unknown command 'mcp'" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Follows the prior mcp → builder rename. "build" and "builder" were two
separate top-level command groups that both meant "build a transaction"
to users — just from different audiences (deterministic templates vs.
AI tool registry). Fold everything into one builder hierarchy so the
full build + review workflow lives in one place.
Layout:
bitbadges-cli builder templates <name> [flags] # 18 deterministic templates
bitbadges-cli builder tools list # tool registry discovery
bitbadges-cli builder tools call <tool> --args # fine-grained registry calls
bitbadges-cli builder review <input> # reviewCollection(), grouped findings
bitbadges-cli builder explain <input> # interpretTransaction / interpretCollection
bitbadges-cli builder validate <input> # pure validateTransaction()
bitbadges-cli builder resources list|read # static MCP resources
bitbadges-cli builder session list|show|reset # persisted session scratch pad
Changes:
- src/cli/commands/build.ts renamed to templates.ts. Top-level
`build` command renamed to `templates`, export is now
`templatesCommand`. 18 template commands unchanged — only the
wrapping container moved.
- builder.ts attaches `templatesCommand` as a subcommand, wraps the
prior `list` / `call` commands under a new `tools` subcommand.
- builder.ts adds top-level `review`, `explain`, `validate` commands.
`review` is a faithful port of `sdk review` (supports numeric
collection IDs with API fetch). `explain` auto-detects tx vs
collection shape and routes to interpretTransaction or
interpretCollection. `validate` wraps validateTransaction for a
fast low-level structure check.
- New `ensureTxWrapper` helper normalizes loose inputs — bare
`{typeUrl, value}` Msgs get wrapped in `{messages: [msg]}` so both
review and validate accept piped template output cleanly.
- cli/index.ts no longer registers the old buildCommand top-level;
`builder templates` is the canonical path.
Auto-review on template output:
- `builder templates vault --backing-coin USDC` now pipes stdout JSON
+ stderr review findings by default. The stdout stream stays pure
JSON for downstream sign/broadcast pipelines.
- `--dry-run` flag removed (subsumed by the default review).
- New `--json-only` flag suppresses the automatic review for
pipelines that want zero stderr noise.
- Auto-review is called with `selectedSkills: []` so the reviewer's
skill fan-out (which defaults to "run every protocol check") is
disabled — the user just built with a specific template, so only
cross-cutting audit/ux checks should fire. A full skill-union
review stays one step away via `builder review <file>`.
Verified end-to-end:
- `npm run build` clean (589 files, tsc dual build, tsc-alias,
madge circular check pass)
- `builder templates vault --backing-coin USDC --creator bb1test`
→ JSON on stdout, 0 critical / 5 warning / 5 info on stderr,
verdict `warn`
- `… --json-only` → pure JSON on stdout, empty stderr
- `builder templates vault --json-only | builder review -` → 6
critical / 8 warning / 5 info (full skill fan-out as expected
for an explicit review invocation)
- `builder templates vault --json-only | builder explain -` →
prose transaction summary
- `builder templates vault --json-only | builder validate -` →
5 actionable static validation issues
- `builder tools list --names | wc -l` → 51
- `builder tools call get_current_timestamp` round-trips
- `build vault` (old top-level) → commander suggests `builder`
- `builder list` (old flat command) → `unknown command 'list'`
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previous review output was one wall of unformatted text with no visual
separation between the stderr review section and the stdout JSON dump.
When both streams hit the terminal, the user couldn't tell where the
review ended and the transaction JSON began.
Adds src/cli/utils/terminal.ts with:
- `makeColor(stream)` — narrow ANSI helper with TTY detection
(honours NO_COLOR env var, respects `TERM=dumb`, inspects the
specific stream's isTTY since stderr/stdout can differ per
redirection)
- `rule(ch, width, label?)` — horizontal rules with optional label
- `renderReview(result, { stream, title })` — single renderer used
by both `builder templates` auto-review and `builder review`, so
they stay in lockstep. Wraps long text to the stream's column
width, groups findings by severity, bold-tinted sigils
(■ critical / ▲ warning / ● info), gray code slugs, summary
bar with each count tinted only when non-zero
- `renderJsonBoundary(stream)` — the "─── Transaction JSON (stdout)
───" rule that fires only when stdout AND stderr are both TTYs
(interactive use); suppressed when piped so it never lands in a
redirected JSON file
Updates:
- templates.ts `emit()` now calls renderReview for auto-review and
renderJsonBoundary when appropriate
- builder review command uses renderReview (shared path)
- builder validate gets the same treatment inline — rule headers,
sigils, tinted summary, TTY-aware
Output file paths strip ANSI defensively before writing, so
`--output-file` always produces plain-text files even if rendered
from a TTY session.
Verified:
- `script -q -c "builder templates vault …" /dev/null` → ANSI
rendering with boundary rule between review + JSON
- `builder templates vault … 2>/tmp/err >/tmp/out` → plain-text
review in /tmp/err, no boundary rule, clean JSON in /tmp/out
- `… --json-only 2>/tmp/err >/tmp/out` → /tmp/err empty (unchanged)
- `builder validate -` on piped input → plain-text rule sections
with bold sigils and summary
- `npm run build` clean (589 files, madge pass)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two changes to the templates auto-review path, both driven by the "build → review" UX pass: 1. Order flip: JSON now emits BEFORE the review. The user's final eyeline after a template run lands on the review verdict instead of a wall of JSON, and in interactive terminals scrolling up shows the build output above the critique. Works cleanly with pipes too — stdout JSON flushes first, stderr review after. 2. Auto-validate: before running `reviewCollection`, auto-review now also runs `validateTransaction` from core/validate.ts. These are both static, local, pre-broadcast checks, but they were previously split across `builder validate` and the review path. Folding them into one emit path means templates can't silently produce JSON that the chain would reject. In practice this surfaces genuine template bugs. Running the `vault` template today fires 4 structural errors (all in buildVault's output — PathMetadata.image doesn't exist, canUpdateAutoApproveAllIncomingTransfers missing, etc.) that the design-level reviewCollection never caught. Fixing those is out of scope for this commit — the change here just makes them visible on every run. Implementation notes: - emit() structure: JSON output → optional --explain → validate → review. Both checks are skipped by --json-only. - New `renderValidate()` helper local to templates.ts. Kept separate from the richer `renderReview()` because the shapes differ (ValidationIssue vs Finding). Both share the same color palette and rule style via `makeColor`/`rule` in utils/terminal.ts. - Removed the pre-JSON boundary rule (`renderJsonBoundary`): with the order flipped, the review's own "━━━ Auto-Review ━━━" header is the natural visual separator. Decision recorded in the commit so future readers know why templates do both validate+review and not simulate: simulation is a network + auth concern with 100-1000x higher latency and failure modes orthogonal to JSON correctness. Draw the line at static checks; simulation is opt-in via a separate path. Verified: - `builder templates vault --backing-coin USDC` → JSON on stdout, Auto-Validate (4 errors/1 warning) + Auto-Review (5 warn/5 info) on stderr in that order - `--json-only` → pure JSON, empty stderr (unchanged) - `npm run build` clean Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The auto-validate layer added in 0f976b8 surfaced 41 structural errors spread across 5 templates (vault, smart-account, subscription, crowdfund, auction, prediction-market). All five failed for the same set of shared-helper bugs. This commit fixes them at the source. core/builders/shared.ts: - `alwaysLockedPermission()` previously returned just `{permanentlyPermittedTimes: [], permanentlyForbiddenTimes: FOREVER}` and was used for every permission type. The validator correctly rejects that for permission shapes that require extra scoping fields. Adds two new scoped variants: - `alwaysLockedTokenIdsPermission()` — for canUpdateValidTokenIds / canUpdateTokenMetadata, returns the base permission plus `tokenIds: FOREVER` so the validator's TokenIdsActionPermission check passes. - `alwaysLockedCollectionApprovalPermission()` — for canUpdateCollectionApprovals, returns the base permission plus fromListId/toListId/initiatedByListId = 'All', transferTimes, tokenIds, approvalId, amountTrackerId, challengeTrackerId. `emptyPermissions()` and `frozenPermissions()` now call the correct variant for each field. - `defaultBalances()` was emitting userPermissions without `canUpdateAutoApproveAllIncomingTransfers`. The SDK constructor requires it as `[]`. Added. - `buildAliasPath()` was setting `metadata: {uri, customData, image}` on both the path and its denomUnits. PathMetadata only has `{uri, customData}`; the `image` field doesn't exist on-chain. Drops `image` from the proto emission and now sets placeholder URIs (`ipfs://METADATA_ALIAS_<denom>` and `ipfs://METADATA_ALIAS_<denom>_UNIT`) so the path has a valid shape and downstream metadata tooling has something to substitute. The `image` parameter is kept in the function signature for back-compat; a follow-up commit will wire it through the metadata placeholder surface. core/builders/smart-account.ts: - Was calling `alwaysLockedPermission()` for canUpdateCollectionApprovals directly. Switched to `alwaysLockedCollectionApprovalPermission()`. core/builders/auction.ts: - `initiatedByListId: ''` on the mint-to-winner approval — empty string is an invalid list ID. Changed to `'All'` with a comment explaining the tradeoff (users who want a specific bidder list can patch the approval). core/builders/crowdfund.ts: - Three approvals were falling back to `params.crowdfunder || ''` for toListId / initiatedByListId. Empty string fails validation. Changed fallback to `'All'` so the template produces valid JSON without the flag; the reviewer still surfaces the missing crowdfunder as a warning. Verified with the auto-validate sweep after each fix: vault 0 errors (was 4) smart-account 0 errors (was 7) subscription 0 errors (was 3) crowdfund 0 errors (was 17) auction 0 errors (was 10) prediction-market 0 errors (was 10) [12 other templates] 0 errors Design-level warnings from reviewCollection are unchanged — those remain real advisories about permission neutrality, missing noForcefulPostMintTransfers, etc. This commit only addresses structural validation, not the review findings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ad section
User caught that the earlier metadata.image workarounds (both in SDK
builders and indexer prompts) were masking a root-cause issue: we've
been teaching LLMs to emit `metadata.image` on PathMetadata when the
proto has EXACTLY `{ uri, customData }`. Every downstream consumer
was then hot-patching the invalid field back out. This commit fixes
the teaching at source and removes the hot-patches.
Root-cause fixes:
- core/verify-standards.ts — the verifier was telling callers
"Set metadata.image to <BITBADGES_DEFAULT_IMAGE>" as a fix when the
image was missing. That's invalid proto. Rewritten to check
metadata.uri and instruct callers to set a placeholder URI and put
the image inside the off-chain JSON at that URI. Added a new
explicit error case when `metadata.image` appears at all, with the
clear message "PathMetadata only has { uri, customData }". New
helper `isFakeOrMissingUri` that knows `ipfs://METADATA_*`
placeholders are valid.
- core/review-ux/metadata.ts — the `review.ux.alias_paths_missing_images`
check was reading `pathMeta?.image`, `unitMeta?.metadata?.image`,
`_metadataForUpload?.image` — i.e. every wrong location we'd ever
stuffed an image into. Now it reads ONLY the metadataPlaceholders
sidecar keyed by path/unit metadata.uri, which is where images are
supposed to live. Recommendation text rewritten.
- builder/resources/skillInstructions.ts — removed 6 sites where the
skill prompt taught LLMs to put `image` on alias path / denomUnit /
wrapper path metadata. Replaced with explicit guidance that
PathMetadata is `{ uri, customData }` only and that images belong
in the off-chain JSON registered via metadataPlaceholders. Example
JSON blobs in the skill docs now show the placeholder URI pattern.
- builder/tools/session/addAliasPath.ts +
builder/tools/session/addCosmosWrapperPath.ts — these had zod
schemas + JSON-schema inputSchemas that declared
`metadata.image` REQUIRED, AND runtime handlers that propagated
`pathImage` onto `unit.metadata.image`. Both rewritten: the
schemas drop `image`, the handlers strip any incoming `image`
field and fill a placeholder URI (`ipfs://METADATA_ALIAS_<denom>` /
`ipfs://METADATA_WRAPPER_<denom>`) when one isn't provided. Tool
descriptions updated to point callers at the off-chain JSON flow.
Indexer side (committed separately on bitbadges-indexer):
- routes/ai-builder/core/promptBuilder.ts — the export-mode prompt
Rules block was ambiguous about whether image/name/description go
on the on-chain metadata proto or in the metadataPlaceholders
sidecar. Rewritten to be explicit: on-chain metadata is ONLY
`{ uri, customData }`, everything else lives in the sidecar.
New templates.ts feature — "Metadata To Upload" section:
Templates walk their emitted tx body and surface every placeholder
URI (`ipfs://METADATA_*`) in a new stderr section alongside the
auto-validate / auto-review output. For each placeholder:
- the URI
- what it's for (Collection / Token / Approval / Alias path /
Denom unit / Wrapper path)
- which field on the proto references it
- which fields the off-chain JSON needs (name/description/image)
- a PROVIDED / NEEDED badge keyed off `data._meta.metadataPlaceholders`
Gives CLI users a concrete TODO list: here's what you still have to
upload before broadcast. Honoring the "build + review = static"
philosophy, the section is informational only — no network calls.
Verified:
- `npm run build` clean (589 files, madge pass)
- 12/12 templates still produce 0 structural errors after the
skillInstructions + session-tool rewrites
- `builder templates vault --backing-coin USDC` shows two
NEEDED placeholders (`ipfs://METADATA_ALIAS_uvault` and
`ipfs://METADATA_ALIAS_uvault_UNIT`) with the expected field list
Known follow-up: the `metadataPlaceholders()` / `singleTokenMetadata()`
helpers in shared.ts still use `DEFAULT_METADATA_URI` and stuff
name/description/image into customData as JSON. That's the old
shape. A follow-up commit will rewire them to emit
`ipfs://METADATA_COLLECTION` / `ipfs://METADATA_TOKEN_<range>`
placeholders so collection/token metadata also surfaces in the
Metadata To Upload section.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
custom-2fa permission shape (Gap B) Two compatibility gaps surfaced by the indexer/frontend audit: Gap A (moderate) — Silent metadata loss in alias/wrapper session tools ───────────────────────────────────────────────────────────────────── The earlier metadata.image purge (14bc772) made addAliasPath / addCosmosWrapperPath strip any inbound `image` field from PathMetadata. Correct on the proto side, but the strip went into the void: there was no way for an LLM (or any other caller) to attach an image to an alias path's placeholder URI, so images were lost silently. This commit adds first-class top-level params to both tools: addAliasPath / addCosmosWrapperPath now accept: pathName, pathDescription, pathImage denomUnitName, denomUnitDescription, denomUnitImage The handlers route those (plus any legacy fields the caller stuffed nested-on-metadata for backwards compat) into the session's metadataPlaceholders sidecar keyed by the path's auto-generated ipfs://METADATA_ALIAS_<denom> / ipfs://METADATA_WRAPPER_<denom> placeholder URI. Same routing pattern as setCollectionMetadata — content into the sidecar, only { uri, customData } onto the proto. This restores the AI builder flow's ability to attach images to alias paths without touching the on-chain proto. The Metadata To Upload section in templates auto-review will now render those entries as PROVIDED instead of NEEDED when the caller passes the new params. Gap B (low) — custom-2fa template still using bare permission helper ───────────────────────────────────────────────────────────────────── `core/builders/custom-2fa.ts` was using the bare alwaysLockedPermission() for canUpdateValidTokenIds, which the validator now rejects (needs tokenIds field per the cf8c379 fix). Switched to alwaysLockedTokenIdsPermission(). custom-2fa was the only template still hand-rolling permissions outside of emptyPermissions/frozenPermissions. Verified: - npm run build clean (589 files, madge pass) - All 12 templates still produce 0 structural errors - frontend tsc --noEmit clean against the linked SDK Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tection
Four related false-positive fixes driven by the vault form sidebar:
1. Unbacking approval detection: previously gated on
fromListId.startsWith('!') || fromListId === 'All'. The vault form
emits withdrawal approvals with fromListId: 'AllWithoutMint', which
failed both checks. Switch to structural matching using the canonical
backing address from invariants.cosmosCoinBackedPath.address —
fromListId/toListId that contains or equals the backing address is
the unambiguous signal. Also recognize vault-deposit / vault-withdraw
approval ID prefixes as explicit hints.
2. Soulbound detection: previously excluded backed-minting approvals
from the transfer-path count, so any smart-token collection that
used backing/unbacking for its transfer flow got flagged as
soulbound. Now backed-minting approvals count as transfer paths —
users CAN move tokens out via the backing address, so they are not
soulbound.
3. Metadata URI placeholder violations: verify-standards flagged
aliasPathsToAdd[*].metadata.uri as "missing or fake placeholder"
when the vault form was in pre-apply state. The frontend stores
alias path metadata in the WithDetails shape —
{ uri: '', customData: '', metadata: { name, image, description } } —
with the uri left blank until the metadata auto-apply flow uploads
the inline content to IPFS on submit. Added isPreApplyMetadata()
detection that suppresses the uri check when a nested metadata.metadata
object with inline name/image/description exists. Same fix for the
denomUnits[*].metadata.uri check.
4. review.ux.alias_paths_missing_images: the existing check only looked
at metadataPlaceholders sidecar entries. Now also looks at the
WithDetails nested inline image (pathMeta.metadata.image /
unitMeta.metadata.image) so frontend pre-apply state with an inline
image is recognized as "has image".
Tests: 1144/1144 core green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…mint noise Swap the recursive stringify/bigintify hack for the proper class-based pipeline the SDK already has: new MsgUniversalUpdateCollection(value).convert(BigIntify) The class knows which fields are numeric via each nested class's `getNumberFieldNames()` and only converts those — addresses, URIs, denoms, IDs, approvalIds stay as strings. No more false conversions of any-string-that-looks-numeric. normalizeForReview() moved to its own file (`core/review-normalize.ts`) so `auditCollection`, `verifyStandardsCompliance`, and `reviewCollection` can all call it without a circular dependency. The function is idempotent — running it twice is a no-op, so pipelines that stack (reviewCollection → verifyStandardsCompliance) work naturally. Downstream checks now assume bigint numeric fields. Rewrote ~35 strict string comparisons in audit.ts / verify-standards.ts / review-ux/*.ts to use bigint literals (`=== 1n`, `!== 0n`, `=== MAX_UINT64` where `MAX_UINT64 = 18446744073709551615n`). Removed every `String(x) === '0'` defensive wrapper — no longer needed. Also: - Removed the "mint approval is PUBLIC" audit finding. Approvals are typically gated by claim plugins, merkle challenges, or out-of-band allowlists; flagging All+All was noise on every public-mint template. - Collection metadata URI placeholder check now detects the frontend WithDetails shape (nested metadata.metadata with inline name/image/description) and suppresses the "URI is placeholder" warning when the auto-apply flow will populate the URI on submit. Tests: 1144/1144 core green. Review spec updated to pass bigint ranges since runUxChecks callers bypass reviewCollection's upfront normalization. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Msg class .convert(BigIntify) pipeline is clean for numeric
fields but the nested proto classes (PathMetadata, CollectionMetadata,
CollectionApproval) only keep proto-spec fields. The frontend stores
WithDetails inline content on those objects:
aliasPathsToAdd[i].metadata.metadata.{name,image,description}
collectionMetadata.metadata.{name,image,description}
collectionApprovals[i].details.{name,description,image}
These are dropped when we wrap in MsgUniversalUpdateCollection to
get numeric conversion, so review checks that read them false-fire
with "URI is missing or placeholder" / "alias paths missing display
images" / "approval rules are missing names" on the frontend vault
and subscription forms, which are exactly in that pre-apply state.
normalizeForReview now walks the raw (pre-conversion) input and
reattaches those dropped fields onto the converted result. The
numeric conversion still runs; only the WithDetails sidecar fields
are preserved. Non-mutating to the input; mutates the converted
output in place.
Also: branch the `review.ux.forceful_transfers_allowed` message on
whether the finding is firing because of forceful override approvals
(count > 0) or because the noForcefulPostMintTransfers invariant is
unset (count = 0). Previously both cases produced "Currently 0
non-mint approval(s) have forceful transfer overrides enabled..."
which reads as nonsense when count is 0.
Tests: 1144/1144 core green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ddress list transfers Four related review-finding adjustments: 1. Drop `review.ux.addresslist_transfers_allowed` entirely. Address list tokens can legitimately have transfer approvals — the check was flagging every such collection as a warning with no real signal. 2. Downgrade `review.ux.forceful_transfers_allowed` from critical to warning and rewrite the message. Forceful transfer overrides are legitimate for many standard flows (subscription revoke, auction seller-forced settlement, prediction market resolution, credit tokens, etc.) — firing as critical on every collection that uses them produced unactionable noise. The finding now lists the specific approval IDs so the user can verify each one, and the wording reframes it as "verify these are intentional" rather than "this is dangerous." 3. Move quest/reward-escrow funding check from verify-standards (critical) to review-ux (warning). The user can fund mint escrow at any time post-creation by sending coins to the escrow address, so blocking a build on unfunded escrow is wrong. The new check also applies to ANY mint approval that pays out coins (not just Quest) — Subscription, Crowdfund, Auction, Bounty, etc. share the same flow. It cross-references required denoms from coinTransfers against mintEscrowCoinsToTransfer to report exactly which denoms still need funding. 4. Reword `review.ux.amount_scaling_with_approver_funds` detail. The old copy said "expected for prediction markets and credit tokens but dangerous for bids or offers" which read as judgemental — the new copy asks the user to verify the per-use amount under scaling makes sense for their specific use case. Tests: 1143/1143 core green (dropped one obsolete address-list test). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…noise
Two noisy findings removed after debug investigation:
1. `review.ux.mint_escrow_unfunded`: the check read
`value.mintEscrowCoinsToTransfer` from the collection WIP state to
decide whether the escrow was funded. But the frontend never
populates that field on the WIP — `withCoinsToEscrow` is tracked
separately in TxTimelineContext and only attached to the Msg at the
final broadcast step (CreateTxMsgUniversalUpdateCollection.tsx:106).
That meant the warning fired on every quest / subscription /
crowdfund / auction form regardless of what the user actually set.
Combined with the fact that mint escrow can be topped up at any
time post-creation, this was pure noise — removed.
2. `review.audit.directed transfer channel`: the audit info finding
fired on any approval with specific from/to lists and "All"
initiator. The message ("Allows transfers from X to Y, initiated
by anyone. Verify this directed channel is intentional.") restated
what the approval definition already says without adding signal.
Fired on every smart-token backing/unbacking pair and similar
patterns. Removed.
Tests: 1143/1143 core green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…backing supply
Four related changes to the permission / forceful analysis:
1. Forceful check outcomes cleaned up into four distinct cases:
hasOverrides + !invariantBlocksForceful → warning (verify intent)
hasOverrides + invariantBlocksForceful → critical (on-chain fail)
!hasOverrides + !invariantBlocksForceful → info (future could enable)
!hasOverrides + invariantBlocksForceful → silent (locked)
Previously the warning and mismatch would double-fire because both
conditions matched simultaneously when the invariant was set. Now
they are mutually exclusive and the mismatch message gets a fuller
explanation + lists the specific approval IDs causing the conflict.
2. The "future approvals could enable forceful" info finding now
detects when canUpdateCollectionApprovals is permanently forbidden
with a blanket All/All/All scope and appends a note that the
finding is redundant in that case (no future approvals can be
added anyway).
3. New critical audit finding:
`review.audit.transferability.post_mint_transfer_approvals_can_be_modified`
Mirrors the existing "Mint approvals can be modified - UNLIMITED
SUPPLY RISK" finding but for the transferability side — fires when
canUpdateCollectionApprovals is permitted/neutral without a
blanket lock AND the collection has post-mint transfer approvals.
4. New critical audit finding:
`review.audit.supply.backing_approvals_can_be_modified`
Same supply-risk concern as mutable mint approvals, but for
backing approvals (allowBackedMinting: true). Backing approvals
are the smart-token mint source; fromListId is the backing
address, not "Mint", so the existing mint check missed them.
Now covered explicitly with its own supply-risk finding.
Tests: 1143/1143 core green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The "blanket permanent lock on canUpdateCollectionApprovals" detection previously only matched on from/to/initiatedBy list IDs, missing the other five scope fields. A CollectionApprovalPermission has 8 scope fields total: fromListId === 'All' toListId === 'All' initiatedByListId === 'All' transferTimes covers [1 → MAX_UINT] tokenIds covers [1 → MAX_UINT] approvalId === 'All' amountTrackerId === 'All' challengeTrackerId === 'All' (plus permanentlyForbiddenTimes covers forever) A partial match (e.g. the three list IDs set to All but tokenIds scoped to a single range) does NOT permanently lock transferability — it only freezes a subset. New `isCollectionApprovalBlanketLocked` helper in review-ux/shared.ts walks all 8 fields + the time range; `isTransferabilityPermanentlyLocked` is the convenience wrapper. audit.ts has an equivalent inline check for parity. Message text cleanup: - Drop "(All/All/All forbidden)" from the forceful_transfers_not_locked suffix — the "permanently locked" phrasing is sufficient and the parenthetical was misleading given only 3 of 8 fields were checked. - Drop "Lock canUpdateCollectionApprovals with a blanket 'All/All/All' permanently-forbidden scope" from the post-mint transferability recommendation. Now reads "Permanently lock canUpdateCollectionApprovals". - Drop "including introducing forceful transfer approvals that move tokens without holder consent" from the post-mint transferability detail — it was conflating two separate concerns. Severity upgrade: - `review.ux.forceful_transfers_not_locked` upgraded from `info` to `warning` when case B fires (no overrides now, but invariant could never be added later). This is a real trust assumption worthy of more than an info. Stays at `info` in the special case where transferability is already permanently locked, since no future approval can be added anyway. Tests: 1143/1143 core green. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… false
The case B finding (no current forceful approvals + invariant does not
block them) previously said "The noForcefulPostMintTransfers invariant
is not set" — which is a lie when the form explicitly set the invariant
to `false` (it IS set, just to false). Now distinguishes the two:
explicit false: "This collection has noForcefulPostMintTransfers
set to false, which permits forceful post-mint
transfers."
unset: "The noForcefulPostMintTransfers invariant is not
set, which by default permits forceful post-mint
transfers."
Title rephrased from "Forceful transfers not locked at creation" to
"Forceful transfers permitted post-mint" — the old title implied the
user forgot to lock something, when the unlocked state is often
intentional (vault, subscription, etc.).
Recommendation text also reframed so "this is intentional for your
design" is a first-class option, not an afterthought.
No logic change — the 4-case outcome table stays:
invariant=true + no overrides → silent
invariant=true + has overrides → critical (mismatch)
invariant=false + no overrides → warning (this finding)
invariant=false + has overrides → warning (verify each intentional)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The four forceful-transfer outcomes are now:
hasOverrides + !invariantBlocks → critical (was warning)
hasOverrides + invariantBlocks → critical (unchanged — on-chain fail)
!hasOverrides + !invariantBlocks → critical (was warning)
except when transferability is
permanently locked via a blanket
canUpdateCollectionApprovals
forbidden entry → info
!hasOverrides + invariantBlocks → silent (unchanged — safe state)
Rationale: forceful transfers are an immutable decision at creation
time. The invariant can't be toggled later, and existing approvals
with override flags can only be removed via another update. Holders
depend on the current state being stable, so any non-safe case is a
critical sign-off point.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ctions
BitBadges has two minting approaches — standard mint (fromListId:
"Mint" approvals) and 1:1 IBC backing (allowBackedMinting approvals
at the backing address with cosmosCoinBackedPath set). Many review
findings were phrased assuming the standard-mint flow, which read
oddly for smart-token collections where tokens enter circulation
via IBC deposits.
Key terminology swaps in review item text:
minting → creating new tokens / token creation
mint approvals → token-creation approvals
post-mint → once in circulation / after creation
minted tokens → newly created tokens
Specific findings updated:
audit.ts
- "No post-mint transfer approval found (soulbound)" retitled
"Tokens are non-transferable / soulbound". Detail no longer
says "soulbound after minting" — now "cannot leave their
initial holder once they are in circulation".
- "Missing autoApproveAllIncomingTransfers for mint collection"
retitled "Recipients cannot receive newly created tokens".
- "Mint approval X has NO supply limits" — detail now says
"create unlimited new tokens" instead of "mint unlimited".
Title keeps "Mint approval" since it literally references
fromListId: "Mint".
- "Mint approvals can be modified — UNLIMITED SUPPLY RISK"
detail rewrites "mint approvals" → "token-creation (Mint)
approvals" and "mint limits" → "creation limits".
- "Backing approvals can be modified" detail drops the awkward
"smart-token mint source" phrasing.
- "Token ID creation is not locked" recommendation drops
"locked mint approvals" in favor of "locked token-creation
approvals".
review-ux/approvals.ts
- all_includes_mint detail: "allows minting new tokens" →
"allows new tokens to be created from the Mint address";
"post-mint transfer approvals" → "approvals that should
only govern tokens already in circulation".
- auto_approve_disabled_on_mintable detail: "mint approvals"
→ "approvals with fromListId: Mint"; "minted tokens" →
"created tokens"; "minting will silently fail" →
"creation flow will silently fail".
- forceful_transfers_not_locked title: "Forceful transfers
permitted post-mint" → "Tokens can be moved without holder
consent".
review-ux/skills.ts
- no_mint_approvals finding retitled "No token-creation
approvals configured". Detail explicitly notes smart tokens
using IBC backing or wrapper paths are excluded.
verify-standards.ts
- common mint-approval autoApprove check: "mint approvals"
→ "token-creation (Mint) approvals"; "minted tokens" →
"newly created tokens".
Other mint-centric text kept unchanged where the finding is
literally about the "Mint" list ID (e.g. approvals that require
overridesFromOutgoingApprovals because they send from Mint) —
the technical term is accurate and a plain-English rewrite would
lose the field-level specificity a developer needs.
Tests: 1143/1143 core green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…h branch Case B of the forceful check (no current override approvals + invariant does not block them) previously used the title "Tokens can be moved without holder consent". That's accurate for case A (real overrides present), but for case B where the count is 0, no tokens can currently be moved forcefully — the finding is about future risk, not current state. Retitled to "Forceful transfers are not permanently blocked" which is forward-looking and matches the detail text (which talks about "any approval the manager adds later could introduce forceful transfers"). Case A keeps "Forceful transfer overrides enabled" since real approvals with override flags exist there. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…fund templates
Mirror the frontend security cleanup (b38538d0c) in the SDK's own
template builders and their protocol validator.
Changes match the same rules the frontend registries now follow:
* Mint approvals keep `overridesFromOutgoingApprovals: true` (required
by the token standard) but drop `overridesToIncomingApprovals: true`
(redundant — recipients auto-approve via defaultBalances).
* Non-mint approvals (settlement / redeem / refund / burn) drop BOTH
override flags. The holder self-initiates their own burn and
auto-approves via `autoApproveSelfInitiatedOutgoingTransfers: true`
on defaultBalances. Previously the outgoing override let any third
party initiate the burn AND redirect the payout via
`overrideToWithInitiator: true` on the coinTransfer — a theft
vulnerability on pm-redeem, pm-settle-*, and crowdfund-refund,
plus a griefing vector on auction-burn and crowdfund-burn.
* No `requireFromEqualsInitiatedBy: true` added — the default behavior
is correct (self-initiated self-burns auto-approve; third parties
have no path unless the holder grants an explicit user-level
outgoing approval). Leaves flows open-ended.
Files:
core/builders/crowdfund.ts
- deposit-refund (Mint): incoming override → false
- deposit-progress (Mint): incoming override → false
- refund (!Mint): both overrides → false
- invariant noForcefulPostMintTransfers → true
core/builders/auction.ts
- mint-to-winner (Mint): already had incoming: false, no change
- auction-burn (!Mint): already had no approvalCriteria (no overrides)
- invariant noForcefulPostMintTransfers → true
core/builders/prediction-market.ts
- pairedMint (Mint): incoming override → false
- preRedeem (!Mint): both overrides → false; requireFromEqualsInitiatedBy
removed (leave open-ended)
- settlementApproval (!Mint): both overrides → false (4 variants:
settle-yes / settle-no / settle-push-yes /
settle-push-no)
- invariant noForcefulPostMintTransfers → true
core/crowdfunds.ts (protocol validator)
- Removed the requirement that deposit-refund and deposit-progress
approvals set `overridesToIncomingApprovals: true`. Comment
explains the rationale — the collection-level incoming override
is redundant with defaultBalances auto-approve and no longer
required by the protocol.
Tests: 1143/1143 core green including builders.spec.ts which exercises
every template through verifyStandardsCompliance.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…a (single source of truth)
Collapse the placeholder sidecar onto a single canonical location:
`tx.messages[i].value._meta.metadataPlaceholders`. Every producer
writes there, every consumer reads from there, no fallback paths,
no top-level copy anywhere.
Prior state carried the sidecar in two parallel shapes (top-level
`tx.metadataPlaceholders` for the AI builder flow, per-msg
`msg._meta.metadataPlaceholders` for CLI templates). Both had to be
kept in sync at every producer, and every consumer had to know which
one to check first. The split caused image auto-apply bugs on multi-
hop data flows where the shapes drifted.
SDK changes:
- buildMsg() now writes `msg.value._meta.metadataPlaceholders`
instead of `msg._meta.metadataPlaceholders` so the sidecar is
scoped inside the value (same scope as other per-msg _meta fields
on intent/bid/listing/recurring-payment/pm-*-intent builders).
- review-normalize.ts `reattachInlineMetadata` now preserves `_meta`
through the MsgUniversalUpdateCollection proto-class round-trip
(proto classes strip unknown fields, so the review pipeline would
otherwise drop `_meta` between normalize and check).
- review-ux/metadata.ts three checks (placeholder_images,
unnamed_approvals, alias_paths_missing_images) now read from
`value?._meta?.metadataPlaceholders`. Fixtures in review.spec.ts
updated to match.
- sessionState.ts drops the top-level `metadataPlaceholders` field
from the SessionTransaction interface and every writer. New
`getMsgPlaceholders(session)` helper lazily creates
`messages[0].value._meta.metadataPlaceholders` on first use; all
four writer sites (setCollectionMetadata, setTokenMetadata,
addApproval, setApprovalMetadata) route through it. export/import
no longer carry a sibling field — the sidecar rides inside
`messages` automatically.
- tools/session/addAliasPath + addCosmosWrapperPath rewired through
getMsgPlaceholders.
- CLI templates.ts writes `data.value._meta.metadataPlaceholders`
and renders from the same location. CLI builder.ts `preview`
upload stops lifting to top-level — just POSTs the tx as-is.
doctor-probe fixture moved onto per-msg shape.
- Prompt resources (masterPrompt.ts, examplesDocs.ts) rewritten to
show placeholder sidecars inside `messages[0].value._meta` in
every example JSON block.
Also landed in this commit (from the same working session, all
interconnected with the review + update-flow work):
- Auction post-settlement validator: treat missing mint-to-winner
approval as a valid post-settlement state (autoDeletionOptions.
afterOneUse removes it after first use) instead of a hard error.
- Auction builder now mirrors custom-2fa single-NFT pattern — token
metadata inherits collection metadata automatically via
METADATA_TOKEN_DEFAULT with tokenIds narrowed to [1,1].
- buildAliasPath returns {path, placeholders} so smart-account,
vault, credit-token, prediction-market all emit per-msg sidecars
for their alias path metadata URIs. Previous behavior silently
dropped the image arg.
- verify-standards.ts takes optional onChainCollection to overlay
create-only fields (defaultBalances, invariants) on update txs so
verifyCommonMintRules doesn't false-positive on update flows.
isUpdateTransaction() short-circuit for when onChainCollection is
absent. review.ts threads ctx.onChainCollection through.
- Security fixes for auction/PM/crowdfund override flags on non-Mint
approvals (now rely on defaultBalances auto-approve + self-
initiated outgoing defaults).
- KNOWN_SYSTEM_APPROVAL_PREFIXES for unnamed_approvals check — AI
builder uses `smart-token-backing_<hex>` style IDs that the old
fixed ID list missed.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…owdfund skill text Port the skill-doc generator from bitbadges-builder-mcp/scripts/gen-skill-docs.ts to bitbadgesjs-sdk/scripts/gen-skill-docs.ts with the new import path (src/builder/resources/skillInstructions.ts). Output destination unchanged (../bitbadges-docs/x-tokenization/examples/skills). Also tighten the auction / PM / crowdfund skill summary text to reflect the override-flag drops on non-mint approvals that landed earlier this session: - Auction: noForcefulPostMintTransfers locked in invariants, burn approval has no approvalCriteria at all (relies on defaultBalances + burn destination), post-settlement auto-deletion of mint-to-winner is documented as a valid state per the validator fix. - PM: noForcefulPostMintTransfers documented, non-mint approvals (redeem, settlement, transferable) must not set override flags. - Crowdfund: noForcefulPostMintTransfers documented, refund approval must not set override flags. Deposit/success/progress Mint-side approvals keep overridesFromOutgoingApprovals: true as the chain requires, with overridesToIncomingApprovals: false. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bitbadges-builder-mcppackage intobitbadgesjs-sdk/src/mcp/(78 files). The separate repo is now obsolete../mcp,./mcp/registry,./mcp/tools,./mcp/resources,./mcp/skills,./mcp/session. AddstypesVersionsso consumers on classicmoduleResolution: nodestill resolve the types.bitbadges-builder-mcpbin name as a second entry onbitbadgesjs-sdk— existing Claude Desktop configs continue to work after switching the install fromnpm i -g bitbadges-builder-mcptonpm i -g bitbadgesjs-sdk.bitbadges-cli mcpcommand group (list,call,session,resources) that dispatches the registry in-process — no subprocess, no MCP protocol round-trip. Sessions persist under~/.bitbadges/sessions/<id>.jsonso agents can compose a collection across multiple CLI invocations.src/core/validate.ts(~90 cases) that was sitting uncommitted locally. Split into its own commit for reviewability.Why
Two coupled problems:
file:path, we ended up with SDK →bitbadges-builder-mcp→bitbadgesjs-sdk. Publishing was a chicken-and-egg problem, two physical SDK copies lived in the graph, and consumers hitinstanceoffailures across the boundary.Folding the MCP into the SDK collapses both. One install, one import, one registry.
Companion PRs
src/routes/ai-builder/imports frombitbadges-builder-mcp/tools→bitbadgesjs-sdk/mcp/tools.bitbadges-builder-mcp→bitbadgesjs-sdk.reviewItemssidebar becomes a thin adapter over the SDK'sreviewCollection(). Depends on this PR (forreviewCollection/Finding/ReviewContext) and bitbadges-indexer#92.bitbadgeschaind mcpcobra delegation that forwards tobitbadges-cli mcp(long-lived, merges on hard fork).bitbadgeschain / feat/v30has abitbadgeschaind mcpdelegation command plus a.gitignorefix (landed on the existing feature branch, no separate PR).Test plan
bun run buildclean (dual ESM + CJS, tsc-alias, madge circular check)bitbadges-cli mcp list --names→ 50 toolsbitbadges-cli mcp call get_current_timestamp,get_skill_instructions, unknown-tool error pathbitbadges-cli mcp resources list --uris→ 11 resourcesbitbadges-cli mcp resources read bitbadges://skills/all— body matches previous MCP resource contentset_collection_metadata→get_transactionwith--session demo, state persists to disknode dist/cjs/mcp/index.jsresponds totools/listJSON-RPCbun link bitbadgesjs-sdk)tsc --build ./srcclean with the new subpath importsbitbadges-builder-mcpGitHub repo after this PR lands🤖 Generated with Claude Code