docs(config): fix claude examples to use claude-agent-acp#829
docs(config): fix claude examples to use claude-agent-acp#829feiyun968-agent wants to merge 2 commits into
Conversation
Both config.toml.example and docs/config-reference.md used
`command = "claude"` with `args = ["--acp"]`, but the Claude Code
CLI has no --acp flag. ACP support is provided by the separate
@agentclientprotocol/claude-agent-acp npm package.
Update both files to match docs/claude-code.md:
- command = "claude-agent-acp"
- args = []
- env = { CLAUDE_CODE_OAUTH_TOKEN } (OAuth-first, per project recommendation)
Fixes openabdev#632
OpenAB PR ScreeningThis is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Screening reportscreening pass complete.GitHub comment: #829 (comment) IntentPR #829 fixes stale Claude configuration examples that tell users to run FeatDocs/config fix. It updates Claude examples to use Who It ServesDeployers and maintainers. Rewritten PromptUpdate OpenAB Claude configuration examples so every documented Claude ACP backend uses Merge PitchLow-risk docs-only fix for a real setup trap. Main reviewer concern: confirm Best-Practice ComparisonOpenClaw and Hermes Agent do not materially apply. This is documentation consistency, not runtime scheduling, persistence, delivery, retry, or execution isolation. Implementation Options
Comparison Table
RecommendationMerge the conservative docs-only fix after verifying the snippets match the current Claude source-of-truth docs. Split CI guard or generated-snippet work into follow-up. |
This comment has been minimized.
This comment has been minimized.
chaodu-agent
left a comment
There was a problem hiding this comment.
LGTM. This aligns the config examples with the Claude ACP adapter flow and keeps the generic env example neutral.
This comment has been minimized.
This comment has been minimized.
|
CHANGES REQUESTED What This PR DoesFixes stale Claude config examples that referenced the non-existent How It WorksUpdates Findings
Finding Details🟢 F1: Command and args fix is correct
🟡 F2: OAuth token should not be in env var examplePer
The documented approach is Suggested fix: Remove the Claude-specific env var line entirely, or use 🟡 F3: Comment "or set CLAUDE_CODE_OAUTH_TOKEN" is misleadingThe added comment Suggested fix: Baseline Check
What's Good (🟢)
Review by 超渡法師 🙏 on behalf of the 法師團隊 |
Dismissing approval — env var approach contradicts docs/claude-code.md guidance. See comment for details.
howie
left a comment
There was a problem hiding this comment.
Review Summary
Verified this fix against all source-of-truth files. The changes are accurate and consistent.
Verified Cross-Consistency
Checked docs/claude-code.md, docs/multi-agent.md, charts/openab/values.yaml, and README.md — all already use claude-agent-acp. No stray claude --acp remains anywhere in the repo after this PR.
npm Package Name
The added install comment in config.toml.example uses @agentclientprotocol/claude-agent-acp, which matches the exact package name pinned in the Dockerfile (@agentclientprotocol/claude-agent-acp@0.25.0). Correct.
Minor Observations (non-blocking)
1. Version pin in install comment
The added comment says:
# # npm install -g @agentclientprotocol/claude-agent-acpThe Dockerfile pins @0.25.0. For a user-facing config example, using unpinned (latest) is fine, but worth a mention in case you want consistency.
2. env table example in docs/config-reference.md
The generic env field description changed from ANTHROPIC_API_KEY to OPENAI_API_KEY as a neutral example. Reasonable choice. An even more provider-agnostic placeholder like MY_AGENT_API_KEY = "${MY_AGENT_API_KEY}" might be cleaner, but OPENAI_API_KEY works since Codex is one of the supported backends.
3. command field description still lists claude as an example binary
| `command` | string | *required* | Agent binary (e.g. `kiro-cli`, `claude`, `codex`, ...). |claude is still a valid binary — it's just not the right one for ACP (claude-agent-acp is). Consider updating this list to use claude-agent-acp instead of claude to avoid confusing users who may try command = "claude" directly.
Overall this is a clean, well-scoped fix. The root cause analysis is correct, the PR description clearly documents the source-of-truth files, and the changes have no behaviour impact.
canyugs
left a comment
There was a problem hiding this comment.
🔍 Panel Review Synthesis — PR #829
Verdict: ✅ APPROVE with suggested changes
The core fix is correct and unambiguous — command = "claude-agent-acp" with args = [] matches all existing source-of-truth docs. The PR solves the user-facing breakage reported in #632. The suggested changes below are all low-cost improvements that would make the PR internally consistent.
Findings
| # | Severity | Finding | Convergence |
|---|---|---|---|
| 1 | 🟡 Major | Install hint only installs @agentclientprotocol/claude-agent-acp but auth hint requires claude binary from @anthropic-ai/claude-code. Should be: npm install -g @anthropic-ai/claude-code @agentclientprotocol/claude-agent-acp |
— |
| 2 | 🟡 Major | CLAUDE_CODE_OAUTH_TOKEN env example contradicts the file's own security warning and docs/claude-code.md's canonical container auth flow (claude auth login with PVC persistence). The comment flattens the container-vs-CI distinction. |
2/3 convergence |
| 3 | 🟡 Minor | docs/config-reference.md:89 field-spec table still lists bare claude as an example binary — internally inconsistent with this PR's purpose. |
2/3 convergence |
Suggested Changes
- Fix install hint (
config.toml.example):
npm install -g @anthropic-ai/claude-code @agentclientprotocol/claude-agent-acp-
Align auth guidance — either remove the
CLAUDE_CODE_OAUTH_TOKENenv var example and point toclaude auth login(matchingdocs/claude-code.md), or add a comment clarifying this is the CI/scripts path. -
Update field-spec table (
docs/config-reference.md:89): Replaceclaudewithclaude-agent-acpin the example list.
Panel: co-蔻迪克斯 ✅ · cl-克勞德 ✅ · ct-柯派羅 ✅ · g-居門奈 ⏳
0. Discord Discussion URL
https://discord.com/channels/1492804973032112279/1504271921532108911
1. What problem does this solve?
Fixes #632.
config.toml.exampleanddocs/config-reference.mdboth usedcommand = "claude"withargs = ["--acp"], but the Claude Code CLI has no--acpflag. Users who follow these examples get an immediate failure:While investigating #632,
docs/config-reference.mdwas found to have the same error and is fixed in the same PR.2. At a Glance
Source of truth:
docs/claude-code.md,docs/multi-agent.md,charts/openab/values.yaml, andREADME.mdall already useclaude-agent-acp.3. Prior Art & Industry Research
Not applicable — trivial docs fix, no architectural change.
4. Proposed Solution
Sync both files with
docs/claude-code.md.5. Why This Approach
Direct sync with existing correct documentation. No behaviour change.
6. Alternatives Considered
None — the fix is unambiguous.
7. Validation
Docs-only change. Verified
docs/claude-code.md(line 31),docs/multi-agent.md,charts/openab/values.yaml, andREADME.mdall useclaude-agent-acpas source of truth.