docs: align verify commands with CI, fix doc drift, add architecture guides#900
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds contributor docs and architecture references, updates root guidance to current lint/typecheck/test and Rust verification commands, and refreshes security/community wording. The new ChangesContributor docs, architecture guides, and quality bar update
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
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 `@docs/architecture/ai-subsystem.md`:
- Line 71: The mutating-tools approval list in the architecture docs includes a
stale shell tool name; update the list to match the current shell tool surface
by removing bash_session_run and keeping the shell-related entries aligned with
src/modules/ai/tools/shell.ts, especially the bash_run and bash_background tool
names referenced in the approval flow description.
🪄 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: 299fa4ee-fda7-4b5a-9592-3fe507777e2d
📒 Files selected for processing (13)
.github/PULL_REQUEST_TEMPLATE.mdCODE_OF_CONDUCT.mdCONTRIBUTING.mdREADME.mdSECURITY.mdTERAX.mddocs/README.mddocs/architecture/ai-subsystem.mddocs/architecture/pty-shell-integration.mddocs/architecture/security-model.mddocs/architecture/terminal-renderer-pool.mddocs/architecture/two-process-model.mddocs/contributing/testing.md
4b77da7 to
538f95d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/architecture/ai-subsystem.md`:
- Line 70: The read-only tools list is outdated and still references the old
grep/glob names. Update the documentation text in the architecture section to
use the current tool names `fs_search`, `fs_grep`, and `fs_glob` alongside
`read_file` and `list_directory`, so the tool list matches the actual APIs.
In `@docs/architecture/security-model.md`:
- Around line 44-48: Update the approval policy description in the security
model docs to match the current tool surface in src/modules/ai/tools/tools.ts.
Remove the stale read-only entries for grep and glob if they are no longer
exposed, and revalidate the mutating tools list against the actual tool
definitions so the documented needsApproval set matches the source of truth. Use
the tool names from tools.ts and keep the read-only vs mutating approval
behavior consistent with the implementation.
🪄 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: 70db8c7b-b750-424f-937c-4bc1113a5e4b
📒 Files selected for processing (13)
.github/PULL_REQUEST_TEMPLATE.mdCODE_OF_CONDUCT.mdCONTRIBUTING.mdREADME.mdSECURITY.mdTERAX.mddocs/README.mddocs/architecture/ai-subsystem.mddocs/architecture/pty-shell-integration.mddocs/architecture/security-model.mddocs/architecture/terminal-renderer-pool.mddocs/architecture/two-process-model.mddocs/contributing/testing.md
✅ Files skipped from review due to trivial changes (9)
- docs/README.md
- docs/architecture/two-process-model.md
- .github/PULL_REQUEST_TEMPLATE.md
- SECURITY.md
- docs/architecture/pty-shell-integration.md
- CONTRIBUTING.md
- CODE_OF_CONDUCT.md
- docs/architecture/terminal-renderer-pool.md
- TERAX.md
|
Thanks for this, really solid contribution. The provider list and CI-command drift were both real, and the new architecture docs are grounded in the actual code, not filler. Rebased on main and dropped a stale tool name CodeRabbit caught. Merged. |
What
This PR sets up proper in-repo contributor documentation and fixes existing doc drift:
TERAX.md,README.md,CONTRIBUTING.md, and the PR template to match what CI actually runs (.github/workflows/ci.yml).TERAX.mddrift from the current codebase:command-palette/andspaces/modules.0.5.xversion pin inSECURITY.md.docs/tree with long-form contributor guides:docs/README.md- indexdocs/architecture/two-process-model.md- IPC boundary and command referencedocs/architecture/pty-shell-integration.md- PTY, shell init, OSC, ConPTY, Job Objectdocs/architecture/security-model.md- consolidated security boundariesdocs/architecture/ai-subsystem.md- AI stack + "Adding a provider" walkthroughdocs/architecture/terminal-renderer-pool.md- renderer pool and DormantRing invariantsdocs/contributing/testing.md- testing contract and core-subsystem invariantsWhy
The project already had a strong
README.md,CONTRIBUTING.md, andTERAX.md, but:TERAX.mdhad drifted fromconfig.ts(providers),shell_init.rs(fish), and the actual module tree.docs/only held screenshots.How
TERAX.mdremains the source of truth. Each new guide starts with "If anything conflicts withTERAX.md,TERAX.mdwins."TERAX.mdand thedocs/README.mdindex.Testing
pnpm check-typescleanpnpm test- 245 tests passed; one pre-existing suite failure insrc/app/eager-budget.test.ts(SyntaxError importing.mjs), unrelated to docspnpm lint- passed with pre-existing warnings (biome does not lint.md)cargo clippy --all-targets --locked -- -D warnings- could not run;cargois not installed in this environmentcargo test --locked/cargo nextest run --locked- could not run;cargois not installed in this environment**/*.mdreturns zero matchesNotes for reviewer
.gitignorechange adding.codegraph/and the untrackedopencode.json/.opencode/files were left out of this PR because they are not part of the documentation work.Summary by CodeRabbit