Skip to content

fix(marketplace-api): close dual-write drift gaps from PR #215 review#230

Merged
samxu01 merged 1 commit intomainfrom
fix/marketplace-api-dual-write-drift
Apr 24, 2026
Merged

fix(marketplace-api): close dual-write drift gaps from PR #215 review#230
samxu01 merged 1 commit intomainfrom
fix/marketplace-api-dual-write-drift

Conversation

@lilyshen0722
Copy link
Copy Markdown
Contributor

Summary

Follow-up to PR #215 addressing the 4 review comments that landed after merge. All changes are localized to backend/routes/marketplace-api.ts; the 5th review comment (ESM harmonization) was retracted — every .ts route in backend/routes/ uses the same CJS-in-TS pattern, so marketplace-api.ts is already conforming.

Fixes

# Path Severity Fix
1 POST /publish (update) P1 Wrap existing.save() in try/catch so an AR-succeeds + save-fails race returns 201 + drift warning instead of a generic 500 that hides the partial commit
2 POST /fork P1 Wrap Installable.create(forkDoc) the same way; skip source stats.forkCount bump on failure so a retry doesn't double-count
3 DELETE /manifests/:id P2 Add ADR-001 invariant #5 comment so a future refactor doesn't "helpfully" cascade-delete User rows / memory
4 RUNTIME_MAP P2 Add explicit 'local-cli': 'standalone' entry for ADR-005 (was falling through the || 'standalone' default silently)

Review comments addressed

Tests

New: backend/__tests__/unit/routes/marketplace-api.dual-write.test.js

  • update path: AR sync succeeds, existing.save() throws → 201 + warnings + isNew: false
  • fork path: AR sync succeeds, Installable.create throws → 201 + warnings + forkedFrom preserved + forkCount bump skipped

Full run:

__tests__/unit/routes/marketplace-api.dual-write.test.js     2/2 ✓
__tests__/unit/routes/marketplace.official.test.js           1/1 ✓
__tests__/unit/routes/registry.publish.test.js               2/2 ✓
__tests__/unit/models/installable.test.ts                  11/11 ✓

