Skip to content

feat: ACP SDK v2 → v3 migration Qchainer prep agent trader showcase - #94

Closed
drQedwards wants to merge 6 commits into
Virtual-Protocol:mainfrom
drQedwards:feat/acp-sdk-v2-to-v3-migration
Closed

feat: ACP SDK v2 → v3 migration Qchainer prep agent trader showcase#94
drQedwards wants to merge 6 commits into
Virtual-Protocol:mainfrom
drQedwards:feat/acp-sdk-v2-to-v3-migration

Conversation

@drQedwards

@drQedwards drQedwards commented Aug 1, 2026

Copy link
Copy Markdown

Showcase Project

What shipped

  • Project slug: acp-sdk-v2-to-v3-migration
  • Project title: ACP SDK v2 → v3 Migration
  • Builder name and URL: Dr. Q (@jk_drq) / Qchain — https://x.com/jk_drq
  • EconomyOS primitives used: wallet, acp
  • Public proof:
  • Optional soul.md: showcase/acp-sdk-v2-to-v3-migration/soul.md (public/redacted)

Project package

  • Added or updated showcase/acp-sdk-v2-to-v3-migration/showcase.json
  • Added demo artifacts, prompt, proof, and redacted report
  • Added reusable skill under showcase/acp-sdk-v2-to-v3-migration/skills/acp-sdk-v2-to-v3-migration/
  • Used top-level skills/<skill-name>/ only when the skill is shared across projects (this skill is project-specific)
  • Set skills[].sourcePath in showcase.json
  • Linked all public artifacts from the manifest
  • Included exactly three feedback prompts
  • hidden: true not set — publish card after merge
  • Linked soul.md as public/redacted agent context

Skill standard

  • Skill path: showcase/acp-sdk-v2-to-v3-migration/skills/acp-sdk-v2-to-v3-migration
  • SKILL.md includes when to use it and when not to use it
  • Inputs, tools, credentials, and preconditions are explicit
  • Approval gates are listed for spending / live mode / dashboard upgrade
  • Stop conditions and handoff rules are listed
  • Validation checks and output contract are included

Safety and redaction

  • No card numbers, CVVs, OTPs, magic links, API keys, access tokens, private prompts, wallet material, or private account records are published
  • Live workflow evidence is redacted (offline dry-run only)
  • Public/private boundaries are explained
  • Optional soul.md does not include private instructions, credentials, account data, wallet material, or operational secrets
  • X Space is public watch-page only — no scraped/re-hosted audio binary

Video / X proof

  • Builder X: https://x.com/jk_drq
  • Primary piano Space: https://x.com/i/spaces/1dKrPPWnNDzJX (Distorted Face Piano)
  • Manifest wiring:
    • links.video / links.share → Space URL
    • visual.videoLabel → names X
    • visual.videoUrl omitted (Spaces have no stable public video.twimg.com mp4)
    • visual.posterUrl → package poster PNG
  • Notes: showcase/acp-sdk-v2-to-v3-migration/proof/jk-drq-piano-spaces.md

Validation ran

node showcase/acp-sdk-v2-to-v3-migration/scripts/self-check.mjs   # 10/10 pass
node showcase/acp-sdk-v2-to-v3-migration/examples/v3-provider.mjs
node showcase/acp-sdk-v2-to-v3-migration/examples/v3-client.mjs
node scripts/validate-showcase.mjs                               # manifests OK

Publish path

After this PR is approved and merged to main, changes under showcase/** trigger the EconomyOS docs sync. The accepted manifest is published into /community#showcase by the docs workflow.

Qchain Druck added 3 commits August 1, 2026 18:43
Public offline-proofed migration kit mapping AcpClient callbacks to
AcpAgent entry events, AssetToken budgets, and session lifecycle actions.
Includes reusable skill, before/after examples, and self-check scripts.
Wire the builder's piano Space into showcase links/artifacts/videoLabel
per Spaces (page URL, no direct mp4) rules, and refresh PR proof notes.
@drQedwards

Copy link
Copy Markdown
Author

Public X video proof wired

Builder: @jk_drq

Primary piano Space used as links.video / links.share:

Manifest notes:

  • visual.videoLabel names X
  • visual.videoUrl omitted (Spaces page, no stable public amplify mp4)
  • local poster kept for the card
  • proof write-up: showcase/acp-sdk-v2-to-v3-migration/proof/jk-drq-piano-spaces.md

If you have additional piano Space URLs/status clips on @jk_drq that should be listed alongside this one, drop them in-thread and I’ll append artifacts.

Add t.me/Qchaingoldbot to showcase demo/artifacts and proof notes.
@drQedwards

Copy link
Copy Markdown
Author

Telegram demo surface

Public desk bot wired into the showcase:

  • https://t.me/Qchaingoldbot (@Qchaingoldbot)
  • Manifest: links.demo + artifact
  • Notes: showcase/acp-sdk-v2-to-v3-migration/proof/telegram-qchaingoldbot.md

X piano Space remains links.video / links.share.

@drQedwards drQedwards changed the title feat: ACP SDK v2 → v3 migration showcase feat: ACP SDK v2 → v3 migration Qchainer prep agent trader showcase Aug 1, 2026
Adds .github/workflows/showcase-acp-sdk-v2-to-v3-migration.yml so the
package is gated on every PR touching showcase/acp-sdk-v2-to-v3-migration:

- validate-showcase.mjs manifest validation
- node --check on all example sources (v3 provider/client, phase map, v2 legacy)
- npm run check (10 offline self-check assertions)
- skill installability into ~/.agents and ~/.claude
- proof artifact presence (piano Spaces, offline validation, Telegram, poster, soul)

Declares builder.association = "champion" in showcase.json and asserts it in
CI. Note: GitHub's own author_association is permission-derived and cannot be
set by a contributor; the workflow reports both values side by side in the job
summary so reviewers can see the declared vs. derived value.
@drQedwards

Copy link
Copy Markdown
Author

Pipeline is green on the fork — needs workflow approval upstream

Pushed 1998f69, which adds a package-scoped CI workflow and declares the builder association.

What's in the commit

  • .github/workflows/showcase-acp-sdk-v2-to-v3-migration.yml — gated on showcase/acp-sdk-v2-to-v3-migration/**
  • showcase.jsonbuilder.association: "champion", asserted in CI

Steps the workflow runs

  1. node scripts/validate-showcase.mjs — manifest validation
  2. node --check on v3-provider.mjs, v3-client.mjs, phase-event-map.mjs, v2-provider.legacy.mjs
  3. npm run check — 10 offline self-check assertions
  4. Skill installability into ~/.agents/skills and ~/.claude/skills
  5. Proof artifact presence — piano Spaces notes, offline validation receipt, Telegram notes, poster, soul.md

Result on the fork — run 30723206354, job package self-check, success, all 10 steps passing:

github author_association : N/A
declared association      : champion
builder.association = champion
Validated 55 showcase project manifest(s).
10 checks passed

On association — GitHub's author_association is derived from repository permissions and cannot be set by a contributor. champion is a manifest-level declaration only. The workflow prints GitHub's derived value alongside the declared one in the job summary so the distinction is visible in review rather than implied.

The ask

Both typecheck and the new workflow have been sitting at action_required on this repo since 18:47 UTC — every run is 0s, nothing has executed upstream. Fork PRs from unaffiliated accounts need a maintainer to approve workflow runs.

Could someone approve the runs on this PR? That's the only thing between this and a reviewable green PR — the code side is done and passing. Happy to re-push if you'd prefer the workflow scoped differently or dropped entirely in favour of the existing typecheck job.

Comment thread showcase/acp-sdk-v2-to-v3-migration/showcase.json Outdated
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

This is a really impressive and detailed migration kit, thanks for putting it together! The side-by-side examples and the offline self-check are great touches.

There are a couple of structural things that are pretty far off the standard showcase pattern, which we should probably align on before merging:

  1. Custom CI Workflow: This PR adds a new file to .github/workflows/. Showcase packages are meant to be validated by the repo's existing CI, not to add their own. This isn't a pattern we can scale to all showcase items, as it would make CI maintenance really complex. Could this workflow file be removed?

  2. Embedded Application: The PR also includes a package.json and runnable scripts inside the showcase/ directory. The established pattern is to keep the showcase package as data/docs (manifest, markdown, images) and link to the runnable code in an external repository (like your fork). This keeps the main repo focused on the showcase data itself.

I've also left a small suggestion inline for the tagline. One other copy note: the topics list in the manifest should be trimmed to just the standard values from the contributor guide (e.g., skills, agents). The current list has several extra ones like sdk and typescript.

Let me know what you think!

Suggested edits (couldn't be anchored inline — copy manually)

showcase/acp-sdk-v2-to-v3-migration/showcase.json (around line 4)
Totally optional, but the contributor guide suggests a shorter, verb-led tagline. How about something like this?

  "tagline": "Migrate ACP Node SDK integrations from v2 callbacks to v3 events",

Refreshed after a new push. Any inline suggestions from the first pass may now be stale.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@celesteanglm

celesteanglm commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

First, a housekeeping item: this PR and #95 are the same commit. Both point at head b169bd604e84 on drQedwards:feat/acp-sdk-v2-to-v3-migration. The only difference is the base branch — this PR targets main, #95 targets chore/update-tagline, a stale side branch, which is why #95 renders as 404 files instead of 20.

There is nothing in #95 that isn't already here, so there's nothing to combine — please close #95 and keep this one as the single PR for this work. Two open PRs on one commit also means every push you make here silently updates #95, which makes the review history confusing for both of us. Closing #95 first will keep things clean.

The rest of this review applies to the 20 files in this PR.


The skill itself is in good shape — SKILL.md meets the full standard (when-to-use and when-not, inputs, tools, credentials, preconditions, approval gates, stop conditions, validation, output contract), skills/ is markdown-only, there are exactly three feedback prompts, sourcePath is set, and scripts/validate-showcase.mjs passes. I also scanned all 20 files for secrets and found none.

The manifest, the proof, and the repo-scope additions are what need work.

Blocking

  1. The proof link doesn't match the claim. showcase.json:29,30,57links.video, links.share, and artifacts[0] all resolve to x.com/i/spaces/1dKrPPWnNDzJX, an X Space titled "Distorted Face Piano." It's a live piano performance with no relationship to an SDK migration, and proof/jk-drq-piano-spaces.md concedes as much ("the migration kit itself remains offline-proofed code + skill"). The link returns 200, so validation passes, but the published card's watch button would send visitors to music. Either record a demo of the migration and self-check, or drop links.video/visual.videoLabel and rely on the offline validation receipt.

  2. Every artifact link dies on merge. showcase.json:26,44,46,63,68,73,78,83,88,93,98,103links.repo, skills[0].href, and 10 of 12 artifacts[].href point at github.com/drQedwards/acp-cli-demos/tree/feat/acp-sdk-v2-to-v3-migration/.... That's a fork branch; the links break as soon as it's deleted. Repoint to https://github.com/Virtual-Protocol/acp-cli-demos/blob/main/showcase/acp-sdk-v2-to-v3-migration/.... Your posterUrl already uses the correct form.

  3. .github/workflows/showcase-acp-sdk-v2-to-v3-migration.yml should be deleted. None of the 58 merged showcases add a workflow; the repo has three total, all shared. Steps 1 and 3 duplicate the existing validate-showcase and typecheck jobs, and the push: trigger hardcodes your fork's branch name into upstream CI. This was raised in the automated review and hasn't been addressed. scripts/validate-showcase.mjs already covers it.

  4. showcase.json:23builder.association: "champion" is a self-conferred status. It isn't a field scripts/validate-showcase.mjs knows about and it appears in no merged manifest. Workflow lines 55-63 then run test "${declared}" = "champion", which makes CI assert your own claim about yourself. Please remove both.

  5. proof/telegram-qchaingoldbot.md:11,16 — claims a "Hyperliquid probe long xyz:GOLD" and that "Trading actions still go through ACP CLI + approved signer policy", in a package that ships no trading code and no such proof. links.demot.me/Qchaingoldbot resolves but demonstrates nothing about the migration. Please remove the trading claims or substantiate them.

  6. The v2 → v3 claim can't be verified against this repo. There are zero references to AcpAgent, AssetToken, or session.setBudget anywhere in acp-cli-demos, and the skill hedges its own API surface — evmProvider, // or 'provider' depending on SDK minor version, and "Exact adapter constructor options change across minors." For a document published as a migration guide, please cite the SDK version or release that introduced this surface.

  7. examples/v3-client.mjs:60-64buildClientHandler({ autoComplete: true }) calls session.complete(...) unattended, which is the fund-release step. The "Approval gates" section covers funding and LIVE=1 but never mentions this flag. Document it as approval-gated/demo-only, or remove the option.

  8. package.json + scripts/ inside showcase/ — only 1 of 58 merged packages ships a runnable npm package; the convention is docs/data plus a manifest, with runnable code linked externally. The added workflow exists solely to run npm run check here. Keeping examples/ as illustrative source and moving the harness to your own repo would resolve this and item 3 together.

  9. The PR title says "Qchainer prep agent trader showcase", but the package contains no trader — it's an SDK migration kit. Worth retitling to match. If any live path is intended, the approval gates need an explicit numeric max-spend bound.

Non-blocking

  • showcase.json:8-17 — 8 topics including sdk, typescript, nodejs, x-spaces, telegram. Merged packages average 3-5 and stay closer to the standard buckets.
  • showcase.json:6status: "validated offline demo"; the established value is validated demo.
  • showcase.json:4 — the tagline runs three clauses and names six API symbols. The shorter verb-led version from the automated review reads better.
  • examples/v3-client.mjs:36-42assertExactFundAmount coerces with Number() and compares with !==, i.e. float equality on USDC amounts. Compare base units or strings instead.
  • assets/poster.svg (26 KB) is committed but unreferenced; only poster.png is used.

Please also rebase onto main before this merges — the branch is currently behind, and a clean diff keeps the post-merge docs sync predictable.

To recap the sequencing: close #95, rebase this branch onto main, then address the items above. If you believe there's work in #95 that isn't here, say so before closing and I'll take another look.

@celesteanglm celesteanglm mentioned this pull request Aug 16, 2026
18 tasks
@drQedwards drQedwards closed this Aug 22, 2026
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