Skip to content

feat(swarm-protocol): open mirror of the Arcanea swarm packaging protocol#78

Draft
frankxai wants to merge 6 commits into
mainfrom
claude/arcanea-web3-marketplace-op4qnl
Draft

feat(swarm-protocol): open mirror of the Arcanea swarm packaging protocol#78
frankxai wants to merge 6 commits into
mainfrom
claude/arcanea-web3-marketplace-op4qnl

Conversation

@frankxai

Copy link
Copy Markdown
Owner

What this is

Public, forkable mirror of @arcanea/swarm-protocol — the chain-agnostic packaging layer behind the Web3 agent-swarm marketplace (built in the production app frankxai/arcanea-ai-app#175). Per SIP: the orchestration pattern is forkable; encoded-self is not.

A Swarm Package Manifest is the unit that gets tokenized as a Swarm License NFT and metered per-invocation across Base, Polygon, and Solana. It composes: topology (queen + worker mesh) + portable agent specs + license terms + bps royalty split (sums to 10000) + SIP attestation + per-chain bindings.

Contents

  • packages/swarm-protocol/ — types, dependency-free validator, deterministic canonicalizer, pack-swarm CLI.
  • Three reference manifests: creative-author-council, catalog-kits, guardian-orchestration.

Zero runtime dependencies — compiles with plain tsc, tests with node --test (14/14).

Verify

cd packages/swarm-protocol && pnpm i && pnpm test
node dist/cli.js list && node dist/cli.js validate creative-author-council

🤖 Generated with Claude Code


Generated by Claude Code

…ocol

Public, forkable mirror of @arcanea/swarm-protocol (per SIP: the orchestration
pattern is forkable; encoded-self is not). The chain-agnostic Swarm Package
Manifest that backs the Web3 agent-swarm marketplace — topology + agent specs +
license terms + bps royalty split + SIP attestation + per-chain bindings, with a
dependency-free validator, deterministic canonicalizer, pack-swarm CLI, and the
three reference manifests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017NdpPwQn5L3o77D1f9PMp9
@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown

Deployment failed with the following error:

Invalid vercel.json file provided

@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
arcanea-2 Error Error Jun 23, 2026 10:45am
arcanea-web Ready Ready Preview, Comment, Open in v0 Jun 23, 2026 10:45am

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cfa7ea67-f937-4b72-a5fc-dc2fb6a131b9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/arcanea-web3-marketplace-op4qnl

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the @arcanea/swarm-protocol package, which provides a chain-agnostic packaging layer, validator, CLI, and reference manifests for Arcanea agent swarms. The code review highlights several critical improvements for robustness and type safety. Specifically, defensive checks should be added to the validator in schema.ts to prevent runtime TypeErrors when handling null or non-object elements in agents, royalty.recipients, and chains. Additionally, the validateAgent type guard should be corrected to return false upon validation failures, property spreading in buildManifest should be reordered to protect default fields from being overwritten, and the FNV-1a fingerprinting function should be updated to hash UTF-8 bytes instead of UTF-16 code units.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +117 to +124
if (!isNonEmptyString(topo.queen)) {
errors.push('topology.queen: required agent id');
} else if (agentIds.size && !agentIds.has(topo.queen)) {
errors.push(`topology.queen: '${topo.queen}' is not in agents`);
} else {
const q = (agents as AgentPackage[]).find((a) => a.id === topo.queen);
if (q && q.role !== 'queen') errors.push(`topology.queen: agent '${topo.queen}' must have role 'queen'`);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If agents is undefined or not an array, calling (agents as AgentPackage[]).find will throw a TypeError at runtime. Additionally, if agents contains null or non-object elements, accessing a.id will throw. Since this validator is designed to be dependency-free and return a structured validation result instead of throwing, we should add defensive checks to ensure agents is an array and that each element is a valid object before accessing properties.

Suggested change
if (!isNonEmptyString(topo.queen)) {
errors.push('topology.queen: required agent id');
} else if (agentIds.size && !agentIds.has(topo.queen)) {
errors.push(`topology.queen: '${topo.queen}' is not in agents`);
} else {
const q = (agents as AgentPackage[]).find((a) => a.id === topo.queen);
if (q && q.role !== 'queen') errors.push(`topology.queen: agent '${topo.queen}' must have role 'queen'`);
}
if (!isNonEmptyString(topo.queen)) {
errors.push('topology.queen: required agent id');
} else if (agentIds.size && !agentIds.has(topo.queen)) {
errors.push(`topology.queen: '${topo.queen}' is not in agents`);
} else if (Array.isArray(agents)) {
const q = (agents as any[]).find((a) => a && typeof a === 'object' && a.id === topo.queen);
if (q && q.role !== 'queen') errors.push(`topology.queen: agent '${topo.queen}' must have role 'queen'`);
}

Comment on lines +156 to +163
let sum = 0;
recips.forEach((r, i) => {
const ro = r as Record<string, unknown>;
if (!isNonEmptyString(ro.label)) errors.push(`royalty.recipients[${i}].label: required`);
if (!isNonEmptyString(ro.address)) errors.push(`royalty.recipients[${i}].address: required (use '<placeholder>')`);
if (!isInt(ro.bps) || (ro.bps as number) < 0) errors.push(`royalty.recipients[${i}].bps: required non-negative int`);
else sum += ro.bps as number;
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If royalty.recipients contains null or non-object elements, accessing ro.label or other properties will throw a TypeError at runtime. We should add a defensive check to ensure each recipient is a valid object before validating its properties.

      let sum = 0;
      recips.forEach((r, i) => {
        if (typeof r !== 'object' || r === null) {
          errors.push(`royalty.recipients[${i}]: must be an object`);
          return;
        }
        const ro = r as Record<string, unknown>;
        if (!isNonEmptyString(ro.label)) errors.push(`royalty.recipients[${i}].label: required`);
        if (!isNonEmptyString(ro.address)) errors.push(`royalty.recipients[${i}].address: required (use '<placeholder>')`);
        if (!isInt(ro.bps) || (ro.bps as number) < 0) errors.push(`royalty.recipients[${i}].bps: required non-negative int`);
        else sum += ro.bps as number;
      });

Comment on lines +195 to +201
} else {
chains.forEach((c, i) => {
const co = c as Record<string, unknown>;
if (!CHAINS.includes(co.chain as Chain)) errors.push(`chains[${i}].chain: base | polygon | solana`);
if (!STANDARDS.includes(co.standard as string)) errors.push(`chains[${i}].standard: invalid token standard`);
if (!isNonEmptyString(co.network)) errors.push(`chains[${i}].network: required label`);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If chains contains null or non-object elements, accessing co.chain or other properties will throw a TypeError at runtime. We should add a defensive check to ensure each chain binding is a valid object before validating its properties.

  } else {
    chains.forEach((c, i) => {
      if (typeof c !== 'object' || c === null) {
        errors.push(`chains[${i}]: must be an object`);
        return;
      }
      const co = c as Record<string, unknown>;
      if (!CHAINS.includes(co.chain as Chain)) errors.push(`chains[${i}].chain: base | polygon | solana`);
      if (!STANDARDS.includes(co.standard as string)) errors.push(`chains[${i}].standard: invalid token standard`);
      if (!isNonEmptyString(co.network)) errors.push(`chains[${i}].network: required label`);
    });

Comment on lines +46 to +67
function validateAgent(a: unknown, i: number, errors: string[]): a is AgentPackage {
const p = `agents[${i}]`;
if (typeof a !== 'object' || a === null) {
errors.push(`${p}: must be an object`);
return false;
}
const o = a as Record<string, unknown>;
if (!isNonEmptyString(o.id)) errors.push(`${p}.id: required non-empty string`);
if (!isNonEmptyString(o.name)) errors.push(`${p}.name: required non-empty string`);
if (!isNonEmptyString(o.title)) errors.push(`${p}.title: required non-empty string`);
if (o.role !== 'queen' && o.role !== 'worker') errors.push(`${p}.role: must be 'queen' | 'worker'`);
if (!isNonEmptyString(o.domain)) errors.push(`${p}.domain: required non-empty string`);
if (!isNonEmptyString(o.voice)) errors.push(`${p}.voice: required non-empty string`);
if (!ELEMENTS.includes(o.element as ElementAffinity)) errors.push(`${p}.element: invalid element`);
if (!isStringArray(o.capabilities)) errors.push(`${p}.capabilities: required string[]`);
if (o.systemPrompt !== undefined && !isString(o.systemPrompt)) errors.push(`${p}.systemPrompt: must be string`);
if (o.specUri !== undefined && !isString(o.specUri)) errors.push(`${p}.specUri: must be string`);
if (o.systemPrompt === undefined && o.specUri === undefined) {
errors.push(`${p}: must carry either systemPrompt or specUri (the licensed IP must be locatable)`);
}
return true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The validateAgent function is declared as a TypeScript type guard (a is AgentPackage), but it returns true even when there are validation errors (as long as a is an object). This violates the contract of a type guard and can lead to type-safety issues downstream. We should track if any errors were added during the validation of this agent and return false if so.

function validateAgent(a: unknown, i: number, errors: string[]): a is AgentPackage {
  const p = `agents[${i}]`;
  if (typeof a !== 'object' || a === null) {
    errors.push(`${p}: must be an object`);
    return false;
  }
  const o = a as Record<string, unknown>;
  const initialErrorsCount = errors.length;
  if (!isNonEmptyString(o.id)) errors.push(`${p}.id: required non-empty string`);
  if (!isNonEmptyString(o.name)) errors.push(`${p}.name: required non-empty string`);
  if (!isNonEmptyString(o.title)) errors.push(`${p}.title: required non-empty string`);
  if (o.role !== 'queen' && o.role !== 'worker') errors.push(`${p}.role: must be 'queen' | 'worker'`);
  if (!isNonEmptyString(o.domain)) errors.push(`${p}.domain: required non-empty string`);
  if (!isNonEmptyString(o.voice)) errors.push(`${p}.voice: required non-empty string`);
  if (!ELEMENTS.includes(o.element as ElementAffinity)) errors.push(`${p}.element: invalid element`);
  if (!isStringArray(o.capabilities)) errors.push(`${p}.capabilities: required string[]`);
  if (o.systemPrompt !== undefined && !isString(o.systemPrompt)) errors.push(`${p}.systemPrompt: must be string`);
  if (o.specUri !== undefined && !isString(o.specUri)) errors.push(`${p}.specUri: must be string`);
  if (o.systemPrompt === undefined && o.specUri === undefined) {
    errors.push(`${p}: must carry either systemPrompt or specUri (the licensed IP must be locatable)`);
  }
  return errors.length === initialErrorsCount;
}

Comment on lines +29 to +38
export function buildManifest(input: ManifestInput, now: string = new Date().toISOString()): SwarmManifest {
const manifest: SwarmManifest = {
manifestVersion: SWARM_MANIFEST_VERSION,
attestation: input.attestation ?? arcaneaAttestation(),
createdAt: input.createdAt ?? now,
updatedAt: input.updatedAt ?? now,
...stripUndefined(input),
} as SwarmManifest;
return assertValidSwarmManifest(manifest);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Spreading stripUndefined(input) last allows any extra properties on input (such as manifestVersion if passed from a raw JS object at runtime) to overwrite the defaults. To ensure runtime safety and robustness, we should spread stripUndefined(input) first, and then apply the default/required fields.

Suggested change
export function buildManifest(input: ManifestInput, now: string = new Date().toISOString()): SwarmManifest {
const manifest: SwarmManifest = {
manifestVersion: SWARM_MANIFEST_VERSION,
attestation: input.attestation ?? arcaneaAttestation(),
createdAt: input.createdAt ?? now,
updatedAt: input.updatedAt ?? now,
...stripUndefined(input),
} as SwarmManifest;
return assertValidSwarmManifest(manifest);
}
export function buildManifest(input: ManifestInput, now: string = new Date().toISOString()): SwarmManifest {
const manifest: SwarmManifest = {
...stripUndefined(input),
manifestVersion: SWARM_MANIFEST_VERSION,
attestation: input.attestation ?? arcaneaAttestation(),
createdAt: input.createdAt ?? now,
updatedAt: input.updatedAt ?? now,
} as SwarmManifest;
return assertValidSwarmManifest(manifest);
}

Comment on lines +73 to +81
export function manifestFingerprint(manifest: SwarmManifest): string {
const text = canonicalize(manifest);
let hash = 0x811c9dc5;
for (let i = 0; i < text.length; i++) {
hash ^= text.charCodeAt(i);
hash = Math.imul(hash, 0x01000193);
}
return (hash >>> 0).toString(16).padStart(8, '0');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Standard FNV-1a operates on bytes. Hashing UTF-16 code units directly via charCodeAt is non-standard and can lead to unexpected behavior with multi-byte characters (like emojis or non-ASCII text) because it XORs a 16-bit value into a 32-bit hash. Using TextEncoder to get UTF-8 bytes is standard and robust.

Suggested change
export function manifestFingerprint(manifest: SwarmManifest): string {
const text = canonicalize(manifest);
let hash = 0x811c9dc5;
for (let i = 0; i < text.length; i++) {
hash ^= text.charCodeAt(i);
hash = Math.imul(hash, 0x01000193);
}
return (hash >>> 0).toString(16).padStart(8, '0');
}
export function manifestFingerprint(manifest: SwarmManifest): string {
const text = canonicalize(manifest);
const bytes = new TextEncoder().encode(text);
let hash = 0x811c9dc5;
for (let i = 0; i < bytes.length; i++) {
hash ^= bytes[i];
hash = Math.imul(hash, 0x01000193);
}
return (hash >>> 0).toString(16).padStart(8, '0');
}

@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code Review — feat(swarm-protocol): Swarm Package Manifest Protocol

Overall this is a well-structured, zero-dependency package with clean separation of concerns and a solid test suite. A few bugs and gaps are worth addressing before merge.


Bugs

1. validateAgent always returns true regardless of errors (src/schema.ts:231)

function validateAgent(a: unknown, i: number, errors: string[]): a is AgentPackage {
  // … pushes errors …
  return true;  // ← always true, even when errors were added
}

The type predicate a is AgentPackage is misleading here — the return value is never used by the caller (it correctly relies on the errors array), but this signals a type guarantee it doesn't actually enforce. Change the return type to void or return errors.length === priorCount.

2. buildManifest spread order can be confusing (src/pack.ts:127–135)

const manifest: SwarmManifest = {
  manifestVersion: SWARM_MANIFEST_VERSION,
  attestation: input.attestation ?? arcaneaAttestation(),
  createdAt: input.createdAt ?? now,
  updatedAt: input.updatedAt ?? now,
  ...stripUndefined(input),  // ← overrides the explicit fields above if input carries them
} as SwarmManifest;

When input.attestation is defined, it's first assigned via ?? then overridden again by the spread — same value so no bug, but the logic is redundant. The ?? fallback is already correct; the spread duplication is noise and could cause surprising behaviour if the order is ever changed.

3. FNV-1a fingerprint operates on UTF-16 code units, not UTF-8 bytes (src/pack.ts:174)

hash ^= text.charCodeAt(i);  // UTF-16 code unit, not byte

Standard FNV-1a is defined over bytes. For manifests with non-ASCII content (e.g., Unicode characters in systemPrompt, agent names, or descriptions), this produces a different fingerprint than a byte-level FNV-1a implementation. The comment correctly calls this "not a cryptographic hash", but if external tooling (e.g., Rust/Go validators) re-implements the fingerprint, they'll get different results. Consider using TextEncoder for consistency, or explicitly document the UTF-16 variant.


Security

4. No scheme validation on specUri (src/schema.ts:246)

if (o.specUri !== undefined && !isString(o.specUri)) errors.push(...)

The type comment says ipfs:// | ar:// | https:// are the valid schemes, but the validator accepts any string. A file:// or HTTP URI with side-effects could be included without warning. Add a check:

const SPEC_URI_SCHEMES = ['ipfs://', 'ar://', 'https://'];
if (o.specUri !== undefined && !SPEC_URI_SCHEMES.some(s => (o.specUri as string).startsWith(s))) {
  errors.push(`${p}.specUri: must start with ipfs://, ar://, or https://`);
}

5. Placeholder addresses pass validation silently

<placeholder> addresses in royalty.recipients satisfy the isNonEmptyString check and will pass full validation. If a manifest is deployed with placeholder addresses, royalties route to an invalid destination. Consider adding a strict validation mode (or a separate assertDeployReady function) that rejects <placeholder> values.


Canon Alignment

6. Guardian element mismatch in guardian-orchestration.json

Per .arcanea/lore/CANON_LOCKED.md, the Heart Gate is Maylinn (love/healing). In the manifest, guardian-heart has "element": "Wind", and guardian-voice (Alera, truth/expression) also has "element": "Wind". Two wind guardians in the same swarm looks like a copy-paste slip. Maylinn's domain (love, healing) aligns more naturally with Water or Earth in the Five Elements system.


Test Coverage Gaps

7. CLI is untested (src/cli.ts — 90 lines, zero test coverage)

The list, validate, canonicalize, and fingerprint subcommands are real user-facing logic and exercise the resolver path. At minimum: a test that the CLI exits 0 on a valid slug and exits 1 on an invalid manifest.

8. Missing negative cases

  • Worker agent id not in agents array (only queen-not-in-agents is tested)
  • manifestVersion mismatch
  • Empty agents array
  • Duplicate agent ids
  • gateAlignment is accepted without validation — test or document that it's a free-form label

Minor / Style

9. typescript: "^6.0.0" in devDependencies

TypeScript 6.x is pre-release at the time of this PR. Pinning to "^5.x" is safer until TS 6 GA. The "ignoreDeprecations": "6.0" tsconfig flag suggests this is intentional — if so, document why (e.g. which TS 6 feature is required).

10. pack-swarm binary requires dist/ to be built before use

There's no prepare script, so npm install -g from source will install a broken binary until pnpm build is run. Adding "prepare": "pnpm build" to package.json would fix this.


What's Working Well

  • Zero-runtime-dependency design is the right call for a protocol package.
  • Accumulating all validation errors (vs. failing fast) is user-friendly and the right default.
  • Deterministic canonicalize is correctly implemented — key-sorted JSON is a well-understood primitive for IPFS CID stability.
  • buildManifest validating on construction is a good guard.
  • The three reference manifests are well-formed and serve as clear examples.
  • 14 tests covering the core invariants (bps sum, topology role references, agent IP requirement, chain binding) are solid.

Suggested merge criteria:

  • Fix bugs 1 and 4 (misleading type predicate, unvalidated specUri)
  • Add CLI smoke tests
  • Confirm or correct the guardian-heart element

Mirror of the arcanea-ai-app hardening pass. CI root cause was a new
package.json without a matching pnpm-lock.yaml update; regenerated the lockfile.

Validator: null/object guards (never throws), real validateAgent pass/fail,
specUri scheme allow-list, new checkDeployReady() strict gate. pack.ts:
safer buildManifest spread + UTF-8 FNV-1a fingerprint. cli.ts: exported,
unit-testable main(). +9 tests (23/23). manifest: guardian-heart Wind -> Water.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017NdpPwQn5L3o77D1f9PMp9
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code Review — packages/swarm-protocol

Overall this is clean, well-structured work. Zero runtime dependencies, validator that accumulates all errors, injectable now for deterministic tests, and the two-tier validate / checkDeployReady pattern are all solid design choices. The 23 tests cover the important positive + negative paths. Below are findings from most to least critical.


Bug

topology.workers role is not enforced by the validator

schema.ts correctly validates that the queen agent has role: 'queen', but the workers block only checks existence, not role:

// Queen — role IS checked:
if (q && q.role !== 'queen') errors.push(`topology.queen: agent '${topo.queen}' must have role 'queen'`);

// Workers — role is NOT checked, only existence:
(topo.workers as string[]).forEach((w) => {
  if (agentIds.size && !agentIds.has(w)) errors.push(`topology.workers: '${w}' is not in agents`);
});

A manifest that lists a role: 'queen' agent in topology.workers (or a role: 'worker' in both queen and workers) will pass validateSwarmManifest. The bundled manifests are correct, so the regression surface is only in hand-authored manifests — but the validator should catch it.

Fix: add a mirror of the queen check for workers:

(topo.workers as string[]).forEach((w) => {
  if (agentIds.size && !agentIds.has(w)) errors.push(`topology.workers: '${w}' is not in agents`);
  else if (Array.isArray(agents)) {
    const wAgent = (agents as Array<Partial<AgentPackage> | null | undefined>).find((a) => a?.id === w);
    if (wAgent && wAgent.role !== 'worker') errors.push(`topology.workers: agent '${w}' must have role 'worker'`);
  }
});

Security

cli.ts resolvePath accepts unsanitized user paths

function resolvePath(arg: string): string {
  if (existsSync(arg)) return arg;
  const asSlug = join(MANIFEST_DIR, arg.endsWith('.json') ? arg : `${arg}.json`);
  ...
}

The raw CLI argument is passed directly to existsSync and resolved with join. A caller can supply ../../etc/passwd or a network path. This is low risk for a local CLI (the user already has filesystem access), but the project's security rules require sanitizing file paths.

Suggested mitigation — resolve to an absolute path and check that non-slug arguments are within a safe root, or at minimum warn in the README that this CLI must not be exposed via web processes or shell injection:

import { resolve } from 'path';
// At the top of resolvePath, reject obviously bad input:
if (arg.includes('\0') || arg.startsWith('-')) throw new Error(`Invalid path: '${arg}'`);

Design / Maintainability

export * from all modules in index.ts

export * from './types';
export * from './licensing';
export * from './schema';
export * from './pack';

Four star-exports mean any future name addition in any module silently shadows another. TypeScript raises an error only on duplicate value exports (not type exports), so type Foo collisions are invisible. Explicit named exports would make the public surface intentional and auditable at a glance — especially important for a protocol package whose API will be consumed by external tooling.


PLACEHOLDER constant is not exported

schema.ts defines const PLACEHOLDER = '<placeholder>' privately. checkDeployReady compares addresses against it. Any code (or manifest author) that uses a slightly different string (e.g. <TBD> or an empty string) will silently pass the deploy gate. Consider exporting it from licensing.ts (where the royalty helpers live) so all consumers share one source of truth.


ignoreDeprecations: "6.0" in tsconfig.json

"ignoreDeprecations": "6.0"

This suppresses TypeScript 6.0 deprecation warnings. It's not blocking anything today but it signals unaddressed usage of APIs that will be removed in a future release. Worth tracking which deprecated construct triggers this and fixing it before the next TypeScript major upgrade.


Test Coverage

Validator: no test for worker-role enforcement

test('topology queen + workers reference real agents with correct roles', ...) walks the bundled manifests and checks correct roles — but it doesn't exercise what the validator does when a worker has the wrong role. Adding a negative case to manifest.test.ts would close this:

test('rejects a topology worker with role queen', () => {
  const m = loadManifest('creative-author-council');
  const worker = m.agents.find((a) => a.role === 'worker')!;
  (worker as unknown as Record<string, unknown>).role = 'queen';
  const res = validateSwarmManifest(m);
  assert.equal(res.valid, false);
  assert.ok(res.errors.some((e) => e.includes("must have role 'worker'")));
});

CLI: canonicalize and fingerprint subcommands not tested through the CLI

Both are tested as functions directly, which is the important part. Adding a thin CLI smoke test similar to the validate test would ensure the wiring is complete, but this is low priority.


Minor Observations (no action needed)

  • FNV-1a fingerprint — correctly documented as non-cryptographic. Good discipline.
  • Canon alignmentguardian-heart element corrected to Water per the commit message; all Guardian names/elements match .arcanea/lore/CANON_LOCKED.md.
  • buildManifest default now parameter — evaluated at call time (correct JavaScript semantics), injectable for tests (good). No issue.
  • ChainBinding.network — confirmed present in types.ts as non-optional string, consistent with validator and manifests.
  • Pricing amounts as strings"299000000" / "150000" as strings avoids float drift for on-chain amounts. Correct choice.

Summary

Category Finding Severity
Bug Worker role not validated in validateSwarmManifest Medium
Security Unsanitized path in CLI resolvePath Low
Design export * star exports in index.ts Low
Design PLACEHOLDER unexported Low
Config ignoreDeprecations: "6.0" not tracked Low
Tests Missing negative test for worker-role enforcement Low

The worker role check and the PLACEHOLDER export are worth fixing before merge. The rest are advisory.

Test Packages CI failed at `actions/setup-node@v5` with "Unable to locate
executable file: pnpm": setup-node's default package-manager-cache detects
pnpm (root packageManager field) and runs `pnpm store path` before pnpm is
installed. Move `pnpm/action-setup` ahead of `setup-node` (and set cache: pnpm
explicitly) so the cache step finds pnpm.

Also remove an orphaned gitlink at a literal Windows path
('C:\Users\Frank\.claude\plugins\marketplaces\claude-plugins-official') that
had no .gitmodules entry and made `git submodule foreach` exit 128 during
checkout cleanup.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017NdpPwQn5L3o77D1f9PMp9
Mirror of arcanea-ai-app. validateSwarmManifest now enforces that every
topology.workers entry has role 'worker' (previously only the queen's role was
checked). Export PLACEHOLDER so consumers share one deploy-gate sentinel.
+1 negative test (24/24).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017NdpPwQn5L3o77D1f9PMp9
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code Review — feat(swarm-protocol): open mirror of the Arcanea swarm packaging protocol

Overall this is a well-designed, cleanly-implemented package. The zero-runtime-dependency choice is exactly right for a protocol layer that needs to be usable in chain-adjacent environments. Here's a breakdown:


Positives

  • Zero-dependency design — compiles with plain tsc and tests with node --test with no Zod, no Ajv. Correct call for a protocol package.
  • BPS sum invariant — royalty recipients are validated to sum exactly to 10 000 bps. This is a critical invariant and it's correctly enforced.
  • Worker role validation (added in second commit) — every topology.workers entry is now checked for role === 'worker'. Good hardening.
  • CI ordering fix — moving pnpm/action-setup before setup-node (with cache: pnpm) is the correct fix for the "pnpm not found during cache step" failure.
  • Broken submodule removal — dropping the stale C:\Users\Frank\... gitlink was the right call.
  • Maylinn element correction (guardian-heart: Wind → Water) aligns with the canonical Heart Gate domain (love/healing = Water). Canon-consistent.
  • buildManifest + assertValidSwarmManifest pair — callers cannot construct an invalid manifest; validation happens at build time, not just at validate-time. Good defensive API.
  • Deterministic canonicalization — recursive key-sort is the right strategy for reproducible IPFS CIDs.
  • FNV-1a documented as non-cryptographic — important caveat, clearly stated.
  • PLACEHOLDER sentinel exported — consumers can reference it rather than hardcoding the string.

Issues / Suggestions

1. gateAlignment field is unvalidated (minor–medium)

All three reference manifests include a gateAlignment field ("flow", "source", "unity"), but validateSwarmManifest in schema.ts never checks it. If this field is part of the public type, invalid values (e.g. "unknown") will silently pass validation. Either:

  • Add a GATE_ALIGNMENTS allow-list and validate it, or
  • Drop the field from validated manifests if it's purely informational.

2. TypeScript version mismatch (medium)

package.json pins typescript: "^6.0.0" as a dev dependency. If the monorepo root resolves a different TypeScript version (likely 5.x given the project's current state), you may get subtle type-incompatibility errors in CI on other Node matrix entries. Pin to the same major the workspace root uses, or hoist the devDependency.

3. loadJson parameter shadows the path import name (nit)

// cli.ts
import { join } from 'path';
// ...
function loadJson(path: string): unknown {   // 'path' as param — confusing next to 'path' module
  return JSON.parse(readFileSync(path, 'utf8'));
}

Since only join is destructured from 'path', there's no real shadowing bug, but renaming the param to filePath avoids reader confusion.

4. CLI reads arbitrary file paths without size guard (minor)

loadJson will happily parse a 500 MB JSON file into memory. For a local dev tool this is probably acceptable, but adding a simple stat-based file-size check (e.g. reject > 1 MB) before parsing would prevent accidental/malicious DoS on the process. Low priority given the tool's target context.

5. No test coverage for gateAlignment or CLI subcommands (minor)

The 24 tests cover manifest validation thoroughly, but:

  • No test for an invalid gateAlignment value (which currently passes silently — see issue 1).
  • No test for CLI main() with the fingerprint or canonicalize subcommands.
  • No negative test for oversized secondaryRoyaltyBps (> 10 000).

6. ipfs://<pending> in chain metadataUri (informational)

These pass the URI scheme check because <pending> starts with... wait, no — ipfs://<pending> starts with ipfs://, so it passes the SPEC_URI_SCHEMES prefix check. But the chain bindings are not subject to URI scheme validation in the current schema. That's fine for templates, but worth a note in the README that all <placeholder> / <pending> values must be replaced before minting.


Canon Alignment

  • Element table in the schema (Fire, Water, Earth, Wind, Void, Spirit) matches the canonical Five Elements.
  • guardian-sight Lyria using element: "Void" is canon-consistent (intuition/vision aligns with Void/potential).
  • The Lumina orchestrator using element: "Spirit" for the meta-swarm is correct per the Source Gate mapping.

Summary

Blocking: None.

Recommended before merge:

  • Validate gateAlignment or explicitly remove it from the schema type (Issue 1).
  • Verify TypeScript version alignment with the monorepo root (Issue 2).

Nice-to-have: Issues 3–6 above.

The core protocol logic is sound, the CI fixes are correct, and the lore/canon alignment is good. Ready to merge once the gateAlignment validation gap is resolved.

Mirror of arcanea-ai-app. gateAlignment (optional) is now checked against the
10 canonical Guardian Gates instead of passing any string. +2 tests
(gateAlignment allow-list, secondaryRoyaltyBps over-cap). 26/26.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017NdpPwQn5L3o77D1f9PMp9
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code Review — feat(swarm-protocol): open mirror of the Arcanea swarm packaging protocol

Overall: well-structured, zero-dependency package with solid test coverage and clear separation of concerns. A few issues worth addressing before merge.


Bugs / Correctness

1. Test path assumes compiled output location

In test/manifest.test.ts:

const CLI = join(__dirname, '..', '..', 'dist', 'cli.js');
const MANIFEST_DIR = join(__dirname, '..', '..', 'manifests');

This test compiles into dist-test/test/manifest.test.js. From there, ../../dist/cli.js resolves to the correct path — but only if pnpm build ran before pnpm test. The test script already enforces this (pnpm run build && pnpm run build:test && node --test ...), so the ordering is safe. However, the CLI test will silently produce a misleading error if someone runs node --test dist-test/... without building first. A guard or clearer error would help.

2. specUri placeholder values pass schema validation

Manifests ship "specUri": "ipfs://<story-writer-spec>" which passes the scheme check (startsWith('ipfs://')) in the validator. These are clearly not real CIDs. checkDeployReady catches this for contract addresses, but not for specUri — a specUri containing < is still accepted as valid. Consider adding the same placeholder check to checkDeployReady for specUri fields.

3. Non-deterministic buildManifest default

export function buildManifest(input: ManifestInput, now: string = new Date().toISOString()): SwarmManifest {

Two successive calls without injecting now can produce different createdAt/updatedAt strings at second boundaries, making reproducible pinning fragile if callers forget the second argument. The README should warn that callers who care about determinism must supply now.


Security

4. CLI path traversal not sanitized

resolvePath(arg) in cli.ts accepts user-supplied strings without sanitization:

const asSlug = join(MANIFEST_DIR, arg.endsWith('.json') ? arg : `${arg}.json`);

A path like ../../sensitive-file resolves to an absolute path outside manifests/. This is a CLI-only risk (not a web endpoint), so blast radius is bounded to what the caller can already read — but worth noting. At minimum, reject slugs containing / or \ with a clear error message.

5. Good: execFileSync (not exec) in tests

Correct choice — prevents shell injection in test helpers.


Code Quality

6. private: true inconsistency

package.json declares "private": true but also ships name, exports, types, main, and a files list as if it were a publishable package. If this is workspace-internal only, the publishing fields are noise. If it's meant to be published, "private": true must be removed. The PR description calls this a "forkable mirror," which suggests it may be intended for eventual publication — clarify and align.

7. TypeScript version mismatch

This package declares "typescript": "^6.0.0" in devDependencies while the rest of the monorepo uses typescript@5.9.3. The ignoreDeprecations: "6.0" in tsconfig confirms the intent, but this divergence should be intentional and documented in the package README or a comment in tsconfig.json, otherwise a future pnpm update could silently break other packages.

8. Removed submodule at Windows path

-C:\Users\Frank\.claude\plugins\marketplaces\claude-plugins-official

This was a git submodule accidentally committed from a developer's local machine. The deletion is correct, but it may be worth a sentence in the PR description explaining this so reviewers don't think it's an unrelated change.


Missing Test Coverage

The test suite is comprehensive (14 tests, good negative-path coverage), but these gaps are worth closing:

  • CLI canonicalize and fingerprint commands — only validate and list are tested via execFileSync. Fingerprint output format and canonicalize stability aren't exercised end-to-end.
  • checkDeployReady happy path — there's no test for a manifest with real addresses that passes checkDeployReady. The negative path is tested; the positive isn't.
  • royaltyFromProfile with explicit addresses — only the placeholder/default path is tested.

Performance

canonicalize uses recursive deep-copy with key sorting — fine for manifest sizes in play here (<10 KB). manifestFingerprint uses FNV-1a (32-bit) and is correctly documented as non-cryptographic. No concerns at this scale.


Suggestions (Non-blocking)

  • CI packages-ci.yml change — moving pnpm setup before Node setup and adding cache: pnpm is the right order per actions/setup-node docs. Good fix.
  • PLACEHOLDER constant — Exporting PLACEHOLDER = '<placeholder>' from the public API is a nice touch; callers can reference it instead of spelling the string.
  • checkDeployReady extends validateSwarmManifest — clean composition pattern, no redundant validation.

Summary

Area Status
Code quality ✅ Clean
TypeScript types ✅ Well-typed
Zero-dep validator ✅ Good design
Test coverage ⚠️ CLI commands partially untested
Security ⚠️ CLI path traversal (low severity)
Bugs ⚠️ specUri placeholder leaks through validator; non-deterministic default now
private flag ⚠️ Contradicts publishing fields

Recommend addressing items 2, 3, and 6 before merge; items 1, 4, and the test gaps are lower priority but worth tracking.

Mirror of arcanea-ai-app. The deploy gate now flags agent specUris still
containing a placeholder marker, consistent with its address/metadataUri
checks — nothing with placeholders can be marked deploy-ready. +2 tests
(deploy-ready happy path, specUri placeholder rejection). 28/28.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017NdpPwQn5L3o77D1f9PMp9
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code Review — feat(swarm-protocol): open mirror of the Arcanea swarm packaging protocol

Overall this is a well-structured, thoughtfully designed package. Zero runtime deps, strict TypeScript, accumulated-error validation, and a clean separation of concerns (types / schema / pack / licensing / CLI) are all good choices. The test suite is solid. A few items worth addressing before merge:


Bugs / Correctness

1. TypeScript 6 with ignoreDeprecations: "6.0" (medium)
packages/swarm-protocol/tsconfig.json targets typescript: "^6.0.0" (package.json) and uses "ignoreDeprecations": "6.0". TypeScript 6 is pre-release; the ignoreDeprecations flag suppresses deprecation errors by design, but it also means compile-time protections are silently disabled. If this is intentional (e.g. aligning with a beta), it should be noted in the PR description. Consider pinning to 5.x until TS 6 is stable, or at minimum document why 6.0 is required.

2. buildManifest intermediate as SwarmManifest cast before validation
In pack.ts:659–665, the object is cast to SwarmManifest before assertValidSwarmManifest runs. The spread + cast means TypeScript won't catch unknown extra keys from input. This is a cosmetic issue since runtime validation catches everything, but the cast is slightly misleading. Consider typing the intermediate as unknown and letting the validator coerce it.

3. CLI test references dist/cli.js directly (minor)
manifest.test.ts:1207: const CLI = join(__dirname, '..', '..', 'dist', 'cli.js'). The test script (pnpm test) runs pnpm run build first, so the dist file will exist. However, the test will silently succeed or fail depending on build order if tests are run in isolation. A guard that throws a readable error if CLI doesn't exist would make failures easier to diagnose.

4. manifestFingerprint uses TextEncoder (low)
TextEncoder is a browser Web API available globally in Node.js ≥11. This works, but since the entire package is Node-targeted, the comment should clarify this is Web API usage (so a future consumer doesn't accidentally import this in a worker or edge function without the API).


Security

5. No size cap on CLI JSON loading
cli.ts:461: readFileSync(path, 'utf8') then JSON.parse(...) with no size limit. For a local CLI this is acceptable (the user controls the filesystem), but note it if you ever expose loadJson as a library function.

6. Placeholder addresses pass basic validation
This is intentional and well-documented (PLACEHOLDER = '<placeholder>', and checkDeployReady catches them). The separation of validateSwarmManifest (structural) vs. checkDeployReady (deploy-gate) is the right design. Just make sure callers that matter (NFT mint flow) use checkDeployReady, not just validateSwarmManifest.


Performance

7. sortDeep creates new objects for every nested value
For the expected manifest sizes (tens of agents, not thousands) this is fine. If manifests grow significantly in the future, memoization or a streaming serializer would help, but no action needed now.

8. FNV-1a is 32-bit (~4 billion values)
The comment in pack.ts:699 already calls this out correctly ("Not a cryptographic hash — a cheap deterministic id for diffing and cache keys"). Make sure downstream consumers (cache keys, diffing) treat collisions as possible. Real pinning through IPFS CID is the correct path for uniqueness guarantees.


Test Coverage

The 14 tests cover the critical paths well. A few gaps worth adding:

  • Missing: CLI: canonicalize and CLI: fingerprint command tests. The validate and list commands have tests; the other two do not.
  • Missing: CLI: unknown command exit code. The error path (exit 2) for unrecognized commands is untested.
  • Missing: manifest with gateAlignment absent entirely — the happy path only tests the present+valid and present+invalid cases via the final test. Add a test for gateAlignment: undefined passing validation.
  • Consider: property-based style for royalty bps. The bps-sum invariant is tested on real manifests. A round-trip test with arbitrary valid splits (e.g. a few manually-crafted permutations) would catch edge cases in totalBps.

Lockfile / CI

9. pnpm/action-setup moved before setup-node — correct fix.
The reordering (setup pnpm → setup node with cache: pnpm) is the right change; setup-node needs pnpm installed to find the cache correctly.

10. Large lockfile removal (packages/ai-agents, packages/contracts)
The lock file removes ~4700 lines corresponding to ElizaOS, Hardhat, and Solidity contract dependencies. This is unrelated to the swarm-protocol addition and the PR description doesn't mention it. Please confirm these packages are intentionally being removed from the monorepo, and if so add a note to the PR body so reviewers know it's deliberate.

11. Windows-path submodule deletion — good cleanup.
"C:\\Users\\Frank\\.claude\\plugins\\marketplaces\\claude-plugins-official" was tracked as a submodule; removing it is correct. Worth noting in team standup how this ended up in the repo (git submodule from a Windows dev machine) so it doesn't recur.


Minor / Style

  • schema.ts:749: export const PLACEHOLDER = '<placeholder>' — consider whether this constant belongs in types.ts or licensing.ts so schema.ts doesn't leak a sentinel value into the public API surface. It's currently exported from index.ts via wildcard re-export.
  • royaltyFromProfile only accepts Exclude<RoyaltyProfile, 'custom'> — good. Consider a note in the JSDoc that callers who need a custom split should construct RoyaltySplit directly.
  • All three reference manifests use testnet addresses (base-sepolia, polygon-amoy, solana-devnet). Ensure the CI/deploy process explicitly gates on checkDeployReady before any mainnet interaction.

Summary

Category Status
Architecture ✅ Clean, well-separated
TypeScript types ✅ Strict, accurate
Validation ✅ Accumulates all errors, two-tier (structural + deploy)
Test coverage ⚠️ Missing CLI + edge-case tests
Security ✅ No critical issues; placeholder gating is correct
Performance ✅ Acceptable for expected scale
CI fix ✅ Correct
Lock file cleanup ⚠️ Needs confirmation that ai-agents/contracts removal is intentional
TS 6 dependency ⚠️ Pre-release; document or pin

Great foundation — address the TS 6 concern and the missing CLI tests and this is ready.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants