docs: add configuration system documentation#1207
Conversation
geoffjay
left a comment
There was a problem hiding this comment.
Review: docs: add configuration system documentation
Stack Position
This PR (issue-1200) is stacked on issue-1198 (#1206, needs-rework), which sits at the top of a blocked chain:
issue-1200 (this PR)
└── issue-1198 (#1206) [needs-rework]
└── issue-1197 (#1205) [needs-rework]
└── issue-1196 (#1204) [needs-rework]
└── issue-1195 (#1203) [needs-rework]
└── issue-1199 (#1202)
└── issue-1194 (#1201) [needs-rework]
This PR cannot merge until the chain below it is resolved. Comments below evaluate the content in isolation.
Accuracy Check
Before reviewing content, I verified the key reference entries against the actual code on the issue-1197 branch:
AGENTD_ASK_ORCHESTRATOR_URL- confirmed inapply_env_overrides()✓AGENTD_WRAP_BACKEND- confirmed ✓AGENTD_ORCHESTRATOR_COMMUNICATE_URL- confirmed ✓agent config show --json---jsonis declared#[arg(long, global = true)]on the rootClistruct, so it can appear anywhere in the command line. Documented invocation is correct ✓- The 11 missing env var entries added to
crates/common/src/config.rsexactly match the gap identified in the #1201 review ✓
Non-Blocking Issues
1. Migration guide is missing a renamed-variables callout
The migration guide (Step 3) instructs users to "find the corresponding TOML key in the Complete Key Reference table above". However, three commonly-used env var names were renamed when the shared config was introduced and do not appear in the key reference:
| Old (service-local) | New (shared config) |
|---|---|
AGENTD_ORCHESTRATOR_URL (ask) |
AGENTD_ASK_ORCHESTRATOR_URL |
AGENTD_BACKEND (wrap + orchestrator) |
AGENTD_WRAP_BACKEND / AGENTD_ORCHESTRATOR_BACKEND |
AGENTD_COMMUNICATE_SERVICE_URL (orchestrator) |
AGENTD_ORCHESTRATOR_COMMUNICATE_URL |
A user who has AGENTD_BACKEND=docker in their shell profile will follow the guide, fail to find it in the table, and be stuck. Add a brief "Renamed Variables" callout in the migration guide — even a short table like the one above is enough.
Note: the old names still work via the service-local config path (they are not being removed), but they won't appear in agent config show output or in the TOML file generated by agent config init. The guide should make that clear.
2. AGENTD_HISTORY_SIZE env var collision is undocumented
The reference table correctly maps AGENTD_HISTORY_SIZE to services.hook.history_size (default 500). However, the service-local MonitorConfig also reads AGENTD_HISTORY_SIZE for its own history_size field (metric snapshot ring-buffer, default 120). A user who sets AGENTD_HISTORY_SIZE=120 intending to configure monitor's ring buffer will inadvertently shrink hook's event history from 500 to 120 as well.
This collision is a tracked blocking issue in #1201, so the shared config table is intentionally showing only the hook mapping for now. But a one-line note in the hook row would prevent surprises:
Warning: The service-local monitor config also reads this env var for its metric snapshot ring-buffer (see #1201).
Omit the note if the collision is resolved before this PR merges.
What's Working Well
crates/common/src/config.rsfixes are surgically correct — exactly the 11 entries identified as missing in the #1201 review, placed in logical groupings within the existing table.- Quick Start section is concise and leads with the most common path (
config initthenconfig show). - Precedence section uses a concrete worked example (notify port at 17004 / 19004 / 29004) that makes the three-layer model immediately tangible.
- Four example configs cover the realistic range of deployments well. The Docker example correctly uses
host = "0.0.0.0"andbackend = "subprocess"— exactly what trips people up in containers. - LanceDB path section with platform-specific paths (Linux XDG vs macOS Library) is a genuinely useful detail that would otherwise require reading source code.
- Validation section accurately reflects the error output format from
AgentdConfig::validate()including the collect-all-errors behaviour. - README link is placed logically between Getting Started and Features — easy to find, doesn't clutter the page.
Summary
Two non-blocking suggestions:
- Add a renamed-variables callout to the migration guide for
AGENTD_BACKEND,AGENTD_ORCHESTRATOR_URL, andAGENTD_COMMUNICATE_SERVICE_URL - Note the
AGENTD_HISTORY_SIZEcollision in the hook row (or omit it once #1201 resolves the collision)
The content is accurate, well-structured, and covers the right use cases. No blocking issues.
- Create docs/configuration.md with quick start, file location, precedence rules, complete key reference table, per-service sections, migration guide, and four example configs (minimal, development, production, Docker) - Fix the env var table in agentd-common/config.rs to include all overrides (AGENTD_INDEX_LANGUAGES, AGENTD_RECONCILE_INTERVAL_SECS, all MCP URLs, AGENTD_NOTIFY_SERVICE_URL, AGENTD_COLLECTION_INTERVAL_SECS) - Add link to configuration reference from README.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
This change is part of the following stack:
Change managed by git-spice. |
Add comprehensive documentation for the agentd TOML configuration system, covering quick start, file location, three-layer precedence rules, a complete key reference table for all 40+ settings, per-service sections, four example configs (minimal, development, production, Docker), and a migration guide from env-var-only setups.
Also fixes the environment variable table in
crates/common/src/config.rswhich was missing entries forAGENTD_INDEX_LANGUAGES,AGENTD_RECONCILE_INTERVAL_SECS,AGENTD_NOTIFY_SERVICE_URL,AGENTD_COLLECTION_INTERVAL_SECS, and allAGENTD_MCP_*_URLvariables.Closes #1200