v2.0.0 release polish — resolve all 16 open tickets (#377-#392) - #393
Closed
ar27111994 wants to merge 15 commits into
Closed
v2.0.0 release polish — resolve all 16 open tickets (#377-#392)#393ar27111994 wants to merge 15 commits into
ar27111994 wants to merge 15 commits into
Conversation
All 31 entries under [Unreleased] (Fixed/Added/Changed) were already shipped in v2.0.0 via PRs #359 and #376. Move them into the [2.0.0] section where they belong: - Fixed entries (#348-#356): added as new ### Fixed subsection - Added entries (#352-#353): appended to existing ### Added - Changed entries (#356 items 1,6): appended to existing ### Changed [Unreleased] is now empty, ready for genuine unreleased changes.
- Add validate-ard-urls.mjs script with >=80% HTTP URL threshold - Add validate-ard-urls test suite with 7 test cases - Add validate:ard-urls to validate:release pipeline - Add per-source-kind ARD URL resolution tests (#380) - Regenerate .well-known/ai-catalog.json (99.8% HTTP URLs) - Add --test-timeout=300000 to test:self-hosting (#384) - Add 4 missing docs/guides to README repo structure tree (#385) - Remove stale #232 reference from docs/demo/README.md (#386) - Add 4 missing env vars to README diagnostics section (#387)
…#388, #391) - Add Windows git-bash path workaround to README build section (#381) - Add MSYS doubled-drive-letter troubleshooting section (#381) - Create discover/README.md with directory layout and source pack guide (#388) - Create docs/cheatsheet.md with full CLI command reference (#391) - Link cheat sheet from README Key Playbooks section
CLI routing (cli.ts): Remove MUTATING_DOMAINS substitution that was replacing subcommands with 'help' for mirror, install, stage, activate, quarantine, rebuild, workspace, wire, setup, doctor, and bundle domains. All domains now preserve the original subcommand with --help appended. Domain handlers: Add --help detection and subcommand-specific help output: - discover: printDiscoverSubcommandHelp for all 13 subcommands - mirror: printMirrorSubcommandHelp (plan, locks, acquire, diff, explain) - install/stage: printInstallSubcommandHelp (bundle, refresh, native) - activate: printActivateSubcommandHelp (host, diff, explain, rollback, reset) - quarantine: printQuarantineSubcommandHelp (list, inspect, report, approve, reject, pin) - rebuild: printRebuildSubcommandHelp (clean, full) - workspace: printWorkspaceSubcommandHelp (per-host adapter) - wire: printWireSubcommandHelp (per-host adapter) - setup: printSetupSubcommandHelp (doctor, hosts, login) Tests: Replace 'mutating domains show help' test with comprehensive 'subcommand --help shows subcommand-specific help distinct from parent' covering 11 subcommands across all domains, verifying each shows unique help text and does not create state.
Add tests for: - Missing/non-array entries field returns error - main() function with valid catalog Fix incorrect ok assertion in hash/name counts test
#382 — discover full per-source progress: - Add per-source progress output during sync - Report per-source sync duration and status - Add --no-sync flag for local-only discovery - Progress written to stderr #392 — adapter development guide: - Create ADAPTER-DEVELOPMENT.md with 13-step walkthrough - Cover registration, lifecycle wiring, native config, testing - Include Windsurf example and capability matrix - Link from CONTRIBUTING.md Coverage fixes for validate-ard-urls: - Tests for main() error path and runCliIfDirect - Refactor isDirectExecution into testable function
- Replace nested ternary with statusMap lookup in source-sync progress - Add main() cwd fallback test for validate-ard-urls - Restore accidentally removed catch block in source-sync
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR updates ARD catalog URLs, adds catalog validation, introduces targeted CLI subcommand help, adds discovery sync controls and progress output, and expands changelog, adapter-development, CLI, repository, demo, and Windows troubleshooting documentation. ChangesCLI behavior and discovery flow
ARD catalog URL validation
Release and development documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant CommandHandler
participant HelpRenderer
User->>CLI: invoke subcommand with --help
CLI->>CommandHandler: route original subcommand
CommandHandler->>HelpRenderer: render targeted help
HelpRenderer-->>User: return help output with status 0
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Release-polish bundle for v2.0.0 focused on CLI UX fixes (subcommand-specific --help routing), discovery sync progress visibility and a --no-sync escape hatch, ARD catalog URL quality validation, plus accompanying documentation and test updates.
Changes:
- Add subcommand-specific
--helpbehavior across multiple CLI domains and expand CLI help tests. - Improve
discover syncprogress output and adddiscover full --no-sync. - Add ARD URL-quality validation script + tests, adjust self-hosting test timeout, and update docs/README/CHANGELOG for release readiness.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/workspace.ts | Adds target-specific workspace <host> --help output. |
| src/wire.ts | Adds target-specific wire <host> --help output. |
| src/tests/cli-help.test.ts | Expands coverage to assert subcommand help is distinct from parent help. |
| src/tests/ard-catalog.test.ts | Adds tests to ensure ARD URL mapping uses resolvable origin URLs. |
| src/setup.ts | Adds subcommand-specific help routing for setup. |
| src/rebuild.ts | Adds subcommand-specific help routing for rebuild. |
| src/quarantine.ts | Adds subcommand-specific help routing for quarantine. |
| src/mirror.ts | Adds subcommand-specific help routing for mirror (incl. locks). |
| src/install.ts | Adds subcommand-specific help routing for install/stage subcommands. |
| src/domains/discovery/source-sync/index.ts | Adds per-source progress output to long-running source sync. |
| src/discover.ts | Adds discover subcommand help router + discover full --no-sync. |
| src/cli.ts | Adjusts global help routing to preserve subcommand so domains can render targeted help. |
| src/activate.ts | Adds subcommand-specific help routing for activate. |
| scripts/validate-ard-urls.mjs | Introduces ARD URL quality validation script for release gating. |
| scripts/tests/validate-ard-urls.test.mjs | Adds script-level test coverage for ARD URL validation. |
| README.md | Adds Windows git-bash guidance, missing env vars, guide links, and repo tree updates. |
| package.json | Sets self-hosting test timeout and wires validate:ard-urls into validate:release. |
| docs/guides/TROUBLESHOOTING.md | Adds Windows git-bash/MSYS doubled-drive-letter troubleshooting section. |
| docs/guides/ADAPTER-DEVELOPMENT.md | Adds adapter development guide and references from CONTRIBUTING/README. |
| docs/demo/README.md | Removes stale “tracked by #232” reference, points to shipped demo. |
| docs/cheatsheet.md | Adds CLI cheat sheet for common workflows. |
| discover/README.md | Adds discover/ directory structure and schema/source-pack guidance. |
| CONTRIBUTING.md | Updates key directories section and links adapter guide. |
| CHANGELOG.md | Moves shipped entries from [Unreleased] into [2.0.0]. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+290
to
+293
| assert.ok( | ||
| ard.url!.startsWith("https://"), | ||
| `${kind}: url must be a resolvable https URL, got ${ard.url?.slice(0, 50)}`, | ||
| ); |
Comment on lines
+216
to
220
| (synchronizedState ? synchronizedState.status : "skipped"); | ||
| process.stderr.write(`${statusLabel} (${syncDuration}ms)\n`); | ||
| } | ||
| } catch (error) { | ||
| // Stale-data fallback + persistence tracking. |
Comment on lines
+1099
to
+1110
| sync: { | ||
| heading: "discover sync — Synchronize discovered sources", | ||
| lines: [ | ||
| "Usage: agent-harness discover sync", | ||
| "", | ||
| "Fetches and persists data from all configured discovery sources. Uses the", | ||
| "local index when fresh, performs live harvest otherwise.", | ||
| "", | ||
| "Options:", | ||
| " --no-sync Skip source sync (local discovery only)", | ||
| ], | ||
| }, |
Comment on lines
+368
to
+377
| void test("main uses process.cwd fallback when no cwd option provided", async () => { | ||
| // Verify that main() can be called without options — exercises the | ||
| // `options.cwd ?? process.cwd()` fallback path at line 86. | ||
| // This relies on the project's own .well-known/ai-catalog.json being valid. | ||
| const result = main(); | ||
| // The project catalog should be valid (>=80% HTTP URLs) | ||
| assert.ok(result.ok); | ||
| assert.ok(result.stats.total > 0); | ||
| assert.ok(result.stats.fraction >= 0.8); | ||
| }); |
Comment on lines
+315
to
+320
| - [Host Adapter Types](../src/host-adapters/types.ts) — full type definitions | ||
| - [Registry](../src/host-adapters/registry.ts) — adapter registration | ||
| - [Native Config](../src/host-adapters/native-config.ts) — shared config generation | ||
| - [Native Wire](../src/host-adapters/native-wire.ts) — wire-in infrastructure | ||
| - [V2 CLI Contract](./V2-CONTRACT.md) — host lifecycle model | ||
| - [Contributing Guide](../CONTRIBUTING.md) — contribution workflow |
Comment on lines
+218
to
+235
| | Path | Purpose | | ||
| | ------------------------ | -------------------------------------- | | ||
| | `src/` | TypeScript source | | ||
| | `src/domains/discovery/` | Harvesting, catalog, demand signals | | ||
| | `src/host-adapters/` | Per-host wire and extension installers | | ||
|
|
||
| ### Adding a new host adapter | ||
|
|
||
| See the [Adapter Development Guide](docs/guides/ADAPTER-DEVELOPMENT.md) for a | ||
| step-by-step walkthrough covering registration, lifecycle wiring, capability | ||
| matrix, native config generation, and testing — with a worked Windsurf example. | ||
| | `src/recommend/` | Selection scoring and report generation | | ||
| | `src/mirror/` | Artifact download and provenance | | ||
| | `src/install/` | Staged install lifecycle | | ||
| | `src/activate.ts` | Runtime root swap and rollback | | ||
| | `discover/` | Source config, policy, and seed JSON | | ||
| | `docs/` | Guides, reference, playbooks | | ||
| | `plans/` | Advisor implementation plans | |
|
|
||
| ```bash | ||
| agent-harness discover demand-profile # Scan workspace for demand signals | ||
| agent-harness discover sources # List enabled discovery sources |
| agent-harness mirror plan --host <host> # Preview mirror plan | ||
| agent-harness mirror acquire --host <host> # Download selected assets | ||
| agent-harness mirror diff # Compare local vs remote mirrors | ||
| agent-harness mirror locks # List lock files |
PR Summary by QodoRelease polish: ARD URL validation, subcommand help, and discovery UX
AI Description
Diagram
High-Level Assessment
Files changed (24)
|
Code Review by Qodo
1. Bundle help misrouted
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+170
to
176
| // Preserve the original subcommand so domain handlers can show | ||
| // subcommand-specific help. Mutating domains receive the subcommand + | ||
| // --help flag — handlers must detect --help and show help text instead | ||
| // of executing the subcommand (#383). | ||
| const domainArgs = [subcommand, ...extra]; | ||
| if (wasHelpRequested) { | ||
| domainArgs.push("--help"); |
There was a problem hiding this comment.
1. Bundle help misrouted 🐞 Bug ≡ Correctness
runHelpCommand forwards bundle explain --help to runMirror(['explain','--help']), which describes mirror-artifact explain instead of the bundle-lock explain (bundle-explain) that the bundle explain alias actually runs. This produces incorrect CLI help for the bundle alias and is a regression in the help routing logic.
Agent Prompt
### Issue description
`bundle explain` is an alias for mirror’s `bundle-explain` subcommand in normal execution, but the updated help routing preserves the raw subcommand and passes `explain --help` to `runMirror`, yielding help for the wrong command.
### Issue Context
- Normal execution path translates `bundle explain` to `runMirror(['bundle-explain', ...])`.
- Help execution path does not apply this translation.
- Additionally, mirror’s subcommand-specific help map currently has no entry for `bundle-explain`, so even the canonical `mirror bundle-explain --help` will fall back to generic mirror help.
### Fix Focus Areas
- src/cli.ts[149-206]
- src/cli.ts[81-99]
- src/mirror.ts[26-67]
- src/mirror.ts[112-182]
### Suggested fix
1. In `runHelpCommand`’s `nonFlagArgs.length >= 2` routing, special-case domain `bundle` so that:
- when `subcommand === 'explain'`, dispatch `runMirror(['bundle-explain', ...extra, '--help'], ...)` (or equivalent), not `runMirror(['explain', ...])`.
2. Add subcommand help text entries in `printMirrorSubcommandHelp` for `bundle-explain` and its alias `explain-bundle` so `--help` output is truly subcommand-specific.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1099
to
+1109
| sync: { | ||
| heading: "discover sync — Synchronize discovered sources", | ||
| lines: [ | ||
| "Usage: agent-harness discover sync", | ||
| "", | ||
| "Fetches and persists data from all configured discovery sources. Uses the", | ||
| "local index when fresh, performs live harvest otherwise.", | ||
| "", | ||
| "Options:", | ||
| " --no-sync Skip source sync (local discovery only)", | ||
| ], |
There was a problem hiding this comment.
2. Discover help flags mismatch 🐞 Bug ≡ Correctness
discover sync --help documents a --no-sync option that the sync implementation never reads (it only checks --full), while discover full implements --no-sync but the discover full help output does not mention it. This makes the CLI help misleading for both discover sync and discover full.
Agent Prompt
### Issue description
The new subcommand-specific discover help text is inconsistent with actual flag handling:
- `discover sync` help advertises `--no-sync`, but `discover sync` does not implement it.
- `discover full` implements `--no-sync`, but `discover full` help doesn’t list it.
### Issue Context
`discover sync` supports `--full` (force re-harvest) and `discover full` supports `--no-sync` (skip syncing indexed sources). Help should reflect this accurately.
### Fix Focus Areas
- src/discover.ts[1056-1110]
- src/discover.ts[201-226]
- src/discover.ts[243-283]
- src/discover.ts[1310-1363]
### Suggested fix
1. Update `printDiscoverSubcommandHelp('sync')` to remove `--no-sync` and add `--full` with correct meaning.
2. Update `printDiscoverFullHelp()` to include `--no-sync` and describe that it skips `syncIndexedSources` and uses existing local source-sync/catalog index state.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+151
to
+158
| // Per-source progress: print source name before sync starts (#382). | ||
| // Use process.stderr so progress lines don't contaminate stdout when | ||
| // the CLI is invoked in JSON-output mode. | ||
| if (totalSources > 1) { | ||
| process.stderr.write(`${progressLabel} … `); | ||
| } | ||
|
|
||
| const syncStart = Date.now(); |
There was a problem hiding this comment.
3. Sync progress breaks on errors 🐞 Bug ◔ Observability
Per-source progress writes a prefix to stderr before syncing, but the completion line is only emitted in the success path; if a source throws, the catch path never terminates the progress line. This results in concatenated/malformed progress output exactly when failures occur.
Agent Prompt
### Issue description
`syncIndexedSources` writes `"[discover sync] i/N source … "` to `stderr` before starting each source, but prints the status+duration newline only inside the `try` (success) path. When the sync throws and control enters `catch`, the previously-written progress prefix remains unterminated.
### Issue Context
This instrumentation was added for #382. It should produce well-formed progress output for success *and* failure/stale fallback cases.
### Fix Focus Areas
- src/domains/discovery/source-sync/index.ts[146-252]
### Suggested fix
- Track whether a progress prefix was written (e.g., `const wroteProgress = totalSources > 1`).
- Compute `syncDuration` in a `finally` block.
- On `catch`, emit a completion label such as `failed (<ms>)\n` (or `stale (<ms>)\n` depending on the state pushed), so every prefix always ends with a newline.
- Keep writing to `stderr` as currently intended.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+368
to
+376
| void test("main uses process.cwd fallback when no cwd option provided", async () => { | ||
| // Verify that main() can be called without options — exercises the | ||
| // `options.cwd ?? process.cwd()` fallback path at line 86. | ||
| // This relies on the project's own .well-known/ai-catalog.json being valid. | ||
| const result = main(); | ||
| // The project catalog should be valid (>=80% HTTP URLs) | ||
| assert.ok(result.ok); | ||
| assert.ok(result.stats.total > 0); | ||
| assert.ok(result.stats.fraction >= 0.8); |
There was a problem hiding this comment.
4. Brittle ard url test 🐞 Bug ☼ Reliability
The new test calls main() without passing cwd, so it depends on the test runner’s current working directory being the repo root and on the repo’s .well-known/ai-catalog.json remaining valid. This makes the test fragile in IDE runs, alternate CI invocations, or when tests run from a different CWD.
Agent Prompt
### Issue description
`scripts/tests/validate-ard-urls.test.mjs` includes a test that calls `main()` with no options to exercise the `options.cwd ?? process.cwd()` fallback. This relies on global process state (CWD) and the repository’s real `.well-known/ai-catalog.json`, which makes the test non-hermetic.
### Issue Context
The test should validate the fallback behavior without depending on repo contents or invocation directory.
### Fix Focus Areas
- scripts/tests/validate-ard-urls.test.mjs[368-377]
- scripts/validate-ard-urls.mjs[85-104]
### Suggested fix
- Create a temp directory with a valid `.well-known/ai-catalog.json`.
- Temporarily `process.chdir(tempDir)` inside the test, call `main()` with no args, then restore the previous CWD in `finally`.
This still exercises the fallback path while keeping the test hermetic.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+224
to
+235
| ### Adding a new host adapter | ||
|
|
||
| See the [Adapter Development Guide](docs/guides/ADAPTER-DEVELOPMENT.md) for a | ||
| step-by-step walkthrough covering registration, lifecycle wiring, capability | ||
| matrix, native config generation, and testing — with a worked Windsurf example. | ||
| | `src/recommend/` | Selection scoring and report generation | | ||
| | `src/mirror/` | Artifact download and provenance | | ||
| | `src/install/` | Staged install lifecycle | | ||
| | `src/activate.ts` | Runtime root swap and rollback | | ||
| | `discover/` | Source config, policy, and seed JSON | | ||
| | `docs/` | Guides, reference, playbooks | | ||
| | `plans/` | Advisor implementation plans | |
There was a problem hiding this comment.
5. Contributing table broken 🐞 Bug ⚙ Maintainability
The Key directories Markdown table in CONTRIBUTING.md is interrupted by a new heading/paragraph, leaving subsequent rows outside the table and breaking rendering. This degrades the readability of contributor documentation.
Agent Prompt
### Issue description
In `CONTRIBUTING.md`, the `### Adding a new host adapter` section was inserted mid-way through the `Key directories` table, so the remaining rows no longer render as table rows.
### Issue Context
Markdown tables must be contiguous (header + separator + rows without interruption).
### Fix Focus Areas
- CONTRIBUTING.md[216-235]
### Suggested fix
- Move the `### Adding a new host adapter` section below the completed table, or
- Reformat so the table is complete (all directory rows included) before starting the new heading.
- Ensure the remaining rows keep consistent `| col | col |` formatting and alignment markers.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.well-known/ai-catalog.json (1)
26134-26212: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftExclude local
file://catalog entries from.well-known/ai-catalog.json.
writeArdCatalog()serializeslocal-directoryassets unchanged, so the published ARD catalog includes Windows absolute paths likefile:///C:/Users/ar271/.... Filter or redact these entries before writing, and updatevalidate:ard-urlssofile://URLs are rejected/flagged instead of falling into the count-based non-HTTP buckets.🤖 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 @.well-known/ai-catalog.json around lines 26134 - 26212, Update writeArdCatalog() to exclude or redact local-directory assets with file:// URLs before serializing .well-known/ai-catalog.json, preventing Windows absolute paths from being published. Also update validate:ard-urls to explicitly reject or flag file:// URLs rather than counting them in non-HTTP buckets, while preserving validation for supported URL schemes.src/discover.ts (1)
243-247: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winNow-unreachable
hasHelpFlagchecks inside the switch.The new early-return block at Lines 134-143 unconditionally returns before
switch (command)wheneverhasHelpFlagis true — whether or notcommandis inhasSpecificHelp. Since"full","breadth","recall", and"candidate-pool"are all inhasSpecificHelp, theseif (hasHelpFlag)guards insidecase "full"(Lines 244-247) andcase "breadth"/"recall"/"candidate-pool"(Lines 310-313) can never be reached. They resolve to the same output viaprintDiscoverSubcommandHelp's fallback, so there's no behavior change, but the dead branches will register as uncovered in coverage tooling (risking the PR's 100% coverage goal) and are confusing to maintain.🧹 Suggested cleanup
case "full": { - if (hasHelpFlag) { - printDiscoverFullHelp(); - return 0; - } const aiEnrichmentFlags = parseAiEnrichmentFlags(rest);case "breadth": case "recall": case "candidate-pool": - if (hasHelpFlag) { - printDiscoverBreadthHelp(); - return 0; - } await runDiscoveryBreadth(workingDirectory, projectRoot); return 0;Also applies to: 307-313
🤖 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 `@src/discover.ts` around lines 243 - 247, Remove the now-unreachable hasHelpFlag branches from the switch cases for "full", "breadth", "recall", and "candidate-pool" in the discover command handling. Keep each case’s normal execution path unchanged, relying on the earlier help handling and printDiscoverSubcommandHelp fallback.src/domains/discovery/source-sync/index.ts (1)
219-252: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPer-source progress line is never completed on failure.
The success path (Lines 206-218) writes a
status (Nms)\ncompletion line to stderr, but thecatchblock does not. Since the start-of-sync write at Line 155 has no trailing newline ("${progressLabel} … "), an exception here leaves that line unterminated; the next source's start-of-sync write then gets appended to it, producing garbled stderr output — exactly in the scenario (sync failures) where clear per-source reporting matters most.🔧 Suggested fix
entriesDirty ||= context.entriesDirty; + + const syncDuration = Date.now() - syncStart; + if (totalSources > 1) { + const statusLabel = shouldFallBackToStale ? "stale" : "failed"; + process.stderr.write(`${statusLabel} (${syncDuration}ms)\n`); + } } }(Place this after
sourceStates.push({...})whereshouldFallBackToStaleis already computed, so the reported status matches the persisted one.)🤖 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 `@src/domains/discovery/source-sync/index.ts` around lines 219 - 252, Complete the per-source stderr progress line in the catch block after sourceStates.push, using the already computed shouldFallBackToStale value so the reported status matches persisted state. Include the failure duration/status and terminate the output with a newline, mirroring the success-path completion behavior and ensuring the next source starts on a fresh line.src/cli.ts (1)
162-176: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale comment contradicts the new implementation.
Lines 162-167 still describe the old behavior ("Only discover and recommend handle --help internally... we substitute 'help' to prevent mutation"), but the code right below it (170-176) now preserves the real subcommand and appends
--helpfor all mutating domains, not just discover/recommend. This directly contradicts the accurate note added at Lines 170-173, and will confuse future readers.📝 Suggested comment fix
- // When --help appears at subcommand depth (e.g., "discover full --help"), - // route to the domain handler. Only discover and recommend handle --help - // internally for subcommand-specific help output. For mutating domains - // (mirror, install, activate, quarantine, rebuild, workspace, wire, setup), - // we substitute "help" to prevent mutation — the handler then shows its - // generic help without executing any phase. + // When --help appears at subcommand depth (e.g., "discover full --help"), + // route to the domain handler, preserving the original subcommand and + // appending --help. Each domain handler is responsible for detecting + // --help and rendering targeted help instead of executing (`#383`).🤖 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 `@src/cli.ts` around lines 162 - 176, Update the comment above the nonFlagArgs handling in the CLI entrypoint to remove the outdated claim about substituting “help” and limiting internal help handling to discover/recommend. Describe the current behavior consistently with domainArgs: preserve the actual subcommand and append --help when requested so every domain handler can display help without executing the operation.
🧹 Nitpick comments (2)
src/wire.ts (1)
122-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTarget-specific wire help omits
--reset.
printWireHelplists--resetas a valid mode (Line 151), andgetWireModesupports it, but the per-target help added here only documents--preview/--apply/--state-root. Users ofwire <target> --helpwon't discover the reset mode.📝 Suggested fix
"Options:", " --preview Preview the wire plan without applying (default)", " --apply Apply the wire plan to the workspace", + " --reset Reset the wire target to an unwired state", " --state-root <path> Override state directory",🤖 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 `@src/wire.ts` around lines 122 - 131, Update the target-specific help text in printWireHelp to document the supported --reset mode alongside --preview and --apply, matching the modes handled by getWireMode.src/activate.ts (1)
539-609: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIdentical
helpTextslookup + fallback pattern duplicated across 7 files.Each of these files (re)implements the same shape: a
Record<string, {heading, lines}>map, a lookup by subcommand,printCommandHelp({heading, entries: [], sections: [{title: "", lines}]})when found, and a fallback call to the domain's parent help otherwise. Extracting a shared helper (e.g.,printMappedSubcommandHelp(subcommand, helpTexts, fallback)alongside the existingprintCommandHelpinsrc/lib/cli-output.ts) would remove the repeated boilerplate and make it easier to keep future subcommand help texts consistent and complete.
src/activate.ts#L539-L609: extractprintActivateSubcommandHelp's lookup/fallback logic into the shared helper, passingprintActivateHelpas the fallback.src/install.ts#L124-L186: same, passingprintInstallHelpas the fallback.src/mirror.ts#L111-L182: same, passingprintMirrorHelpas the fallback.src/quarantine.ts#L486-L560: same, passingprintQuarantineHelpas the fallback.src/rebuild.ts#L154-L194: same, passingprintRebuildHelpas the fallback.src/setup.ts#L432-L482: same, passingprintSetupHelpas the fallback.src/discover.ts#L1053-L1210: same, keeping the extrafull/breadthfallback branches local to this file since they route to different functions than a plain map miss.🤖 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 `@src/activate.ts` around lines 539 - 609, Extract the repeated mapped-subcommand lookup and rendering from printActivateSubcommandHelp into a shared helper beside printCommandHelp, then use it with printActivateHelp as fallback in src/activate.ts:539-609, printInstallHelp in src/install.ts:124-186, printMirrorHelp in src/mirror.ts:111-182, printQuarantineHelp in src/quarantine.ts:486-560, printRebuildHelp in src/rebuild.ts:154-194, and printSetupHelp in src/setup.ts:432-482. In src/discover.ts:1053-1210, reuse the helper for the standard map-miss path but keep the existing full/breadth fallback branches local because they dispatch to different functions.
🤖 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 `@CONTRIBUTING.md`:
- Around line 224-235: Keep the directory map table contiguous by moving the
rows for src/recommend, src/mirror, src/install, src/activate.ts, discover,
docs, and plans above the “Adding a new host adapter” heading, or explicitly
close the table and start a separate table after the prose.
In `@discover/README.md`:
- Around line 34-40: Add the required assetKinds field to the source-pack entry
checklist in the documentation, alongside the existing required fields, and
describe it as an array of asset kinds consistent with the v2.0.0 contract.
In `@docs/cheatsheet.md`:
- Around line 27-37: Update the discovery command list in the cheatsheet to
document the `discover full --no-sync` workflow alongside `discover full`,
indicating that it runs the full pipeline without synchronizing sources.
In `@docs/guides/ADAPTER-DEVELOPMENT.md`:
- Around line 271-273: Replace the no-op test body in “windsurf workspace --help
shows windsurf-specific help” with a real built-cli-harness invocation and
assertions verifying the command succeeds and includes Windsurf-specific help
output; if executable test code cannot be provided here, explicitly label the
snippet as pseudocode instead of leaving a passing empty test.
- Around line 276-280: Update the links in ADAPTER-DEVELOPMENT.md, including the
`#supported-hosts` reference and the ../src/... and ../CONTRIBUTING.md paths, to
resolve from the guide through the repository root using
../../README.md#supported-hosts, ../../src/..., and ../../CONTRIBUTING.md
equivalents.
- Around line 36-45: Update the adapter scaffold documentation to import
AssetHostNativeConfig from ../types.js at its usage site, and remove the
undefined HostNativeConfigContext parameter and call-signature examples. Keep
lifecycleHost, recommendationHost, and wireHost examples typeable using the
string host values accepted by HostAdapter.
In `@docs/guides/TROUBLESHOOTING.md`:
- Around line 176-178: Update the fenced error block in TROUBLESHOOTING.md to
use the text language identifier while preserving its existing error content.
In `@README.md`:
- Around line 1518-1526: Add docs/guides/ADAPTER-DEVELOPMENT.md to the
repository tree listing in README.md, alongside the other entries under
docs/guides, so the documented structure includes the existing adapter guide.
In `@scripts/validate-ard-urls.mjs`:
- Around line 119-120: Remove the stale eslint-disable-next-line directive
immediately preceding runCliIfDirect, leaving the direct runCliIfDirect
invocation unchanged.
- Around line 21-23: Update isHttpUrl in scripts/validate-ard-urls.mjs to parse
input with URL, returning true only for http: or https: protocols with a
non-empty hostname and false for malformed values. In
scripts/tests/validate-ard-urls.test.mjs, extend the classification tests with
malformed HTTP(S) URLs and verify they do not increment httpCount.
In `@src/discover.ts`:
- Around line 1099-1110: Move the --no-sync option documentation from
helpTexts.sync to the discover full help text, updating the Options list used by
printDiscoverFullHelp. Keep discover sync help limited to flags handled by the
sync case, and document --no-sync under discover full where it is implemented.
In `@src/tests/ard-catalog.test.ts`:
- Around line 229-260: Update the sourceKinds matrix in the source-kind coverage
test to include the omitted docs, marketplace, and local-manifest SourceKind
values with valid originUrl and manifestEntry fixtures, or explicitly narrow the
test’s claim to the currently covered harvestable kinds and document those
exclusions.
In `@src/tests/cli-help.test.ts`:
- Around line 252-382: The test named “subcommand --help shows
subcommand-specific help distinct from parent (`#383`)” lacks coverage for
quarantine, rebuild, wire, workspace, and setup routes. Add equivalent isolated
CLI invocations and assertions for each domain/subcommand help path, verifying
the command-specific title and description while rejecting the corresponding
parent help text.
---
Outside diff comments:
In @.well-known/ai-catalog.json:
- Around line 26134-26212: Update writeArdCatalog() to exclude or redact
local-directory assets with file:// URLs before serializing
.well-known/ai-catalog.json, preventing Windows absolute paths from being
published. Also update validate:ard-urls to explicitly reject or flag file://
URLs rather than counting them in non-HTTP buckets, while preserving validation
for supported URL schemes.
In `@src/cli.ts`:
- Around line 162-176: Update the comment above the nonFlagArgs handling in the
CLI entrypoint to remove the outdated claim about substituting “help” and
limiting internal help handling to discover/recommend. Describe the current
behavior consistently with domainArgs: preserve the actual subcommand and append
--help when requested so every domain handler can display help without executing
the operation.
In `@src/discover.ts`:
- Around line 243-247: Remove the now-unreachable hasHelpFlag branches from the
switch cases for "full", "breadth", "recall", and "candidate-pool" in the
discover command handling. Keep each case’s normal execution path unchanged,
relying on the earlier help handling and printDiscoverSubcommandHelp fallback.
In `@src/domains/discovery/source-sync/index.ts`:
- Around line 219-252: Complete the per-source stderr progress line in the catch
block after sourceStates.push, using the already computed shouldFallBackToStale
value so the reported status matches persisted state. Include the failure
duration/status and terminate the output with a newline, mirroring the
success-path completion behavior and ensuring the next source starts on a fresh
line.
---
Nitpick comments:
In `@src/activate.ts`:
- Around line 539-609: Extract the repeated mapped-subcommand lookup and
rendering from printActivateSubcommandHelp into a shared helper beside
printCommandHelp, then use it with printActivateHelp as fallback in
src/activate.ts:539-609, printInstallHelp in src/install.ts:124-186,
printMirrorHelp in src/mirror.ts:111-182, printQuarantineHelp in
src/quarantine.ts:486-560, printRebuildHelp in src/rebuild.ts:154-194, and
printSetupHelp in src/setup.ts:432-482. In src/discover.ts:1053-1210, reuse the
helper for the standard map-miss path but keep the existing full/breadth
fallback branches local because they dispatch to different functions.
In `@src/wire.ts`:
- Around line 122-131: Update the target-specific help text in printWireHelp to
document the supported --reset mode alongside --preview and --apply, matching
the modes handled by getWireMode.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 4b556047-0b8e-4857-8ac8-921cde51f755
📒 Files selected for processing (25)
.well-known/ai-catalog.jsonCHANGELOG.mdCONTRIBUTING.mdREADME.mddiscover/README.mddocs/cheatsheet.mddocs/demo/README.mddocs/guides/ADAPTER-DEVELOPMENT.mddocs/guides/TROUBLESHOOTING.mdpackage.jsonscripts/tests/validate-ard-urls.test.mjsscripts/validate-ard-urls.mjssrc/activate.tssrc/cli.tssrc/discover.tssrc/domains/discovery/source-sync/index.tssrc/install.tssrc/mirror.tssrc/quarantine.tssrc/rebuild.tssrc/setup.tssrc/tests/ard-catalog.test.tssrc/tests/cli-help.test.tssrc/wire.tssrc/workspace.ts
Comment on lines
+224
to
+235
| ### Adding a new host adapter | ||
|
|
||
| See the [Adapter Development Guide](docs/guides/ADAPTER-DEVELOPMENT.md) for a | ||
| step-by-step walkthrough covering registration, lifecycle wiring, capability | ||
| matrix, native config generation, and testing — with a worked Windsurf example. | ||
| | `src/recommend/` | Selection scoring and report generation | | ||
| | `src/mirror/` | Artifact download and provenance | | ||
| | `src/install/` | Staged install lifecycle | | ||
| | `src/activate.ts` | Runtime root swap and rollback | | ||
| | `discover/` | Source config, policy, and seed JSON | | ||
| | `docs/` | Guides, reference, playbooks | | ||
| | `plans/` | Advisor implementation plans | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Keep the directory map table contiguous.
The “Adding a new host adapter” heading and prose interrupt the table, so the rows at Lines 229-235 no longer render as table rows. Move those rows above the heading, or close the table before the prose and create a separate table.
🤖 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 `@CONTRIBUTING.md` around lines 224 - 235, Keep the directory map table
contiguous by moving the rows for src/recommend, src/mirror, src/install,
src/activate.ts, discover, docs, and plans above the “Adding a new host adapter”
heading, or explicitly close the table and start a separate table after the
prose.
Comment on lines
+34
to
+40
| 3. Add an entry with these required fields: | ||
| - `id`: unique identifier (e.g., `"username-project-name"`) | ||
| - `authorityTier`: `"official-first-party"`, `"trusted-community"`, or | ||
| `"unverified-community"` | ||
| - `sourcePriority`: integer (1–100, higher = preferred) | ||
| - `endpoints`: object with at least a `repo` URL | ||
| - `tags`: array of descriptive tags |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Document all required source-pack fields.
The checklist omits assetKinds, which is required for source-pack entries according to the v2.0.0 release contract. Add it to prevent contributors from creating invalid pack definitions.
Proposed documentation update
- `endpoints`: object with at least a `repo` URL
+ - `assetKinds`: array of supported asset kinds
- `tags`: array of descriptive tags📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 3. Add an entry with these required fields: | |
| - `id`: unique identifier (e.g., `"username-project-name"`) | |
| - `authorityTier`: `"official-first-party"`, `"trusted-community"`, or | |
| `"unverified-community"` | |
| - `sourcePriority`: integer (1–100, higher = preferred) | |
| - `endpoints`: object with at least a `repo` URL | |
| - `tags`: array of descriptive tags | |
| 3. Add an entry with these required fields: | |
| - `id`: unique identifier (e.g., `"username-project-name"`) | |
| - `authorityTier`: `"official-first-party"`, `"trusted-community"`, or | |
| `"unverified-community"` | |
| - `sourcePriority`: integer (1–100, higher = preferred) | |
| - `endpoints`: object with at least a `repo` URL | |
| - `assetKinds`: array of supported asset kinds | |
| - `tags`: array of descriptive tags |
🤖 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 `@discover/README.md` around lines 34 - 40, Add the required assetKinds field
to the source-pack entry checklist in the documentation, alongside the existing
required fields, and describe it as an array of asset kinds consistent with the
v2.0.0 contract.
Comment on lines
+27
to
+37
| ```bash | ||
| agent-harness discover demand-profile # Scan workspace for demand signals | ||
| agent-harness discover sources # List enabled discovery sources | ||
| agent-harness discover sync # Harvest source data (network) | ||
| agent-harness discover catalog # Build full asset catalog | ||
| agent-harness discover select # Filter catalog by demand + policy | ||
| agent-harness discover full # Run full pipeline (demand → select) | ||
| agent-harness discover stats # Catalog statistics summary | ||
| agent-harness discover enrich # AI enrichment pass (optional) | ||
| agent-harness discover ard-export # Export to ARD ai-catalog.json | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the discover full --no-sync workflow.
The cheat sheet lists discover full but omits the new synchronization bypass, so users cannot find the documented way to run discovery without syncing sources.
🤖 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/cheatsheet.md` around lines 27 - 37, Update the discovery command list
in the cheatsheet to document the `discover full --no-sync` workflow alongside
`discover full`, indicating that it runs the full pipeline without synchronizing
sources.
Comment on lines
+36
to
+45
| ```typescript | ||
| import type { HostAdapter, HostAdapterModule } from "./types.js"; | ||
| import { collectActivatedAssetPrerequisiteDiagnostics } from "../lib/asset-prerequisites.js"; | ||
|
|
||
| const adapter: HostAdapter = { | ||
| id: "windsurf", | ||
| displayName: "Windsurf", | ||
| aliases: ["windsurf-ide"], | ||
| lifecycleHost: "opencode", | ||
| wireHost: "windsurf" as HostTarget, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files | rg '(^|/)ADAPTER-DEVELOPMENT\.md$|package\.json$|src/|tests/|docs/' || true
printf '\nTarget file excerpt:\n'
if [ -f docs/guides/ADAPTER-DEVELOPMENT.md ]; then
nl -ba docs/guides/ADAPTER-DEVELOPMENT.md | sed -n '1,120p'
fi
printf '\nSearch relevant symbols:\n'
rg -n "HostTarget|HostNativeConfigContext|AssetHostNativeConfig|interface HostAdapter|type HostAdapter|interface HostAdapterModule|type HostAdapterModule" .Repository: ar27111994/agent-harness
Length of output: 12181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant type definitions/usages:\n'
rg -n "HostTarget|HostNativeConfigContext|AssetHostNativeConfig|HostAdapter|HostAdapterModule|wireHost|lifecycleHost" --glob '!node_modules' --glob '!dist' --glob '!build' .Repository: ar27111994/agent-harness
Length of output: 29213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Docs excerpt with line numbers (using awk since nl is unavailable):\n'
awk 'NR>=1 && NR<=130 { printf "%5d %s\n", NR, $0 }' docs/guides/ADAPTER-DEVELOPMENT.md
printf '\nDocs excerpt around native config examples:\n'
awk 'NR>=100 && NR<=125 { printf "%5d %s\n", NR, $0 }' docs/guides/ADAPTER-DEVELOPMENT.md
printf '\nDocs excerpt around compatibility matrix example:\n'
awk 'NR>=220 && NR<=260 { printf "%5d %s\n", NR, $0 }' docs/guides/ADAPTER-DEVELOPMENT.md
printf '\nTypes and registry excerpts:\n'
awk 'NR>=40 && NR<=70 { printf "%5d %s\n", NR, $0 }' src/types/core.ts
printf '---\n'
awk 'NR>=1 && NR<=105 { printf "%5d %s\n", NR, $0 }' src/host-adapters/types.ts
printf '---\n'
awk 'NR>=40 && NR<=55 { printf "%5d %s\n", NR, $0 }' src/host-adapters/native-config.tsRepository: ar27111994/agent-harness
Length of output: 8733
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'src/host-adapters/types.ts:\n'
awk 'NR>=1 && NR<=140 { printf "%5d %s\n", NR, $0 }' src/host-adapters/types.ts
printf '\nsrc/host-adapters/native-config.ts:'
awk 'NR>=1 && NR<=70 { printf "%5d %s\n", NR, $0 }' src/host-adapters/native-config.ts
printf '\nsrc/types/catalog.ts relevant excerpt:'
awk 'NR>=1 && NR<=70 { printf "%5d %s\n", NR, $0 }' src/types/catalog.tsRepository: ar27111994/agent-harness
Length of output: 5230
Make the adapter scaffold references typeable.
HostAdapter accepts string host strings for lifecycleHost, recommendationHost, and wireHost, and AssetHostNativeConfig/AssetHostNativeConfigMap are not exported from src/host-adapters/types.js. Import AssetHostNativeConfig from ../types.js where it is actually used and remove the undefined HostNativeConfigContext parameter/call signature examples.
🤖 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/guides/ADAPTER-DEVELOPMENT.md` around lines 36 - 45, Update the adapter
scaffold documentation to import AssetHostNativeConfig from ../types.js at its
usage site, and remove the undefined HostNativeConfigContext parameter and
call-signature examples. Keep lifecycleHost, recommendationHost, and wireHost
examples typeable using the string host values accepted by HostAdapter.
Comment on lines
+271
to
+273
| void test("windsurf workspace --help shows windsurf-specific help", async () => { | ||
| // Use the built-cli-harness to verify CLI integration | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the no-op CLI integration test.
This test always passes because it contains only a comment. Provide an actual built-CLI invocation and assertion, or clearly label the snippet as pseudocode so contributors do not mistake it for coverage.
🤖 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/guides/ADAPTER-DEVELOPMENT.md` around lines 271 - 273, Replace the no-op
test body in “windsurf workspace --help shows windsurf-specific help” with a
real built-cli-harness invocation and assertions verifying the command succeeds
and includes Windsurf-specific help output; if executable test code cannot be
provided here, explicitly label the snippet as pseudocode instead of leaving a
passing empty test.
Comment on lines
+21
to
+23
| function isHttpUrl(url) { | ||
| return typeof url === "string" && /^https?:\/\//i.test(url); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
jq '{engines, type}' package.json
rg -n -C 3 'function isHttpUrl|https?:\\/\\/|new URL' scripts/validate-ard-urls.mjs
rg -n -C 3 'validateArdUrls passes|below 80|malformed|invalid' scripts/tests/validate-ard-urls.test.mjsRepository: ar27111994/agent-harness
Length of output: 1921
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== validator outline =="
ast-grep outline scripts/validate-ard-urls.mjs || true
echo "== validator isHttpUrl and stats references =="
rg -n -C 5 'isHttpUrl|httpCount|total|fraction|validateArdUrls' scripts/validate-ard-urls.mjs
echo "== test classification cases around malformed/current tests =="
sed -n '39,115p;180,205p' scripts/tests/validate-ard-urls.test.mjs | cat -n
echo "== Node URL / regex probe =="
node - <<'JS'
function isHttpUrl(url) {
return typeof url === "string" && /^https?:\/\//i.test(url);
}
const samples = [
"https://",
"https://example",
"http://example",
"http://example.com",
"https://github.com/test/repo",
"https://example.com/",
"",
"not a url",
"file:///home/user/local-skill.md",
"ftp://example.com",
123,
null,
];
for (const s of samples) {
try {
const parsed = new URL(String(s));
console.log(JSON.stringify({
input: String(s),
current: isHttpUrl(s),
parsedProtocol: parsed.protocol,
parsedHostname: parsed.hostname,
hasHostname: Boolean(parsed.hostname && parsed.hostname.length > 0)
}));
} catch (err) {
console.log(JSON.stringify({
input: String(s),
current: isHttpUrl(s),
parsedProtocol: undefined,
parsedHostname: undefined,
hasHostname: false,
error: err && err.message
}));
}
}
JSRepository: ar27111994/agent-harness
Length of output: 8374
Validate HTTP URLs with URL instead of matching https?://.
isHttpUrl currently counts malformed strings like https:// as HTTP, which can push entries over the 80% threshold without real HTTP(S) URLs. Parse with new URL(url) and only count matches with http:/https: and a non-empty hostname; add malformed URL cases to the classification tests so they do not increment httpCount.
Proposed fix
function isHttpUrl(url) {
- return typeof url === "string" && /^https?:\/\//i.test(url);
+ if (typeof url !== "string") return false;
+ try {
+ const parsed = new URL(url);
+ return (
+ (parsed.protocol === "http:" || parsed.protocol === "https:") &&
+ parsed.hostname.length > 0
+ );
+ } catch {
+ return false;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function isHttpUrl(url) { | |
| return typeof url === "string" && /^https?:\/\//i.test(url); | |
| } | |
| function isHttpUrl(url) { | |
| if (typeof url !== "string") return false; | |
| try { | |
| const parsed = new URL(url); | |
| return ( | |
| (parsed.protocol === "http:" || parsed.protocol === "https:") && | |
| parsed.hostname.length > 0 | |
| ); | |
| } catch { | |
| return false; | |
| } | |
| } |
📍 Affects 2 files
scripts/validate-ard-urls.mjs#L21-L23(this comment)scripts/tests/validate-ard-urls.test.mjs#L39-L97
🤖 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 `@scripts/validate-ard-urls.mjs` around lines 21 - 23, Update isHttpUrl in
scripts/validate-ard-urls.mjs to parse input with URL, returning true only for
http: or https: protocols with a non-empty hostname and false for malformed
values. In scripts/tests/validate-ard-urls.test.mjs, extend the classification
tests with malformed HTTP(S) URLs and verify they do not increment httpCount.
Comment on lines
+1099
to
+1110
| sync: { | ||
| heading: "discover sync — Synchronize discovered sources", | ||
| lines: [ | ||
| "Usage: agent-harness discover sync", | ||
| "", | ||
| "Fetches and persists data from all configured discovery sources. Uses the", | ||
| "local index when fresh, performs live harvest otherwise.", | ||
| "", | ||
| "Options:", | ||
| " --no-sync Skip source sync (local discovery only)", | ||
| ], | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
--no-sync documented under the wrong subcommand.
helpTexts.sync (Lines 1099-1110) lists --no-sync as an option of discover sync, but the sync case (Lines 201-227) never reads --no-sync — it's actually implemented only on discover full (Line 251). printDiscoverFullHelp's own Options list (Lines 1339-1349) doesn't mention --no-sync at all, so users reading discover full --help won't discover the flag, while discover sync --help advertises a flag that does nothing there.
📝 Suggested fix
sync: {
heading: "discover sync — Synchronize discovered sources",
lines: [
"Usage: agent-harness discover sync",
"",
"Fetches and persists data from all configured discovery sources. Uses the",
"local index when fresh, performs live harvest otherwise.",
- "",
- "Options:",
- " --no-sync Skip source sync (local discovery only)",
],
}, lines: [
"--ai-enrich Run AI enrichment after selection",
"--no-ai-enrich Skip AI enrichment",
+ "--no-sync Skip source sync, using existing local catalog data",
"--quiet Suppress expected source health warnings",Also applies to: 1339-1349
🤖 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 `@src/discover.ts` around lines 1099 - 1110, Move the --no-sync option
documentation from helpTexts.sync to the discover full help text, updating the
Options list used by printDiscoverFullHelp. Keep discover sync help limited to
flags handled by the sync case, and document --no-sync under discover full where
it is implemented.
Comment on lines
+229
to
+260
| const sourceKinds: Array<{ | ||
| kind: SourceKind; | ||
| originUrl: string; | ||
| manifestEntry: string; | ||
| }> = [ | ||
| { | ||
| kind: "repo", | ||
| originUrl: "https://github.com/user/repo/blob/main/SKILL.md", | ||
| manifestEntry: "a94df09f75fba2f11c63103c3e573c729226a6e0", | ||
| }, | ||
| { | ||
| kind: "package-registry", | ||
| originUrl: "https://www.npmjs.com/package/example-pkg", | ||
| manifestEntry: "example-pkg", | ||
| }, | ||
| { | ||
| kind: "registry", | ||
| originUrl: | ||
| "https://raw.githubusercontent.com/official-skills/main/SKILL.md", | ||
| manifestEntry: "b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7a8b9c0d", | ||
| }, | ||
| { | ||
| kind: "local-directory", | ||
| originUrl: "file:///home/user/projects/local-skill.md", | ||
| manifestEntry: "e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f0", | ||
| }, | ||
| { | ||
| kind: "ard-registry", | ||
| originUrl: "https://publisher.example.com/.well-known/ai-catalog.json", | ||
| manifestEntry: "f1e2d3c4b5a6f7e8d9c0b1a2", | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover the remaining source kinds or narrow the test claim.
SourceKind also includes docs, marketplace, and local-manifest, but this “all harvestable source kinds” matrix omits them. Add cases or document why they are non-harvestable.
🤖 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 `@src/tests/ard-catalog.test.ts` around lines 229 - 260, Update the sourceKinds
matrix in the source-kind coverage test to include the omitted docs,
marketplace, and local-manifest SourceKind values with valid originUrl and
manifestEntry fixtures, or explicitly narrow the test’s claim to the currently
covered harvestable kinds and document those exclusions.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
CONTRIBUTING.md:222
- The Markdown table under "### Key directories" is broken: the new "Adding a new host adapter" section is inserted before the table is completed, and the remaining rows are missing the leading/trailing pipes, so the rendered table will be malformed.
| Path | Purpose |
| ------------------------ | -------------------------------------- |
| `src/` | TypeScript source |
| `src/domains/discovery/` | Harvesting, catalog, demand signals |
| `src/host-adapters/` | Per-host wire and extension installers |
docs/guides/ADAPTER-DEVELOPMENT.md:319
- The relative links in this "See Also" list are off by one directory (this file is under docs/guides/), so they currently point to non-existent paths like docs/src/…. These should go up two levels to reach repo-root src/ and CONTRIBUTING.md.
- [Host Adapter Types](../src/host-adapters/types.ts) — full type definitions
- [Registry](../src/host-adapters/registry.ts) — adapter registration
- [Native Config](../src/host-adapters/native-config.ts) — shared config generation
- [Native Wire](../src/host-adapters/native-wire.ts) — wire-in infrastructure
- [V2 CLI Contract](./V2-CONTRACT.md) — host lifecycle model
src/discover.ts:1109
- The discover sync help text advertises a
--no-syncoption, butdiscover syncactually supports--full(and--no-syncis only handled bydiscover full). As-is, the CLI will document a flag that has no effect for this subcommand.
"Options:",
" --no-sync Skip source sync (local discovery only)",
],
docs/cheatsheet.md:29
discover sourcesrefreshes/writes the source index (it doesn’t just list sources). The cheat sheet line is misleading for users trying to understand side effects.
agent-harness discover sources # List enabled discovery sources
scripts/tests/validate-ard-urls.test.mjs:372
- This test relies on the repository’s real
.well-known/ai-catalog.jsonand on the test runner’s current working directory being the repo root. That makes the test brittle (e.g., running tests from a different cwd) and couples it to unrelated catalog content changes.
void test("main uses process.cwd fallback when no cwd option provided", async () => {
// Verify that main() can be called without options — exercises the
// `options.cwd ?? process.cwd()` fallback path at line 86.
// This relies on the project's own .well-known/ai-catalog.json being valid.
const result = main();
scripts/validate-ard-urls.mjs:40
validateArdUrls()returns a different shape whencatalog.entriesis not an array ({ ok, errors, entries }vs{ ok, errors, stats }). This makes the function harder to consume safely and is inconsistent with the success/failure shape used elsewhere in the file.
if (!Array.isArray(catalog.entries)) {
errors.push("ai-catalog.json is missing or has no entries array.");
return { ok: false, errors, entries: 0 };
}
scripts/validate-ard-urls.mjs:76
- The script counts empty-string URLs as "missing" (
if (!url)), but the error text says "no url field" which suggests the property is absent. This can confuse diagnostics when the URL is present but empty.
if (missingCount > 0) {
errors.push(
`${missingCount} entries have no url field — every ARD entry must have a url.`,
);
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/discover.ts:1108
discover synchelp text advertises a--no-syncoption, but the implementation only supports--full(and--no-syncis used bydiscover full). This makesdiscover sync --helpmisleading.
"Options:",
" --no-sync Skip source sync (local discovery only)",
CONTRIBUTING.md:227
- The "Key directories" markdown table is broken: the "Adding a new host adapter" section is inserted mid-table, and the remaining rows are no longer formatted as table rows. This renders incorrectly in GitHub and makes the directory list hard to read.
### Adding a new host adapter
See the [Adapter Development Guide](docs/guides/ADAPTER-DEVELOPMENT.md) for a
step-by-step walkthrough covering registration, lifecycle wiring, capability
docs/guides/ADAPTER-DEVELOPMENT.md:319
- The relative links in the "See Also" section point to
../src/...and../CONTRIBUTING.md, but this file lives underdocs/guides/, so those paths resolve todocs/src/...anddocs/CONTRIBUTING.md(nonexistent). Update them to point to the repo-root paths.
- [Host Adapter Types](../src/host-adapters/types.ts) — full type definitions
- [Registry](../src/host-adapters/registry.ts) — adapter registration
- [Native Config](../src/host-adapters/native-config.ts) — shared config generation
- [Native Wire](../src/host-adapters/native-wire.ts) — wire-in infrastructure
- [V2 CLI Contract](./V2-CONTRACT.md) — host lifecycle model
src/domains/discovery/source-sync/index.ts:226
- The new per-source progress output prints a start marker (
…) and prints a completion line on success, but the catch path doesn’t emit a status/newline. When a source fails (or falls back to stale), subsequent stderr output can end up on the same line as the progress prefix. Consider printingstale/failed+ duration in the catch path too.
} catch (error) {
// Stale-data fallback + persistence tracking.
const previousFailures = (previousState?.consecutiveFailures ?? 0) + 1;
scripts/validate-ard-urls.mjs:40
validateArdUrls()returns a different object shape whencatalog.entriesis missing/not an array ({ ok, errors, entries }), but returns{ ok, errors, stats }in all other cases. This inconsistency makes the function harder to consume safely (and diverges from the shapemain()prints on success). Return the same{ ok, errors, stats }shape for the early error path too.
if (!Array.isArray(catalog.entries)) {
errors.push("ai-catalog.json is missing or has no entries array.");
return { ok: false, errors, entries: 0 };
}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
src/discover.ts:1108
- The discover sync subcommand help advertises
--no-sync, butrunDiscoverimplements--fullfordiscover sync(and--no-syncapplies todiscover full). This makesdiscover sync --helpmisleading.
"Options:",
" --no-sync Skip source sync (local discovery only)",
docs/guides/ADAPTER-DEVELOPMENT.md:317
- The "See Also" relative links point to
../src/...and../CONTRIBUTING.md, but this file lives indocs/guides/, so those paths resolve underdocs/(e.g.docs/src/...) and will 404. These should be../../src/...and../../CONTRIBUTING.md.
## See Also
- [Host Adapter Types](../src/host-adapters/types.ts) — full type definitions
- [Registry](../src/host-adapters/registry.ts) — adapter registration
- [Native Config](../src/host-adapters/native-config.ts) — shared config generation
CONTRIBUTING.md:233
- The "Key directories" Markdown table is broken: the new "Adding a new host adapter" section is inserted mid-table, and the remaining rows are no longer part of a table. This renders incorrectly in CONTRIBUTING.md.
| `src/recommend/` | Selection scoring and report generation |
| `src/mirror/` | Artifact download and provenance |
| `src/install/` | Staged install lifecycle |
| `src/activate.ts` | Runtime root swap and rollback |
| `discover/` | Source config, policy, and seed JSON |
scripts/tests/validate-ard-urls.test.mjs:372
- This test calls
main()with nocwdand implicitly depends on the repo's checked-in.well-known/ai-catalog.jsonbeing present and valid, which makes the suite sensitive to unrelated repo changes and to the process working directory. Prefer making the test self-contained byprocess.chdir()into a temp dir containing a minimal valid catalog, then restoring the previous cwd.
void test("main uses process.cwd fallback when no cwd option provided", async () => {
// Verify that main() can be called without options — exercises the
// `options.cwd ?? process.cwd()` fallback path at line 86.
// This relies on the project's own .well-known/ai-catalog.json being valid.
const result = main();
src/domains/discovery/source-sync/index.ts:226
- On the error path, the per-source progress line printed to stderr ("… ") never gets a completion status/newline, so failures leave a dangling progress line and subsequent stderr output continues on the same line.
} catch (error) {
// Stale-data fallback + persistence tracking.
const previousFailures = (previousState?.consecutiveFailures ?? 0) + 1;
src/discover.ts:252
discover fullnow supports--no-sync, butprintDiscoverFullHelp()does not list it under Options. This makes the new flag undiscoverable viadiscover full --help.
const quietMode = rest.includes("--quiet");
const summaryMode = rest.includes("--summary");
const noSync = rest.includes("--no-sync");
const maxBytesIndex = rest.indexOf("--max-scan-bytes");
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
CONTRIBUTING.md:222
- The "Key directories" markdown table is now broken: the table ends after three rows, then additional table rows appear without a header, because the new "Adding a new host adapter" section was inserted mid-table. This renders poorly and makes the directory map hard to read.
| Path | Purpose |
| ------------------------ | -------------------------------------- |
| `src/` | TypeScript source |
| `src/domains/discovery/` | Harvesting, catalog, demand signals |
| `src/host-adapters/` | Per-host wire and extension installers |
docs/guides/ADAPTER-DEVELOPMENT.md:319
- The relative links in the "See Also" section point to
../src/...and../CONTRIBUTING.md, which are incorrect fromdocs/guides/(they resolve underdocs/). These links will be broken on GitHub.
- [Host Adapter Types](../src/host-adapters/types.ts) — full type definitions
- [Registry](../src/host-adapters/registry.ts) — adapter registration
- [Native Config](../src/host-adapters/native-config.ts) — shared config generation
- [Native Wire](../src/host-adapters/native-wire.ts) — wire-in infrastructure
- [V2 CLI Contract](./V2-CONTRACT.md) — host lifecycle model
src/discover.ts:1109
discover synchelp advertises a--no-syncoption, but the command implementation only recognizes--full(see theforceFullFlagcheck) and otherwise either uses the fresh local index or runs a sync. This makes the CLI help misleading.
"Options:",
" --no-sync Skip source sync (local discovery only)",
],
src/domains/discovery/source-sync/index.ts:226
- In the error path, the per-source progress prefix (
[discover sync] i/n …) is printed before the try/catch, but the catch block never writes a completion status + newline. This leaves a hanging progress line whenever a source fails or falls back to stale data.
} catch (error) {
// Stale-data fallback + persistence tracking.
const previousFailures = (previousState?.consecutiveFailures ?? 0) + 1;
scripts/tests/validate-ard-urls.test.mjs:372
- This test depends on the repository's checked-in
.well-known/ai-catalog.jsonbeing present and meeting the 80% threshold. That makes the test suite non-hermetic and likely to become flaky as the repo catalog changes or in contexts where the file isn't present.
void test("main uses process.cwd fallback when no cwd option provided", async () => {
// Verify that main() can be called without options — exercises the
// `options.cwd ?? process.cwd()` fallback path at line 86.
// This relies on the project's own .well-known/ai-catalog.json being valid.
const result = main();
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/tests/cli-help.test.ts`:
- Around line 493-506: Update the runBuiltCli result destructuring in the
discover full --no-sync test to remove the nonexistent exitCode property and
delete the associated exitCode assertion. Keep the stdout/stderr warning
assertion unchanged; successful completion of runBuiltCli already verifies the
command exited successfully.
🪄 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: CHILL
Plan: Pro Plus
Run ID: b4bdcdd6-a475-4e53-871a-280fd44efced
📒 Files selected for processing (5)
scripts/validate-ard-urls.mjssrc/domains/discovery/source-sync.tssrc/domains/discovery/source-sync/index.tssrc/tests/cli-help.test.tssrc/tests/source-sync-internals.test.ts
💤 Files with no reviewable changes (1)
- scripts/validate-ard-urls.mjs
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/discover.ts:1108
- The discover sync help text documents a
--no-syncoption, butdiscover syncactually supports--full(and otherwise uses the fresh local index). This makes--helpmisleading for users.
"Options:",
" --no-sync Skip source sync (local discovery only)",
src/domains/discovery/source-sync/index.ts:227
- When per-source progress is printed to stderr, an exception path leaves the progress line unterminated (no status/duration/newline). This will garble output for the remainder of the sync.
} catch (error) {
// Stale-data fallback + persistence tracking.
const previousFailures = (previousState?.consecutiveFailures ?? 0) + 1;
const errorMessage = getErrorMessage(error);
CONTRIBUTING.md:223
- The “Key directories” Markdown table is broken: the table ends after 3 rows, and the remaining rows are no longer part of the table (they appear as plain text with stray pipes). This makes the CONTRIBUTING docs hard to read/render correctly.
| Path | Purpose |
| ------------------------ | -------------------------------------- |
| `src/` | TypeScript source |
| `src/domains/discovery/` | Harvesting, catalog, demand signals |
| `src/host-adapters/` | Per-host wire and extension installers |
docs/guides/ADAPTER-DEVELOPMENT.md:320
- These “See Also” links are off by one directory. From
docs/guides/, the repo root is../.., so../src/...and../CONTRIBUTING.mdresolve to non-existent paths.
- [Registry](../src/host-adapters/registry.ts) — adapter registration
- [Native Config](../src/host-adapters/native-config.ts) — shared config generation
- [Native Wire](../src/host-adapters/native-wire.ts) — wire-in infrastructure
- [V2 CLI Contract](./V2-CONTRACT.md) — host lifecycle model
- [Contributing Guide](../CONTRIBUTING.md) — contribution workflow
scripts/validate-ard-urls.mjs:40
validateArdUrlsreturns a different result shape whenentriesis not an array ({ entries: 0 }instead of astatsobject). This makes the function’s return type inconsistent for callers/tests and complicates error reporting.
if (!Array.isArray(catalog.entries)) {
errors.push("ai-catalog.json is missing or has no entries array.");
return { ok: false, errors, entries: 0 };
}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
src/discover.ts:1108
- The
discover synchelp text advertises--no-sync, but the implementation uses--full(seeconst forceFullFlag = rest.includes("--full")). This is user-facing CLI help drift and will mislead users.
"Fetches and persists data from all configured discovery sources. Uses the",
"local index when fresh, performs live harvest otherwise.",
"",
"Options:",
" --no-sync Skip source sync (local discovery only)",
docs/guides/ADAPTER-DEVELOPMENT.md:319
- The “See Also” links use incorrect relative paths (e.g.
../src/...resolves underdocs/, not repo root). As written, these links will 404 in GitHub rendering.
- [Host Adapter Types](../src/host-adapters/types.ts) — full type definitions
- [Registry](../src/host-adapters/registry.ts) — adapter registration
- [Native Config](../src/host-adapters/native-config.ts) — shared config generation
- [Native Wire](../src/host-adapters/native-wire.ts) — wire-in infrastructure
- [V2 CLI Contract](./V2-CONTRACT.md) — host lifecycle model
CONTRIBUTING.md:233
- The “Key directories” Markdown table is broken: new “Adding a new host adapter” text was inserted mid-table, and the subsequent rows lost their leading
|, so the rendered docs will be malformed.
| `src/recommend/` | Selection scoring and report generation |
| `src/mirror/` | Artifact download and provenance |
| `src/install/` | Staged install lifecycle |
| `src/activate.ts` | Runtime root swap and rollback |
| `discover/` | Source config, policy, and seed JSON |
scripts/tests/validate-ard-urls.test.mjs:372
- This test is not hermetic: it depends on the repository’s real
.well-known/ai-catalog.jsonbeing present/valid in the current working directory. That makes the suite brittle (e.g. when tests run from a different cwd or in partial checkouts).
void test("main uses process.cwd fallback when no cwd option provided", async () => {
// Verify that main() can be called without options — exercises the
// `options.cwd ?? process.cwd()` fallback path at line 86.
// This relies on the project's own .well-known/ai-catalog.json being valid.
const result = main();
src/domains/discovery/source-sync/index.ts:161
- Per-source progress starts with
process.stderr.write(${progressLabel} …)(no newline). If a source throws during sync, the current code path doesn’t guarantee writing a terminating status/newline, so the terminal can be left with a dangling partial progress line.
// Use process.stderr so progress lines don't contaminate stdout when
// the CLI is invoked in JSON-output mode.
if (totalSources > 1) {
process.stderr.write(`${progressLabel} … `);
}
docs/cheatsheet.md:54
mirror locksrunsgenerateBundleLocks(it generates/writes lock files), so describing it as “List lock files” is inaccurate and could mislead users about side effects.
agent-harness mirror plan --host <host> # Preview mirror plan
agent-harness mirror acquire --host <host> # Download selected assets
agent-harness mirror diff # Compare local vs remote mirrors
agent-harness mirror locks # List lock files
agent-harness mirror explain --asset <id> # Mirror provenance for an asset
src/cli.ts:171
- The help-routing comment here still describes substituting mutating-domain subcommands with
help, but the code now preserves the original subcommand and propagates--helpso domains can emit subcommand-specific help. Updating this comment will prevent confusion for future maintainers.
// generic help without executing any phase.
if (nonFlagArgs.length >= 2) {
const [domain, subcommand, ...extra] = nonFlagArgs;
// Preserve the original subcommand so domain handlers can show
// subcommand-specific help. Mutating domains receive the subcommand +
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/discover.ts:1108
- The
discover synchelp text advertises a--no-syncoption, but the implementation for thesyncsubcommand only supports--full(to force a re-harvest when the local catalog index is fresh). This makesdiscover sync --helpmisleading.
"Fetches and persists data from all configured discovery sources. Uses the",
"local index when fresh, performs live harvest otherwise.",
"",
"Options:",
" --no-sync Skip source sync (local discovery only)",
CONTRIBUTING.md:223
- The "Key directories" Markdown table is broken: it stops after three rows, then a heading is inserted mid-table, and the remaining rows are no longer part of a valid table. This will render incorrectly in GitHub and makes the directory overview hard to read.
| Path | Purpose |
| ------------------------ | -------------------------------------- |
| `src/` | TypeScript source |
| `src/domains/discovery/` | Harvesting, catalog, demand signals |
| `src/host-adapters/` | Per-host wire and extension installers |
docs/guides/ADAPTER-DEVELOPMENT.md:320
- The relative links in the "See Also" section point to
../src/...and../CONTRIBUTING.md, but this file is underdocs/guides/, so../srcresolves todocs/src(nonexistent). These links will be broken on GitHub.
- [Registry](../src/host-adapters/registry.ts) — adapter registration
- [Native Config](../src/host-adapters/native-config.ts) — shared config generation
- [Native Wire](../src/host-adapters/native-wire.ts) — wire-in infrastructure
- [V2 CLI Contract](./V2-CONTRACT.md) — host lifecycle model
- [Contributing Guide](../CONTRIBUTING.md) — contribution workflow
src/domains/discovery/source-sync/index.ts:226
- Per-source progress prints a prefix like
[discover sync] 1/… …before syncing, but the error path doesn’t print a completion status/newline. If a source throws, the progress line is left hanging and the next source’s progress output will be appended on the same line, making logs hard to read.
} catch (error) {
// Stale-data fallback + persistence tracking.
const previousFailures = (previousState?.consecutiveFailures ?? 0) + 1;
src/workspace.ts:51
workspace <target> --helpis checked after parsing--ai-enrich/--no-ai-enrichflags. BecauseparseAiEnrichmentFlags()throws on conflicting flags, a command likeagent-harness workspace opencode --help --ai-enrich --no-ai-enrichwill error instead of printing help. Help handling should short-circuit before any option parsing/validation that can throw.
const aiEnrichmentFlags = parseAiEnrichmentFlags(rest);
// Detect --help flag and show target-specific or parent help (#383).
if (rest.includes("--help") || rest.includes("-h")) {
printWorkspaceSubcommandHelp(target);
return 0;
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves all 16 open tickets blocking the v2.0.0 release. 14 code/doc tickets implemented, 2 post-merge tasks (git tag, GitHub release) pending.
Tickets Resolved
P0 (blocking)
P2 (important)
--no-syncflagP3 (polish)
Post-merge (cannot be done on branch)
Verification
Commits
6 focused commits, one per logical change group.
Summary by CodeRabbit
New Features
--help/-hfor many CLI subcommands (activation, discover, install, mirror, quarantine, rebuild, setup, wire, workspace).discover full --no-syncto skip syncing and use local catalog data.Documentation
Validation
Chores