Extend daemon start help with config and environment guidance (3.2.2) - #131
Conversation
Execplan document for daemon start help guidanceThis PR introduces a new pre-implementation execplan document at Key contentThe execplan formalises the strategy for extending
Structured guidanceThe document provides:
Impact
WalkthroughA new ExecPlan document renumbers archived 3.2.2 daemon-start help guidance as live roadmap task 13.3.3. It establishes roadmap-fit requirements, implementation constraints, risk register, and a two-stage delivery plan using generated metadata (Stage A) and optional temporary-adapter exception (Stage B), with validation commands and recovery procedures. ChangesExecPlan: Daemon-start Help Guidance
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 18✅ Passed checks (18 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Add the pre-implementation execplan for roadmap task 3.2.2, covering the extension of `weaver daemon start --help` with documentation of the `WEAVERD_BIN` and `WEAVER_FOREGROUND` environment variables and at least one startup example. The plan records the placement decision (per-variant `long_about` rather than `after_help`), the test strategy (additive BDD scenarios reusing the existing steps plus `#[rstest]` unit cases for the env-var-set edge case), and the documentation impact on `users-guide.md` and `developers-guide.md`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1ef3819 to
101f5b0
Compare
Renumber the daemon-start help ExecPlan against live roadmap task `13.3.3` while preserving archive `3.2.2` as provenance. Record the roadmap-fit analysis: keep the environment-variable help requirement, but do not build the old standalone clap `long_about` patch unless a temporary-adapter exception is explicitly approved.
Reviewer's GuideAdds a new execplan document that renumbers and reframes the previous 3.2.2 daemon-start help plan into live roadmap task 13.3.3, clarifying that the original clap-only help patch should not be implemented as a standalone change and that the requirement should instead be satisfied via generated command metadata under phase 13. Flow diagram for choosing generated metadata vs temporary adapterflowchart TD
Start["Need to expose WEAVERD_BIN and WEAVER_FOREGROUND in daemon help"]
Check13_3_3["Is live roadmap task 13.3.3 (generated metadata) underway?"]
UseGenerated["Implement via generated CommandMetadata (preferred)"]
NeedTemp["Is there an immediate release/support need?"]
TempAdapter["Add temporary DaemonAction_Start help patch<br/>(label as adapter under 13.1.3)"]
Defer["Defer change until 13.3.3"]
Start --> Check13_3_3
Check13_3_3 -->|Yes| UseGenerated
Check13_3_3 -->|No| NeedTemp
NeedTemp -->|Yes| TempAdapter
NeedTemp -->|No| Defer
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The file is positioned as the ExecPlan for live task 13.3.3 while also being an archive alignment document; consider explicitly stating whether a separate implementation-focused 13.3.3 ExecPlan will be created later so that future maintainers are not confused about where the authoritative build plan for 13.3.3 should live.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The file is positioned as the ExecPlan for live task 13.3.3 while also being an archive alignment document; consider explicitly stating whether a separate implementation-focused 13.3.3 ExecPlan will be created later so that future maintainers are not confused about where the authoritative build plan for 13.3.3 should live.
## Individual Comments
### Comment 1
<location path="docs/execplans/13-3-3-archive-3-2-2-daemon-start-help-guidance.md" line_range="86" />
<code_context>
+`weaver daemon start --help` cannot discover `WEAVERD_BIN`, which overrides the
+spawned daemon binary, or `WEAVER_FOREGROUND`, which keeps the daemon attached
+to the controlling terminal. That information matters during installation
+failures, debugging, and CI jobs.
+
+The problem is still real, but the proposed mechanism is misaligned with the
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “CI” is used without being defined on first use, which violates the acronym-definition instruction.
Please expand “CI” on first use, for example “continuous integration (CI) jobs”, to comply with the requirement to define uncommon acronyms on first use.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Define uncommon acronyms on first use.
</details>
</issue_to_address>
### Comment 2
<location path="docs/execplans/13-3-3-archive-3-2-2-daemon-start-help-guidance.md" line_range="218" />
<code_context>
+
+## Context and orientation
+
+The original plan targeted the prototype CLI path. It proposed adding a
+`#[command(long_about = ...)]` attribute to `DaemonAction::Start` in
+`crates/weaver-cli/src/cli.rs`, then updating unit tests, behavioural tests,
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “CLI” is introduced without being defined on first use, which conflicts with the acronym-definition requirement.
Please expand “CLI” the first time it appears, for example “command-line interface (CLI) path”, so that the acronym is defined on first use.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Define uncommon acronyms on first use.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| `weaver daemon start --help` cannot discover `WEAVERD_BIN`, which overrides the | ||
| spawned daemon binary, or `WEAVER_FOREGROUND`, which keeps the daemon attached | ||
| to the controlling terminal. That information matters during installation | ||
| failures, debugging, and CI jobs. |
There was a problem hiding this comment.
suggestion (review_instructions): The acronym “CI” is used without being defined on first use, which violates the acronym-definition instruction.
Please expand “CI” on first use, for example “continuous integration (CI) jobs”, to comply with the requirement to define uncommon acronyms on first use.
Review instructions:
Path patterns: **/*.md
Instructions:
Define uncommon acronyms on first use.
|
|
||
| ## Context and orientation | ||
|
|
||
| The original plan targeted the prototype CLI path. It proposed adding a |
There was a problem hiding this comment.
suggestion (review_instructions): The acronym “CLI” is introduced without being defined on first use, which conflicts with the acronym-definition requirement.
Please expand “CLI” the first time it appears, for example “command-line interface (CLI) path”, so that the acronym is defined on first use.
Review instructions:
Path patterns: **/*.md
Instructions:
Define uncommon acronyms on first use.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/execplans/13-3-3-archive-3-2-2-daemon-start-help-guidance.md (1)
318-318:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd trailing newline at end of file.
The file should end with a newline character after the final line of content (line 317). Most text editors and POSIX standards expect files to end with a newline.
[type:syntax/md]
Triage:
[type:syntax/md]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/execplans/13-3-3-archive-3-2-2-daemon-start-help-guidance.md` at line 318, Add a single trailing newline character at the very end of the markdown file so the file ends with a newline (ensure the final line is terminated with '\n'); modify the document's final line to include this trailing newline and save the file to satisfy POSIX/editor expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/execplans/13-3-3-archive-3-2-2-daemon-start-help-guidance.md`:
- Line 318: Add a single trailing newline character at the very end of the
markdown file so the file ends with a newline (ensure the final line is
terminated with '\n'); modify the document's final line to include this trailing
newline and save the file to satisfy POSIX/editor expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 328de488-4d50-4af8-99fa-37b5b454fa16
📒 Files selected for processing (1)
docs/execplans/13-3-3-archive-3-2-2-daemon-start-help-guidance.md
Summary
This branch carries the pre-implementation execution plan for roadmap task
3.2.2, which extends
weaver daemon start --helpwith documentation of theWEAVERD_BINandWEAVER_FOREGROUNDenvironment variables and at least onestartup example using one of those overrides. The plan authorises the
forthcoming implementation; no source, test, or runtime behaviour changes
have landed in this branch yet.
Roadmap task: (3.2.2) — see docs/roadmap.md and the Level 8 entry in docs/ui-gap-analysis.md.
Execplan: docs/execplans/3-2-2-extend-daemon-start-help-with-config-guidance.md. Status in the file is
DRAFT; approval on this draft pull request is the gate that authorises implementation.Review walkthrough
long_aboutoverafter_help, theconcat!()idiom, honestWEAVER_FOREGROUNDsemantics, and unit-test rather than BDD coverage of env-var-set edge cases.WEAVERD_BINand crates/weaverd/src/process/launch.rs forWEAVER_FOREGROUND.Validation
This pull request changes only the execplan document. The relevant gateway
results so far:
make markdownlint: 0 errors across 83 files after a line-length fix.The full implementation gateways (
make check-fmt,make lint,make test,make fmt,make markdownlint,make nixie) will run on the implementationpull request that follows approval of this plan.
Notes
(introduced in 2.3.3) untouched and only cross-references it for
consistency.
steps; both have already been performed for this pull request (the branch
was renamed before any pull request existed, so a local rename plus push
was used in line with the system instructions).
plan is approved.
Summary by Sourcery
Document roadmap alignment for daemon-start help guidance by renumbering and revising the execution plan around the live phase-13 command metadata framework.
Documentation:
Chores: