feat(cli): add man page generation - #622
Conversation
…ion, testing, and evaluation - Introduced spec-requirements.md for creating and refining requirements documents using EARS format. - Added spec-design.md for generating and updating design documents based on approved requirements. - Created spec-tasks.md to develop actionable implementation plans and task lists post-design approval. - Implemented spec-impl.md for executing coding tasks according to the defined task list. - Developed spec-test.md for generating comprehensive test documentation and executable test code. - Added spec-judge.md for evaluating multiple versions of spec documents and selecting the best solution. - Implemented spec-system-prompt-loader.md to load the spec workflow system prompt. - Updated kfc-settings.json to define paths for specs and visibility settings. - Created spec-workflow-starter.md to outline the entire spec workflow process and its components.
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesKFC spec workflow
CLI enhancements
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant StatusCommand
participant Repository
participant StatusService
Operator->>StatusCommand: status --tag tag
StatusCommand->>Repository: getContractsByTag(tag)
Repository-->>StatusCommand: matching contracts
StatusCommand->>StatusService: getContractStatus(contractId)
StatusService-->>StatusCommand: status data
StatusCommand-->>Operator: summaries or JSON
sequenceDiagram
participant Build
participant CLIProgram
participant Generator
participant Renderer
Build->>Generator: run build:man
Generator->>CLIProgram: read command metadata
CLIProgram-->>Generator: command descriptions
Generator->>Renderer: validate generated man page
Renderer-->>Build: render result
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 Warning |
|
@temi-Dee Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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.
Inline comments:
In @.claude/agents/kfc/spec-design.md:
- Around line 125-134: Update the design-document requirements in the Important
Constraints section to use only the canonical
`.claude/specs/{feature_name}/design.md` path. Remove the conflicting
`.kiro/specs/{feature_name}/design.md` requirement while preserving the existing
research and design-document expectations.
In @.claude/agents/kfc/spec-judge.md:
- Around line 101-110: The document-judge workflow must safely finalize and
clean up candidates. Update the cleanup and final-copy steps in the judge
instructions to canonicalize and validate every candidate path as being within
spec_base_path, create the final document atomically, and delete reviewed inputs
only after successful finalization; preserve inputs on copy or validation
failure and report the resulting selection summary.
- Around line 80-96: Update evaluate_documents to treat each documents entry as
a path, deriving doc_id from the path and loading or evaluating the
corresponding document without accessing doc.id. Compute each score’s total from
the numeric completeness, clarity, feasibility, and innovation criteria instead
of summing the scores list, while preserving the existing strengths, weaknesses,
accumulation, and selection flow.
In @.claude/agents/kfc/spec-requirements.md:
- Around line 47-50: Update the output filename constraint near the
file-creation requirements to use the same conditional naming rule: produce
requirements{output_suffix}.md when output_suffix is provided, otherwise
requirements.md. Remove any unconditional underscore before the suffix so
requirements_.md is never generated.
In @.claude/agents/kfc/spec-test.md:
- Around line 11-18: Update the input contract in the test specification to
include explicit module and output-path fields for the required {module}.md and
{module}.test.ts files. Apply the same contract consistently to the later input
section, or define a deterministic derivation from task_id and spec_base_path
that fixes both filenames and locations.
In `@scripts/generate-man.ts`:
- Around line 54-61: Require an actual troff renderer in the generate-man
validation instead of treating marker checks as successful rendering: update the
no-renderer branch in scripts/generate-man.ts around the renderer availability
logic to return an unavailable/error result and fail explicitly. In
tests/cli/man.test.ts lines 39-47, require a renderer for the assertion and
remove the structural-marker fallback as syntax validation.
In `@src/cli/program.ts`:
- Around line 31-48: Align the CLI, generated man page, and coverage for the
supported unwatch command: restore its registration in createProgram in
src/cli/program.ts (lines 31-48), regenerate the corresponding UNWATCH section
in man/sorokeep.1 (lines 11-12), and add unwatch to the command contract in
tests/cli/man.test.ts (lines 12-31).
In `@src/commands/status.ts`:
- Around line 46-59: The registerStatusCommand action must explicitly handle the
case where both contractId and options.tag are provided instead of silently
prioritizing the contract ID. Reject or warn about this mutually exclusive
combination before entering either status lookup branch, while preserving the
existing behavior when only one selector is supplied.
- Around line 74-97: The tag-status mapping around getContractStatus must handle
failures per contract instead of propagating them to the shared outer catch.
Update the statuses flow to retain each contract’s contractId, report errors
using the failing contract’s id in both JSON and non-JSON output, and continue
processing remaining contracts without discarding successfully collected
statuses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e1b5c402-5384-44d1-a56c-ce38aeab1cf6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
.claude/agents/kfc/spec-design.md.claude/agents/kfc/spec-impl.md.claude/agents/kfc/spec-judge.md.claude/agents/kfc/spec-requirements.md.claude/agents/kfc/spec-system-prompt-loader.md.claude/agents/kfc/spec-tasks.md.claude/agents/kfc/spec-test.md.claude/settings/kfc-settings.json.claude/system-prompts/spec-workflow-starter.mdREADME.mdman/sorokeep.1package.jsonscripts/generate-man.tssrc/cli/program.tssrc/commands/status.tssrc/db/repositories.tssrc/index.tstests/cli/man.test.tstests/db/repositories.test.tstsconfig.test.jsonvitest.config.ts
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.45.0)
tests/cli/man.test.ts
[warning] 3-3: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
scripts/generate-man.ts
[warning] 2-2: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFileSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 LanguageTool
.claude/system-prompts/spec-workflow-starter.md
[style] ~7-~7: Consider using “who” when you are referring to a person instead of an object.
Context: ...pec Workflow ## Goal You are an agent that specializes in working with Specs in Cl...
(THAT_WHO)
[grammar] ~23-~23: Use a hyphen to join words.
Context: ... plan and todo list. It follows the spec driven development methodology to system...
(QB_NEW_EN_HYPHEN)
[grammar] ~132-~132: Use a hyphen to join words.
Context: ...> [*] : Complete ``` ## Feature and sub agent mapping | Feature ...
(QB_NEW_EN_HYPHEN)
[style] ~148-~148: Consider using a different adverb to strengthen your wording.
Context: ...", "_v4" - spec-tasks and spec-impl are completely different sub agents, spec-tasks is for...
(COMPLETELY_ENTIRELY)
[style] ~257-~257: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e workflow steps in sequential order. - You MUST NOT skip ahead to later steps with...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~258-~258: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...and receiving explicit user approval. - You MUST treat each constraint in the workf...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~259-~259: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...the workflow as a strict requirement. - You MUST NOT assume user preferences or req...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~260-~260: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...requirements - always ask explicitly. - You MUST maintain a clear record of which s...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~261-~261: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d of which step you are currently on. - You MUST NOT combine multiple steps into a ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~279-~279: Ensure spelling is correct
Context: ...steps: 1. Start: Launch spec-impl1 (task1) and spec-impl2 (task3) in parallel ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~279-~279: Ensure spelling is correct
Context: ...unch spec-impl1 (task1) and spec-impl2 (task3) in parallel 2. After task1 complet...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~280-~280: Ensure spelling is correct
Context: ...-impl2 (task3) in parallel 2. After task1 completes: Launch spec-impl3 (task2.1) ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~281-~281: Ensure spelling is correct
Context: ...llel 3. After task2.1, task2.2, and task3 all complete: Launch spec-impl5 (task4)...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~281-~281: Ensure spelling is correct
Context: ... task3 all complete: Launch spec-impl5 (task4) - In default mode, you MUST ONLY execute o...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
.claude/agents/kfc/spec-tasks.md
[grammar] ~124-~124: Use a hyphen to join words.
Context: ...er write automated tests to test the end to end from a user perspective. - User trai...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.23.1)
.claude/agents/kfc/spec-impl.md
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.claude/agents/kfc/spec-system-prompt-loader.md
[warning] 8-8: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.claude/agents/kfc/spec-design.md
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.claude/agents/kfc/spec-requirements.md
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.claude/system-prompts/spec-workflow-starter.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.claude/agents/kfc/spec-test.md
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.claude/agents/kfc/spec-tasks.md
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.claude/agents/kfc/spec-judge.md
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (20)
.claude/system-prompts/spec-workflow-starter.md (1)
1-306: LGTM!.claude/agents/kfc/spec-system-prompt-loader.md (1)
1-39: LGTM!.claude/agents/kfc/spec-requirements.md (1)
1-46: LGTM!Also applies to: 51-71, 75-124
.claude/agents/kfc/spec-design.md (1)
1-124: LGTM!Also applies to: 135-159
.claude/agents/kfc/spec-tasks.md (1)
1-183: LGTM!.claude/agents/kfc/spec-impl.md (1)
1-40: LGTM!.claude/agents/kfc/spec-test.md (1)
1-10: LGTM!Also applies to: 19-79, 91-109
.claude/agents/kfc/spec-judge.md (1)
1-79: LGTM!Also applies to: 97-100, 111-120
.claude/settings/kfc-settings.json (1)
1-24: LGTM!src/cli/program.ts (1)
1-29: LGTM!Also applies to: 50-51
src/index.ts (1)
3-7: LGTM!scripts/generate-man.ts (1)
1-49: LGTM!Also applies to: 73-83
man/sorokeep.1 (1)
1-10: LGTM!Also applies to: 13-48
package.json (1)
14-14: LGTM!Also applies to: 24-25
README.md (1)
67-72: LGTM!tests/cli/man.test.ts (1)
1-7: LGTM!Also applies to: 48-52
tsconfig.test.json (1)
5-14: LGTM!vitest.config.ts (1)
8-12: LGTM!src/db/repositories.ts (1)
127-145: LGTM!tests/db/repositories.test.ts (1)
56-67: LGTM!Also applies to: 498-584
|
Merged as 30f0678 on Dropped two things that had nothing to do with man pages:
Kept the actual man-page work: Your branch was also based on an old point in history (predated several things already merged today), which produced a real conflict in Verified locally: lint, typecheck, full suite (1114/1114), build (which correctly regenerates the man page), and audit all clean. Closing #384 as shipped. |
#546, #328) PR #546 implemented this correctly in spirit (repeatable --channel-plugin flag, dynamic import() of a package's default-exported registration function, clear startup error on failure) but the branch bundled 60+ unrelated files — stray coverage/lint-report artifacts, eslint/vitest config changes, package.json/package-lock changes, and edits across dozens of unrelated core/utils/rpc test files — and its src/index.ts diff targeted a version of the CLI entry point that predates the createProgram()/src/cli/program.ts refactor (#622), so it couldn't be merged as-is. Reimplemented the same feature directly against the current CLI structure, giving credit to #546 for the design (the plugin convention — default-export a function receiving registerAlertChannel — and the disk-based integration test approach that writes a real package into node_modules and dynamically imports it): - src/cli/program.ts: --channel-plugin <package> (repeatable), loaded via a preAction hook before any command's action runs, scoped per createProgram() call (not module-level state) so it stays correctly isolated across multiple program instances in tests. - src/index.ts: switched to parseAsync since the hook is now async. - src/lib.ts: export registerAlertChannel/ChannelDefinition so plugin packages can type against sorokeep's public API. - docs/adding-an-alert-channel.md: new "External plugin package convention" section. - tests/commands/channel_plugin_flag.test.ts: writes a real plugin package to node_modules, loads it via createProgram().parseAsync(), and asserts the channel becomes available — plus repeated-flag and missing-package-error cases. Verified: tsc clean, full suite 1248/1248, npm audit clean, build succeeds, and manually smoke-tested against the compiled CLI binary in an isolated directory — both the success path (plugin channel appears in `alerts channels`) and the failure path (clear error, exit 1) work end-to-end. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extracts CLI construction into createProgram() (src/cli/program.ts) so scripts/generate-man.ts can introspect the Command tree without running the CLI, generates man/sorokeep.1, and wires build:man into npm run build. Per #384. Trimmed from the original PR before merging: - A full set of unrelated ".claude/agents/kfc/*", ".claude/settings/ kfc-settings.json", and ".claude/system-prompts/spec-workflow- starter.md" files - generic AI-agent workflow scaffolding with no connection to sorokeep or this issue. - getContractsByTag() and related changes to src/commands/status.ts / src/db/repositories.ts - that's issue #379's scope (--tag filtering), a different, unrelated feature. The PR's branch was also quite stale (predated the quiet-hours, contract_groups, rpc/client.ts any-type, and CI audit-scope fixes already on main), which produced a large raw diff and one real merge conflict in src/index.ts (this PR's createProgram() refactor vs. the already-merged --extension-jitter-ms option). Resolved by moving the jitter option registration into createProgram() so both are preserved.
#546, #328) PR #546 implemented this correctly in spirit (repeatable --channel-plugin flag, dynamic import() of a package's default-exported registration function, clear startup error on failure) but the branch bundled 60+ unrelated files — stray coverage/lint-report artifacts, eslint/vitest config changes, package.json/package-lock changes, and edits across dozens of unrelated core/utils/rpc test files — and its src/index.ts diff targeted a version of the CLI entry point that predates the createProgram()/src/cli/program.ts refactor (#622), so it couldn't be merged as-is. Reimplemented the same feature directly against the current CLI structure, giving credit to #546 for the design (the plugin convention — default-export a function receiving registerAlertChannel — and the disk-based integration test approach that writes a real package into node_modules and dynamically imports it): - src/cli/program.ts: --channel-plugin <package> (repeatable), loaded via a preAction hook before any command's action runs, scoped per createProgram() call (not module-level state) so it stays correctly isolated across multiple program instances in tests. - src/index.ts: switched to parseAsync since the hook is now async. - src/lib.ts: export registerAlertChannel/ChannelDefinition so plugin packages can type against sorokeep's public API. - docs/adding-an-alert-channel.md: new "External plugin package convention" section. - tests/commands/channel_plugin_flag.test.ts: writes a real plugin package to node_modules, loads it via createProgram().parseAsync(), and asserts the channel becomes available — plus repeated-flag and missing-package-error cases. Verified: tsc clean, full suite 1248/1248, npm audit clean, build succeeds, and manually smoke-tested against the compiled CLI binary in an isolated directory — both the success path (plugin channel appears in `alerts channels`) and the failure path (clear error, exit 1) work end-to-end.
2 participants
Closes #384
Summary
Testing