Add AD4M executor developer skill (build, MCP, bootstrap)#713
Add AD4M executor developer skill (build, MCP, bootstrap)#713HexaField wants to merge 5 commits intocoasys:devfrom
Conversation
Covers build from source, bootstrap seed selection, MCP setup, TLS configuration, port reference, and common issues. Addresses undocumented friction points in developer onboarding.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a new developer guide at Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
data-bot-coasys
left a comment
There was a problem hiding this comment.
Excellent work! This is exactly what's been missing. The bootstrap seed section alone will save hours of debugging for new developers.
A few small notes:
- Ubuntu packages: use
libayatana-appindicator3-devinstead oflibappindicator3-dev— modern Ubuntu (22.04+) ships the ayatana fork and the old package conflicts. rust-client/schema.gqlis a broken symlink (target deleted in 2023). Worth adding:cp tests/js/schema.gql rust-client/schema.gqlto the build steps, or noting it as a known issue.- Rust version: CI uses 1.92, the skill says 1.84.0+ — might want to bump to match.
- Node version: CI uses Node 18, skill says 20+ — worth aligning.
None of these block merging. The content is thorough, accurate, and well-structured. 👏
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
skills/ad4m-executor/SKILL.md (1)
242-242: Prefer a safer stop command than unconditionalkill -9.Using SIGKILL as the default troubleshooting step is aggressive. Prefer graceful termination first and escalate only if needed.
Suggested doc tweak
-| Port bind error | Previous executor still running | `lsof -ti:12100 \| xargs kill -9` | +| Port bind error | Previous executor still running | `lsof -ti:12100 \| xargs -r kill` (use `kill -9` only if process does not exit) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/ad4m-executor/SKILL.md` at line 242, Replace the unconditional SIGKILL troubleshooting command in the "Port bind error" table entry (currently shown as `lsof -ti:12100 \| xargs kill -9`) with a safer, stepwise termination sequence: first attempt a graceful kill (e.g., `lsof -ti:12100 | xargs -r kill`), wait/verify the process is gone, then fall back to `kill -9` only if necessary; update the table cell in SKILL.md accordingly so the command shows the safer escalation instead of always using `kill -9`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/ad4m-executor/SKILL.md`:
- Around line 86-90: Add language identifiers to the two fenced code blocks: the
block containing the error lines starting with "Did not find language source for
given address: QmzSYwd..." should be changed from ``` to ```text, and the ASCII
tree block beginning with "ad4m/" should be changed from ``` to ```text (same
fix also for the second occurrence at lines 222-233). Update those fenced code
blocks in SKILL.md so markdownlint MD040 is satisfied and rendering/tooling
improves.
- Around line 61-64: Update the rebuild matrix so the "Deno/JS changes" path
also rebuilds the executor JS before rust-executor: when JS changes happen under
executor/, run `pnpm build` in executor/ then in rust-executor/ and finally in
cli/ as needed (mirroring the Rust executor change path); adjust the Deno/JS
changes bullet to list "executor/ and rust-executor" rebuild steps to ensure
snapshot/runtime inputs are refreshed.
---
Nitpick comments:
In `@skills/ad4m-executor/SKILL.md`:
- Line 242: Replace the unconditional SIGKILL troubleshooting command in the
"Port bind error" table entry (currently shown as `lsof -ti:12100 \| xargs kill
-9`) with a safer, stepwise termination sequence: first attempt a graceful kill
(e.g., `lsof -ti:12100 | xargs -r kill`), wait/verify the process is gone, then
fall back to `kill -9` only if necessary; update the table cell in SKILL.md
accordingly so the command shows the safer escalation instead of always using
`kill -9`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d8e13f3-f503-4d28-9594-4401e9bc7cac
📒 Files selected for processing (1)
skills/ad4m-executor/SKILL.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/ad4m-executor/SKILL.md`:
- Around line 237-261: The curl examples in the MCP auth flow (the four example
requests: initialize, notifications/initialized, tools/call for
request_capability, and tools/call for generate_jwt) are malformed because they
use "curl ... POST ..." which treats POST as a URL token; change each to use
either "-X POST" (e.g., "curl -s -X POST ...") or remove the stray "POST" and
rely on "-d" to set the method, so the examples become valid and executable;
update all occurrences of the faulty "curl ... POST" invocations in the examples
for initialize, notifications/initialized, request_capability, and generate_jwt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3779c6b4-216c-4ddb-b908-73dd84e76851
📒 Files selected for processing (1)
skills/ad4m-executor/SKILL.md
skills/ad4m-executor/SKILL.md
Outdated
| ## Prerequisites | ||
|
|
||
| ### Required toolchain | ||
| - **Rust 1.84.0+** with `wasm32-unknown-unknown` target |
There was a problem hiding this comment.
.circleci/Dockerfile and rust-toolchain.toml are normative
There was a problem hiding this comment.
Fixed in fb2aab2 — bumped to Rust 1.92+ and now references rust-toolchain.toml and .circleci/Dockerfile as normative sources. ⬡
There was a problem hiding this comment.
Noted — the skill now points to these files as the source of truth for toolchain version. ⬡
…d matrix, code block langs, curl -X POST, safer kill, broken symlink note
HexaField
left a comment
There was a problem hiding this comment.
Thanks Data — all addressed in fb2aab2:
- ✅
libayatana-appindicator3-devreplaceslibappindicator3-dev - ✅
rust-client/schema.gqlbroken symlink added to Common Issues table - ✅ Rust version bumped to 1.92+ (references
rust-toolchain.tomland.circleci/Dockerfileas normative) - ✅ Node version corrected to 18+
⬡
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
skills/ad4m-executor/SKILL.md (1)
145-147:⚠️ Potential issue | 🟡 MinorAdd language tags to fenced code blocks.
These two fenced blocks still lack language identifiers and trigger MD040.
Suggested fix
-``` +```text Failed to generate token: main key not found. call createMainKey() first-
+text
https://deploy-preview-548--fluxsocial-dev.netlify.app/Also applies to: 310-312
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/ad4m-executor/SKILL.md` around lines 145 - 147, Add explicit language identifiers to the two fenced code blocks that currently have no language tag (the block containing "Failed to generate token: main key not found. call createMainKey() first" and the block with "https://deploy-preview-548--fluxsocial-dev.netlify.app/") so they validate against MD040; change their opening fences from ``` to ```text (or another appropriate language like ```bash or ```ansi) to silence the lint rule and make intent explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/ad4m-executor/SKILL.md`:
- Line 421: The table entry for "Port bind error" contains an escaped pipe in
the kill command example (`lsof -ti:12100 \| xargs kill`); edit SKILL.md to
remove the backslash so the command uses a literal pipe between lsof and xargs
(preserve the rest of the text including the note about using `kill -9` only if
needed) so the example copies and executes correctly.
---
Duplicate comments:
In `@skills/ad4m-executor/SKILL.md`:
- Around line 145-147: Add explicit language identifiers to the two fenced code
blocks that currently have no language tag (the block containing "Failed to
generate token: main key not found. call createMainKey() first" and the block
with "https://deploy-preview-548--fluxsocial-dev.netlify.app/") so they validate
against MD040; change their opening fences from ``` to ```text (or another
appropriate language like ```bash or ```ansi) to silence the lint rule and make
intent explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86f54e68-1edc-466c-9eb3-a8bb6213ee17
📒 Files selected for processing (1)
skills/ad4m-executor/SKILL.md
skills/ad4m-executor/SKILL.md
Outdated
|
|
||
| ### ⚠️ Binary collision | ||
|
|
||
| Both crates produce a binary named `ad4m-executor` in `target/release/`. The CLI crate's version has all runtime flags (MCP, TLS, multi-user). **Always build CLI last**, or target it explicitly: |
There was a problem hiding this comment.
No, only CLI creates binaries. rust-executor is a library used in cli and ui
There was a problem hiding this comment.
You're right — corrected in 012442b. Replaced the "binary collision" section with a note that only CLI produces binaries, and rust-executor is a library (its build generates the Deno snapshot needed by CLI). ⬡
skills/ad4m-executor/SKILL.md
Outdated
| **Always use the mainnet seed for development:** | ||
| ```bash | ||
| ad4m-executor init --data-path /tmp/ad4m-data \ | ||
| --network-bootstrap-seed ./cli/mainnet_seed.json |
There was a problem hiding this comment.
the bootstrap seed gets embedded in into the binary. if you want to use the mainnet seed, you don't have to specify this parameter when running the binary
There was a problem hiding this comment.
Fixed in 012442b — rewrote the bootstrap section: mainnet seed is embedded in the binary, ad4m-executor init writes it to the data dir as mainnet_seed.seed. No --network-bootstrap-seed flag needed for standard dev. ⬡
skills/ad4m-executor/SKILL.md
Outdated
| --network-bootstrap-seed ./cli/mainnet_seed.json | ||
| ``` | ||
|
|
||
| Without `--network-bootstrap-seed`, the executor looks for `mainnet_seed.seed` in the data path — a file that doesn't exist after init. This fails silently: the executor starts, agent generates, but all languages fail to resolve. |
There was a problem hiding this comment.
ah, yeah right. ad4m-executor init creates ~/.ad4m and writes the included mainnet seet.
There was a problem hiding this comment.
Updated — the skill now documents the init flow correctly: ad4m-executor init creates the data directory and writes the embedded mainnet seed. ⬡
lucksus
left a comment
There was a problem hiding this comment.
just a few more change requests @HexaField
…inaries), bootstrap seed is embedded (no --network-bootstrap-seed needed), fix escaped pipe in troubleshooting
There was a problem hiding this comment.
♻️ Duplicate comments (1)
skills/ad4m-executor/SKILL.md (1)
144-146:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced blocks (MD040).
Line 144 and Line 309 start fenced code blocks without a language, which can trigger markdownlint warnings. Please use
text(or an appropriate language) for both blocks.Also applies to: 309-311
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/ad4m-executor/SKILL.md` around lines 144 - 146, Two fenced code blocks in SKILL.md are missing language identifiers; add a language (e.g., text) to the opening triple-backtick fences so markdownlint MD040 is satisfied. Specifically, update the fence that wraps the message "Failed to generate token: main key not found. call createMainKey() first" and the other example block later in the document (the fenced block around the example at ~309-311) by changing ``` to ```text for each. Ensure both opening fences include the language identifier and leave the closing fences untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@skills/ad4m-executor/SKILL.md`:
- Around line 144-146: Two fenced code blocks in SKILL.md are missing language
identifiers; add a language (e.g., text) to the opening triple-backtick fences
so markdownlint MD040 is satisfied. Specifically, update the fence that wraps
the message "Failed to generate token: main key not found. call createMainKey()
first" and the other example block later in the document (the fenced block
around the example at ~309-311) by changing ``` to ```text for each. Ensure both
opening fences include the language identifier and leave the closing fences
untouched.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 097a3c03-a997-4868-b1b2-133f54090054
📒 Files selected for processing (1)
skills/ad4m-executor/SKILL.md
…larification - Add language identifiers to fenced code blocks (MD040) - Add wallet unlock after restart section (critical for MCP/JWT) - Add --data-path vs --app-data-path clarification - Add trusted agents section - Add wallet-locked to common issues table
AD4M Executor Developer Skill
Adds an AI-agent-compatible skill file (
skills/ad4m-executor/SKILL.md) that documents everything a developer needs to build and run the AD4M executor from a fresh clone.What it covers
rust-executorthencli), the binary collision gotchacli/mainnet_seed.jsonworks andtests/js/bootstrapSeed.jsondoesn't for standalone useWhy a skill file?
Skill files (SKILL.md) are structured documentation designed to be loaded by AI coding agents (Codex, Claude, etc.) as on-demand context. They follow a progressive disclosure pattern — metadata triggers loading, the body provides actionable instructions. This means any developer using an AI assistant gets the right guidance automatically when working with the executor.
The skill is equally readable by humans as a quickstart reference.
Context
This addresses multiple undocumented friction points encountered during fresh-clone setup on Ubuntu 24, including the bootstrap seed confusion, MCP authentication flow, and the dual binary issue.
⬡
Summary by CodeRabbit