Scope notes

  • Happy paths are unchanged — diff only adds error branches + a comment + a map entry.
  • No schema changes, no migration, no API surface change.
  • Does not touch the legacy /api/marketplace/official router or the /api/registry/* install routes.

Test plan

  • npx jest __tests__/unit/routes/marketplace-api.dual-write.test.js — 2/2 pass
  • npx jest __tests__/unit/routes/marketplace.official.test.js __tests__/unit/routes/registry.publish.test.js __tests__/unit/models/installable.test.ts — 14/14 pass, no regression
  • npm run lint on changed file — no new errors
  • Manual smoke on dev after merge: publish v2 of an existing manifest, fork an active manifest, confirm 201 responses match shape

🤖 Generated with Claude Code

Follow-up on four review comments that landed after #215 was merged:

- Update path (POST /publish on existing manifest): `existing.save()`
  was outside the AR-sync try/catch, so a save failure after AR had
  already committed left the Installable catalog silently behind.
  Wrap save in a try/catch that returns the same 201 + drift warning
  the new-manifest path already uses.
- Fork path (POST /fork): `Installable.create(forkDoc)` was unwrapped,
  producing the same class of orphan if AR succeeded and create threw.
  Mirror the publish-path pattern and skip the source forkCount bump
  on failure so a retry doesn't double-count.
- Delete path (DELETE /manifests/:id): add ADR-001 invariant #5 comment
  noting User rows / memory / pod memberships are intentionally NOT
  cascade-deleted — prevents a future refactor from "helpfully" adding
  an identity cascade.
- `RUNTIME_MAP`: add explicit `'local-cli': 'standalone'` entry for
  ADR-005 so the mapping is self-documenting instead of falling through
  the `|| 'standalone'` default.

Adds `backend/__tests__/unit/routes/marketplace-api.dual-write.test.js`
covering both new drift-warning branches via mocked model failures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@samxu01 samxu01 merged commit 5cd6321 into main Apr 24, 2026
10 checks passed
samxu01 added a commit that referenced this pull request Apr 30, 2026
…249)

PR #248 marked Marketplace as paused based on incomplete picture.
PR #215 (Randy) and #230 (Lily) already shipped the marketplace
backend: 9 endpoints under /api/marketplace (publish, fork, browse,
detail, deprecate, manage), dual-write to Installable + AgentRegistry,
schema additions for forkedFrom/readme/forkCount.

Corrected:
- Marketplace frontend moves to active (mid-queue) — wires UI on top
  of the shipped backend; pre-flight is end-to-end verification on
  api-dev.commonly.me before any UI work.
- Installable taxonomy row clarified: Phase 2's marketplace-operations
  slice is shipped; Phase 3 read-path switch + recon cron + validators
  remain paused with a more specific reactivation trigger.
- New "Marketplace backend extensions" paused row guards against
  scope creep on already-shipped surface.
- Open question #4 reframed: not "build marketplace?" but "where in
  the queue does the frontend land?" — audit resolves it.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
samxu01 added a commit that referenced this pull request May 3, 2026
Round 2 of the doc sweep — tag the rest of the design vision and plan
specs with their actual implementation status, pointing readers at the
current source of truth (ADRs, COMMONLY_SCOPE.md, ADR-011 GTM frame, or
IMPLEMENTATION_SUMMARY.md as appropriate).

design/:
  * HYBRID_SOCIAL_PLATFORM — shipped (vision-level), active frame is ADR-011
  * UI_UX_ROADMAP — superseded by v2; pointer to frontend/design-system + ADR-011
  * SOCIAL_PLATFORM_FOR_AI_AGENTS — shipped via kernel/driver stack;
    note Commonly's product is hybrid not agent-only (Moltbook is the
    agent-only sibling)
  * MVP_COMPETITIVE_STRATEGY — Jan-2026 strategic frame, superseded by ADR-011
  * EXECUTIVE_SUMMARY — Jan-2026 snapshot, current state in COMMONLY_SCOPE + ADR-011
  * AGENT_ORCHESTRATOR — K8s shipped via ADR-009; runtime contract is
    ADR-004 (CAP) + ADR-005 (driver layer); live ops in AGENT_RUNTIME.md
  * MCP_APPS — foundation shipped via ADR-010 Phase 1; Phase 2+ paused
    under ADR-011
  * marketplace-publish-fork — backend shipped (PR #215+#230, 9 endpoints);
    frontend wiring is the active mid-queue track in ADR-011

plans/:
  * cheeky-riding-waffle — implemented (instanceId-aware ensemble
    routing + agentRuntimeAuth on response endpoint)
  * SOCIAL_FUN_FEATURES_SPEC — Phase 1 shipped per IMPLEMENTATION_SUMMARY;
    Phase 2 (agent-first summarization) reframed when ADR-011 paused
    Feed/Digest/Analytics for shell-first GTM

Net: every design + plan doc with stale framing now has a one-paragraph
banner pointing readers at the live contract. No content was deleted —
the original "why we built it this way" reasoning stays for new
contributors.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
samxu01 added a commit that referenced this pull request May 4, 2026
v3 had Part 4 scoped to "inspector Skills tab only" with agent
install/fork pushed entirely out of scope. That left the user-asked
v2 marketplace page and Agents tab undesigned.

v4 expands Part 4 into four sections:

  4a. Top-level V2 Marketplace page (/v2/marketplace) — unified
      Agents + Skills browse, replaces v1 AgentsHub.tsx (4954 lines)
      and SkillsCatalogPage.tsx (2061 lines) over time. Wires on top
      of existing /api/marketplace/* (PR #215+#230) and /api/skills/*.

  4b. Inspector Skills tab — per-pod contextual install. Shows which
      agents have which skills.

  4c. Inspector Agents tab — per-pod agent membership + install/fork
      affordances. Per-installed-agent: Talk to / Configure / Fork.
      Per-browse-agent: Install / Fork.

  4d. Shared <V2MarketplaceList> component (~400 lines) consumed by
      both inspector tabs and the Marketplace page. Future apps and
      widgets ports cleanly onto the same component.

Phasing expanded from 4 phases to 8 active + 2 deferred to cover the
new surfaces and a v1 deprecation cycle. Open questions #8–10 added
covering nav-rail placement, fork ownership backend, and
PersonalityBuilder UX migration.

This ADR now also lands the active "Marketplace frontend" track from
ADR-011 alongside the file-production work — they're the same surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
samxu01 added a commit that referenced this pull request May 4, 2026
…end state

v3 and v4 made two assumptions that turned out to be wrong:

1. Designed a new /v2/marketplace page from scratch — but the route
   already exists and renders v1 AppsMarketplacePage (771 lines) with
   Discover|Installed sub-tabs. /v2/agents/browse and /v2/skills also
   exist as separate v2 routes rendering v1 components. The right move
   is to extend AppsMarketplacePage in place with kind tabs (Apps ·
   Agents · Skills · Integrations), not author a new file.

2. Framed /api/marketplace/* (Randy, PR #215+#230) as legacy AR — but
   per #215's description, it's "ADR-001 Phase 2 for marketplace
   operations — first dual-write path between Installable (canonical)
   and AgentRegistry (compat shim)." The marketplace backend is
   Installable-canonical TODAY; what was missing is frontend wiring.

v5 corrections:

- Part 4a rewritten: extend existing AppsMarketplacePage with kind
  tabs, wire each tab to the right backend (Agents → Installable,
  Skills → AR for now, Apps/Integrations unchanged). Soft-redirect
  /v2/agents/browse and /v2/skills with deep-link query params.

- Part 4d shrunk: the shared piece is a small <MarketplaceCardList>
  subcomponent (~250 lines) extracted FROM the page in Phase 3 and
  reused BY the inspector tabs in Phase 4. v4's heavyweight
  <V2MarketplaceList> was over-engineering.

- Phasing collapsed from 8 active to 5 active. No new top-level page
  to design; v1 deprecation simplifies to one cleanup phase.

- §"Relationship to Installable taxonomy" rewritten to reflect the
  per-kind split — agents are already on Installable, skills are
  still on AR. ADR-013 is the marketplace-frontend track from
  ADR-011 by definition for the agent surface.

- Open question #7 (accelerate Phase 3?) closed/re-framed: the agent
  read-path switch is implicitly done by Randy's dual-write; the
  skills read-path switch defers until separately motivated.

- Open question #8 (nav-rail placement) closed: page already mounted.

- Open question #9 (fork ownership) narrowed: Randy's
  /api/marketplace/mine likely covers it; just verify response shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
samxu01 added a commit that referenced this pull request May 4, 2026
…ll surfaces (Marketplace + inspector tabs) (#287)

* docs(adr): ADR-013 agent file production — extension tool, dev skill bundles, v2 install UI

Decision spans four coordinated changes:

1. `commonly_attach_file` extension tool wrapping the upload endpoint and
   `[[upload:...]]` directive — protocol glue, not a skill, lives alongside
   `commonly_post_message` in the OpenClaw `commonly` extension.

2. Toolchain in `clawdbot-gateway` Dockerfile centered on OfficeCLI
   (Apache-2.0, pinned 30 MB static binary) for DOCX/XLSX/PPTX, plus pandoc
   for md→PDF, markitdown + pypdf for parse direction. ~170 MB image
   growth, replacing the ~600 MB python-office + LibreOffice stack the
   first draft proposed before OfficeCLI discovery.

3. Default skill bundles for the four production dev presets (Theo / Nova
   / Pixel / Ops) — currently shipping with empty `defaultSkills: []`.
   Adds `github`, `officecli`, `pandic-office`, `markdown-converter`,
   `pdf` (plus `tmux` for the engineers).

4. V2 inspector Skills tab (<400 lines, vs v1's 2,061-line catalog page) —
   search + installed list + recommended row + escape hatch to v1 Browse.

Includes "Relationship to Installable taxonomy (ADR-001)" mapping each
piece to the future Installable model and the migration story when
ADR-001 Phase 3 (currently paused per ADR-011) unpauses. None of the
work in this ADR becomes throwaway when Phase 3 lands; the data source
flips, the wire shape doesn't.

Rejected alternatives captured: server-side `/render` service, copying
Anthropic's proprietary skills bundle, authoring duplicate
Commonly-specific office skills, sub-agents per format, skill-only
approach (no kernel tool), and the python-office + LibreOffice stack
the first draft proposed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(adr): ADR-013 v4 — expand Part 4 to cover full v2 install surfaces

v3 had Part 4 scoped to "inspector Skills tab only" with agent
install/fork pushed entirely out of scope. That left the user-asked
v2 marketplace page and Agents tab undesigned.

v4 expands Part 4 into four sections:

  4a. Top-level V2 Marketplace page (/v2/marketplace) — unified
      Agents + Skills browse, replaces v1 AgentsHub.tsx (4954 lines)
      and SkillsCatalogPage.tsx (2061 lines) over time. Wires on top
      of existing /api/marketplace/* (PR #215+#230) and /api/skills/*.

  4b. Inspector Skills tab — per-pod contextual install. Shows which
      agents have which skills.

  4c. Inspector Agents tab — per-pod agent membership + install/fork
      affordances. Per-installed-agent: Talk to / Configure / Fork.
      Per-browse-agent: Install / Fork.

  4d. Shared <V2MarketplaceList> component (~400 lines) consumed by
      both inspector tabs and the Marketplace page. Future apps and
      widgets ports cleanly onto the same component.

Phasing expanded from 4 phases to 8 active + 2 deferred to cover the
new surfaces and a v1 deprecation cycle. Open questions #8–10 added
covering nav-rail placement, fork ownership backend, and
PersonalityBuilder UX migration.

This ADR now also lands the active "Marketplace frontend" track from
ADR-011 alongside the file-production work — they're the same surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(adr): ADR-013 v5 — correct framing after verifying backend/frontend state

v3 and v4 made two assumptions that turned out to be wrong:

1. Designed a new /v2/marketplace page from scratch — but the route
   already exists and renders v1 AppsMarketplacePage (771 lines) with
   Discover|Installed sub-tabs. /v2/agents/browse and /v2/skills also
   exist as separate v2 routes rendering v1 components. The right move
   is to extend AppsMarketplacePage in place with kind tabs (Apps ·
   Agents · Skills · Integrations), not author a new file.

2. Framed /api/marketplace/* (Randy, PR #215+#230) as legacy AR — but
   per #215's description, it's "ADR-001 Phase 2 for marketplace
   operations — first dual-write path between Installable (canonical)
   and AgentRegistry (compat shim)." The marketplace backend is
   Installable-canonical TODAY; what was missing is frontend wiring.

v5 corrections:

- Part 4a rewritten: extend existing AppsMarketplacePage with kind
  tabs, wire each tab to the right backend (Agents → Installable,
  Skills → AR for now, Apps/Integrations unchanged). Soft-redirect
  /v2/agents/browse and /v2/skills with deep-link query params.

- Part 4d shrunk: the shared piece is a small <MarketplaceCardList>
  subcomponent (~250 lines) extracted FROM the page in Phase 3 and
  reused BY the inspector tabs in Phase 4. v4's heavyweight
  <V2MarketplaceList> was over-engineering.

- Phasing collapsed from 8 active to 5 active. No new top-level page
  to design; v1 deprecation simplifies to one cleanup phase.

- §"Relationship to Installable taxonomy" rewritten to reflect the
  per-kind split — agents are already on Installable, skills are
  still on AR. ADR-013 is the marketplace-frontend track from
  ADR-011 by definition for the agent surface.

- Open question #7 (accelerate Phase 3?) closed/re-framed: the agent
  read-path switch is implicitly done by Randy's dual-write; the
  skills read-path switch defers until separately motivated.

- Open question #8 (nav-rail placement) closed: page already mounted.

- Open question #9 (fork ownership) narrowed: Randy's
  /api/marketplace/mine likely covers it; just verify response shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(adr): ADR-013 v5.1 — flag catalog index numbers as 2026-02-05 snapshot

Catalog index (docs/skills/awesome-agent-skills-index.json) has
updatedAt: 2026-02-05 — nearly 3 months stale at draft time. All
numbers derived from it (~1,659 skills, `github` skill 603 stars,
license fields) are point-in-time evidence, not live counts.

Added §Caveat in the Context section spelling this out, plus
inline annotations on each citation. Numbers fetched live via
GitHub API today (OfficeCLI 2,768 stars + v1.0.70 release date)
are explicitly timestamped 2026-05-03 and treated separately.

Reviewers should re-sync the catalog before any sales-pitchy or
external claims using the figures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(adr): ADR-013 v5.2 — replace invented mockup numbers with placeholders

The ASCII mockups for the marketplace page and inspector Agents tab
had invented numbers that looked authoritative:

- "Apps (12)", "Agents (78)" — kind-tab counts I made up
- "★ 4.8 · 240 installs" (Liz), "★ 4.6 · 88 installs" (Theo)
- "★ 4.5 · 175 installs" (Tarik)

Reviewers could mistake these for projections or commitments. Replaced
all with em-dash placeholders (—) and added a mockup note clarifying
that live values come from the marketplace API at runtime.

Same hygiene as v5.1's catalog-snapshot caveats — distinguish what's
real (cited with timestamp) from what's illustrative.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(adr): ADR-013 v5.3 — restore GitHub stars, add per-agent skill drawer

v5.2 over-redacted: removed real GitHub star counts along with the
fake Commonly ratings. v5.3 re-distinguishes:

- GitHub stars (real upstream signal): render. Catalog snapshot for
  catalog skills; live GitHub API for locally-bundled (officecli).
- Commonly review-style ratings (★ 4.8): don't render. The system
  doesn't exist; introducing it needs its own ADR.
- Install counts (real Commonly metric): render where cheap; omit
  otherwise.

Plus added §4d **Per-agent skill management drawer** — the friendly
UX the user flagged was missing. The pod-centric Skills tab (4b)
shows multi-agent rollups; the per-agent drawer is single-column,
single-agent, with inline [×] removal and preset-aware "Recommended
for ___" surfacing capability gaps.

Configure drawer opens from any agent click (Members tab, Agents
tab, Marketplace agent card). Tabs inside: Persona · Heartbeat ·
Skills · Memory. <300 lines target.

Existing 4d (shared subcomponent) renumbered to 4e. Open question
#11 added for per-agent install scoping in POST /api/skills/import.
Phase 4 expanded to cover the drawer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(adr): ADR-013 v5.4 — address 8 PR #287 review comments

Eight inline comments + PR-level summary on PR #287. All addressed:

1. Line 155 — "fall back to python-office stack" was a lie (Part 2
   removes it). Rewrote escape paths honestly: downgrade pin (cheap)
   → staged Dockerfile target (mid) → full re-author (expensive).
   Added binary-mirror requirement (AR/GCS) to insulate from
   upstream artifact swap. Documented operational ownership for
   2am checksum-mismatch incidents.

2. Line 130 — SHA256SUMS placeholder. Confirmed unverified whether
   iOfficeAI publishes SHA256SUMS. Two paths: upstream artifact OR
   committed-constant-checksum. Resolved in Phase 0a (i).
   Open question #13 added.

3. Line 516 — Open question #11 escalated to Phase 0a (ii). If
   syncOpenClawSkills is keyed only on accountId and multiple
   agents share that, per-agent install scoping is a real backend
   change, not a parameter add. Phase 4 may split into 4a/4b.

4. Line 461 — Phase 0b verification rewritten to be skill-free
   (uses acpx_run to invoke pandoc directly) since pandic-office
   doesn't ship until Phase 1. Full skill-loop verification moves
   to end of Phase 1.

5. Line 348 — Inline [×] UX. Committed to per-agent reprovision
   endpoint as the target with "Queued — applies on next
   reprovision" wording as acceptable v1. Eliminated misleading
   "removing on next sync" copy.

6. Line 373 — Configure drawer permissions. Flipped default to
   admin-only after review feedback. Per-field "visible to
   members" is explicit opt-in; Memory tab is never member-
   readable in v1 (ADR-003 §Visibility). Added Invariant 7.

7. Line 227 — Live GitHub stars. Moved from client-side direct
   fetch (60/hr unauth limit, no token in browser) to server-side
   proxy endpoint with TTL cache. Failure mode: ★ — with tooltip.

8. Line 469 — Phase 5 deletion gate. Replaced "no fallback
   complaints" with concrete <N hits/week telemetry threshold on
   redirect path. ~5 lines added to Phase 3 scope.

Plus Invariant 8 making Phase 0a prereqs non-optional, Open
question #12 on workspace-helper extractability, and PR estimate
widened from ~5 to 5–7 to reflect Phase 0a (ii) uncertainty.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(adr): ADR-013 v5.5 — call out acpx_run deprecation + ADR-005 composition

Per CLAUDE.md and ADR-005 Stage 3, acpx_run is being phased out in
favor of A2A-via-DM (agent-to-agent delegation through 1:1
agent-rooms) using wrapper agents like sam-local-codex.

ADR-013 doesn't block on the deprecation — it makes it easier:

- commonly_attach_file is correctly placed at the kernel layer
  (CAP verb), not in any specific runtime. Wrapper agents speaking
  CAP get file-attach for free.

- Phase 0b's acpx_run smoke test is a pragmatic bootstrap using
  the path that exists today. The shape of the test re-runs
  unchanged when ADR-005 Stage 3 completes.

- Default skill bundles in Part 3 are runtime-agnostic by
  construction — SKILL.md instructs the model to invoke binaries;
  the agent decides how to actually run them (acpx_run today,
  wrapper-agent local subprocess tomorrow).

Added §"Relationship to ADR-005" parallel to the existing ADR-001
relationship section. Listed ADR-005 in the companion header. Added
a bullet in §"What this unlocks" calling out that ADR-013 reinforces
ADR-005 Stage 3 rather than competing with it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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