fix(docs): async install docs generation + home-repo guard#314
Conversation
`hivemind install` inside a git repo used to delegate to an inline `docs
sync`, which generated the wiki in the FOREGROUND — minutes of per-page
`claude -p` calls hung install on any non-trivial repo. Mirror the async half
of `hivemind graph init` instead: build the graph inline (fast, no LLM), run
the onboarding, then spawn `docs wiki` DETACHED so install returns immediately
("Generating wiki docs in the background — check with: hivemind docs list").
Also add a home-repo guard: when the resolved git root is $HOME (a dotfiles
repo makes `git rev-parse --show-toplevel` resolve to ~ from any subdir), do
not offer to document the whole home — fall back to the hint. The guard lives
in shouldPromptDocsSetup (unit-tested).
|
Warning Review limit reached
Next review available in: 13 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughInstall-time docs onboarding now delegates to an injected workflow, suppresses prompts for home-directory repositories, handles configuration and consent outcomes, builds documentation state, and can launch a detached wiki worker. ChangesDocs install onboarding
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant runInstallAll
participant runInstallDocsOnboarding
participant GitRootProbe
participant DocsOnboarding
participant DetachedWorker
runInstallAll->>runInstallDocsOnboarding: pass injected install capabilities
runInstallDocsOnboarding->>GitRootProbe: resolve repository top-level
GitRootProbe-->>runInstallDocsOnboarding: return repository root
runInstallDocsOnboarding->>DocsOnboarding: build graph and run consent flow
DocsOnboarding-->>runInstallDocsOnboarding: return onboarding result
runInstallDocsOnboarding->>DetachedWorker: spawn detached docs wiki
Possibly related PRs
🚥 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 |
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 3 files changed
Generated for commit c0c8a23. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/cli/index.ts`:
- Around line 418-424: Update the home-directory comparison in the
shouldPromptDocsSetup configuration to normalize both repoRoot and homedir()
with node:path resolve() before checking equality, while preserving the existing
atHome behavior on non-Windows platforms.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 34960e57-fd4d-4930-949f-f1b79fbfc6ac
📒 Files selected for processing (3)
src/cli/index.tssrc/docs/install-hint.tstests/shared/docs-install-hint.test.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/cli/index.ts (1)
423-432: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant re-import of
deriveProjectKeyinside the nestedonboardclosure.It's already imported and in scope at Line 407 for
autoEnabled; the closure can reuse that binding instead of dynamically importing it again.♻️ Proposed simplification
onboard: async ({ root, orgId, orgName }) => { const { runDocsOnboarding } = await import("../docs/onboarding.js"); - const { deriveProjectKey } = await import("../utils/repo-identity.js"); const { loadCurrentSnapshot } = await import("../graph/load-current.js"); return runDocsOnboarding({🤖 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/index.ts` around lines 423 - 432, Remove the nested dynamic import of deriveProjectKey from the onboard closure and reuse the existing in-scope binding already imported for autoEnabled. Keep the remaining onboarding imports and project key derivation behavior unchanged.src/docs/install-docs.ts (1)
8-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocstring is stale: only mentions spawning
docs wiki, not the seconddocs refresh --fullworker.Lines 91-93 now spawn both tiers detached (per PR objectives), but the file-level comment still describes only 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/docs/install-docs.ts` around lines 8 - 10, Update the file-level installation flow docstring to document both detached workers spawned after consent: docs wiki and docs refresh --full. Keep the existing graph-building and onboarding description, and ensure the comment reflects that LLM generation remains non-blocking.
🤖 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/docs/install-docs.ts`:
- Line 18: Resolve the stale isHomeRoot import in install-docs.ts by either
exporting a matching helper from install-hint.ts or replacing the import and its
usages with the existing intended home-directory check. Ensure the resulting
symbols compile and preserve the current documentation setup prompt behavior.
In `@tests/shared/install-docs.test.ts`:
- Line 121: Update the warning assertion in the install-docs test to compare
against the exact deterministic message “docs setup skipped: graph boom” instead
of using expect.stringContaining, while preserving the existing warn mock
verification.
- Around line 74-81: Update the “home-repo guard is slash-insensitive” test to
assert the deterministic POSIX result: expect action.kind to be "spawned" rather
than accepting both "hint" and "spawned". Keep the existing harness setup and
onboarding call unchanged.
---
Nitpick comments:
In `@src/cli/index.ts`:
- Around line 423-432: Remove the nested dynamic import of deriveProjectKey from
the onboard closure and reuse the existing in-scope binding already imported for
autoEnabled. Keep the remaining onboarding imports and project key derivation
behavior unchanged.
In `@src/docs/install-docs.ts`:
- Around line 8-10: Update the file-level installation flow docstring to
document both detached workers spawned after consent: docs wiki and docs refresh
--full. Keep the existing graph-building and onboarding description, and ensure
the comment reflects that LLM generation remains non-blocking.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2f85f49a-4c47-424b-8777-67deec45ff7e
📒 Files selected for processing (3)
src/cli/index.tssrc/docs/install-docs.tstests/shared/install-docs.test.ts
9ab9700 to
6c9c791
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/docs/install-docs.ts (1)
18-18: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix the stale
isHomeRootimport.The file imports
isHomeRoot, butsrc/docs/install-hint.tsdoes not export it, which will cause a build failure. Either export the helper frominstall-hint.tsor replace this import and its usage with the intended home-directory check.🤖 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/docs/install-docs.ts` at line 18, Fix the stale isHomeRoot import in install-docs.ts by either exporting isHomeRoot from install-hint.ts or replacing the import and all usages with the intended home-directory check. Ensure the resulting symbols match the actual exports and the build succeeds.Source: Linters/SAST tools
🤖 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.
Duplicate comments:
In `@src/docs/install-docs.ts`:
- Line 18: Fix the stale isHomeRoot import in install-docs.ts by either
exporting isHomeRoot from install-hint.ts or replacing the import and all usages
with the intended home-directory check. Ensure the resulting symbols match the
actual exports and the build succeeds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2455e782-bc9f-441a-a615-f0d5c32b4a71
📒 Files selected for processing (3)
src/cli/index.tssrc/docs/install-docs.tstests/shared/install-docs.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/shared/install-docs.test.ts
- src/cli/index.ts
6c9c791 to
9e19a29
Compare
Refinements on the install docs step (last pass guided by codex review): - Skip the redundant "Generate docs?" prompt when auto-sync is already enabled; still run the graph build so its post-build auto-refresh fills an auto-enabled-but-EMPTY corpus (codex: a bare short-circuit never generates). - On consent, spawn only `docs wiki` DETACHED — same as `hivemind graph init`. (An earlier attempt also spawned `docs refresh --full` for per-file docs, but codex correctly noted refresh only updates existing rows and never generates missing per-file docs — that's the heavy `docs generate`, left to auto-sync.) All via the injected seams, unit-tested.
9e19a29 to
5e62f8d
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/docs/install-hint.ts (1)
39-48: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider case-insensitive path comparison on Windows.
While
path.resolvenormalizes slashes and resolves..segments, it does not normalize the casing of drive letters or directory names on Windows. Git tools (like MSYS2 or Git Bash) sometimes return paths with lowercase drive letters (e.g.,c:/Users/x), whileos.homedir()might returnC:\Users\x. A strict===will fail to match these equivalent paths.Consider comparing the paths case-insensitively on Windows, or at least normalizing the case before comparison.
♻️ Proposed refactor for case-insensitive comparison
export function isHomeRoot(gitRoot: string, home: string): boolean { - return resolve(gitRoot) === resolve(home); + const resGit = resolve(gitRoot); + const resHome = resolve(home); + return process.platform === "win32" + ? resGit.toLowerCase() === resHome.toLowerCase() + : resGit === resHome; }🤖 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/docs/install-hint.ts` around lines 39 - 48, Update isHomeRoot to compare resolved paths case-insensitively when running on Windows, while preserving the existing case-sensitive comparison on other platforms. Normalize both resolve(gitRoot) and resolve(home) consistently before equality checking.src/cli/index.ts (1)
425-425: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant import.
deriveProjectKeyis already dynamically imported in the outer scope at line 407. You can safely remove this duplicate inner import.♻️ Proposed refactor
- const { deriveProjectKey } = await import("../utils/repo-identity.js");🤖 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/index.ts` at line 425, Remove the redundant inner dynamic import of deriveProjectKey in the shown CLI flow, and reuse the existing outer-scope import declared earlier. Keep the deriveProjectKey usage unchanged.
🤖 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.
Nitpick comments:
In `@src/cli/index.ts`:
- Line 425: Remove the redundant inner dynamic import of deriveProjectKey in the
shown CLI flow, and reuse the existing outer-scope import declared earlier. Keep
the deriveProjectKey usage unchanged.
In `@src/docs/install-hint.ts`:
- Around line 39-48: Update isHomeRoot to compare resolved paths
case-insensitively when running on Windows, while preserving the existing
case-sensitive comparison on other platforms. Normalize both resolve(gitRoot)
and resolve(home) consistently before equality checking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 56d13eb2-3720-4891-ba8c-0bc37def6c84
📒 Files selected for processing (5)
src/cli/index.tssrc/docs/install-docs.tssrc/docs/install-hint.tstests/shared/docs-install-hint.test.tstests/shared/install-docs.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/docs/install-docs.ts
- tests/shared/install-docs.test.ts
1 participant
Fix the install-time docs footgun (async + home guard)
Follow-up to #305. A real from-0 install surfaced two problems; this fixes both without reverting the feature.
1. Install no longer blocks on the LLM
hivemind installin a repo delegated to an inlinedocs sync, which generated the wiki in the foreground — minutes of per-pageclaude -pcalls hung install. Now it mirrors the async half ofhivemind graph init:docs wikidetached → install returns immediately with "Generating wiki docs in the background — check with: hivemind docs list".The detached-spawn block is the same shape
graph initalready ships (spawnDetachedNodeWorker(["docs","wiki",…])).2. Home-repo guard
If the resolved git root is
$HOME(a dotfiles repo makesgit rev-parse --show-toplevelresolve to~from any subdirectory), install no longer offers to document the whole home — it falls back to the hint. The guard lives inshouldPromptDocsSetup(atHome), unit-tested.Verification
tscclean;shouldPromptDocsSetupunit tests cover the home guard (7 pass).docs wiki, prints the background message, and no longer runs inlinedocs sync.~) and reaches the onboarding; interactive multi-prompt click-through over a piped pty is flaky in CI harnesses, so the async generation is verified via graph-init parity + bundle-level checks.Files
src/cli/index.ts— async delegation (build inline + onboarding + detached wiki)src/docs/install-hint.ts—shouldPromptDocsSetuphome guardtests/shared/docs-install-hint.test.ts— home-guard testsSession Context
Session transcript
Summary by CodeRabbit