-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add AC CLI documentation and update script #195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds extensive ACP CLI documentation under docs/en/ui/cli_tools, introduces a Node.js script to sync AC manual content from a remote repo, updates package.json to include a script invoking it, adds a violet CLI page, adjusts overview index pages, and updates .gitignore to ignore local/*. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant NPM as npm/yarn
participant Script as update-ac-manual.js
participant Git as git CLI
participant FS as Filesystem
Dev->>NPM: run "update-ac-manual"
NPM->>Script: node scripts/update-ac-manual.js main
Script->>Script: Preflight checks (git present)
Script->>Git: git clone <repo> --branch main --depth=1
alt Clone ok
Script->>Script: Validate manual source directory
alt Manual dir present
Script->>FS: Ensure target docs dir exists
Script->>FS: Clean non-index files in target
loop For each source file
Script->>Script: Derive target name (strip numeric prefixes for .md/.mdx)
Script->>FS: Copy file/dir to target
end
Script->>FS: Remove temporary clone directory
Script-->>Dev: Log success
else Manual dir missing
Script-->>Dev: Error: manual source not found
Script->>FS: Cleanup temp dir
end
else Clone fails
Script-->>Dev: Error: auth/timeout/other
Script->>FS: Cleanup temp dir
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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 |
- Add comprehensive AC CLI documentation files to docs/en/ui/cli_tools/ac/ - getting-started.md: installation and first steps guide - configuration.md: shell completion and kubeconfig setup - ac-and-kubectl-usage.md: comparison and usage patterns - managing-cli-profiles.md: profile and session management - extending-with-plugins.md: plugin development guide - developer-command-reference.md: developer command reference - administrator-command-reference.md: admin command reference - Add update-ac-manual script to automate documentation updates - Update package.json with update-ac-manual script - Update .gitignore to exclude local/* directory - Update UI tools index and add violet.mdx placeholder
89a024a
to
4c9b572
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (21)
docs/en/ui/cli_tools/ac/index.mdx (1)
7-7
: Populate Overview or add a short intro.Empty overviewHeaders render a blank page. Add a one‑sentence intro or pass headers to Overview.
.gitignore (1)
3-3
: Anchor ignore to repo root to avoid overmatching nested “local/”.
local/*
will ignore any folder named “local” at any depth. Prefer root‑anchored.Apply this diff:
-local/* +/local/docs/en/ui/cli_tools/index.mdx (1)
7-7
: Add a brief section intro above Overview.A one‑liner improves discoverability (e.g., “Browse the available command‑line tools for ACP.”).
docs/en/ui/cli_tools/ac/ac-and-kubectl-usage.md (1)
11-11
: Soften equivalence claim with kubectl.“The ac binary offers the same capabilities as kubectl” is a strong claim and easy to go stale. Prefer “kubectl‑compatible for core commands and adds ACP features”.
Apply this diff:
-The ac binary offers the same capabilities as the kubectl binary, but extends to natively support additional ACP platform features, including: +The ac binary is kubectl‑compatible for core Kubernetes commands and adds native ACP platform features, including:docs/en/ui/cli_tools/ac/extending-with-plugins.md (2)
17-17
: Use a heading instead of bold for “Procedure”.Fix MD036 (emphasis as heading).
Apply this diff:
-**Procedure** +### Procedure
92-99
: Specify a language for the fenced “Example output” block.Fix MD040 by marking as text.
Apply this diff:
-``` +```text The following compatible plugins are available: /usr/local/bin/<plugin_file></blockquote></details> <details> <summary>docs/en/ui/cli_tools/ac/developer-command-reference.md (2)</summary><blockquote> `5-7`: **Unify naming: “AC CLI” → “ACP CLI”.** Consistent with the rest of the docs and product name. Apply this diff: ```diff -# AC CLI Developer Command Reference +# ACP CLI Developer Command Reference
1383-1398
: Avoid password in CLI examples; prefer interactive or env vars.Passing
--password
leaks secrets via shell history. Show a safer example and add a caution.Apply this diff:
-# Login with all parameters via flags -ac login https://example.com --name prod --username=myuser --password=mypassword +# Login with parameters (avoid --password; use interactive prompt or env vars) +ac login https://example.com --name prod --username=myuser + +> Note: Avoid --password to prevent leaking secrets via shell history. Use interactive prompts or AC_LOGIN_* environment variables.docs/en/ui/cli_tools/ac/getting-started.md (2)
14-15
: Fix merged bullet; split into two items.Minor wording/formatting issue.
Apply this diff:
-- Working directly with project source code Scripting ACP platform operations and automating workflows +- Working directly with project source code +- Scripting ACP platform operations and automating workflows
291-295
: Tighten logout bullet phrasing.Small grammar fix.
Apply this diff:
-- Revoke currently used tokens in the ACP +- Revokes any active tokens in ACPscripts/update-ac-manual.js (5)
17-23
: Consider adding more specific error informationThe error message could be more helpful by including the actual command output or error details to aid debugging.
async function checkGitAvailable() { try { execSync('git --version', { stdio: 'ignore' }); } catch (error) { - throw new Error('Git is not available. Please install git first.'); + throw new Error(`Git is not available. Please install git first. ${error.message ? `(${error.message})` : ''}`); } }
44-48
: Security concern: Consider timeout increase for large repositoriesThe 60-second timeout might be insufficient for large repositories or slow network connections. Consider making this configurable or increasing the default.
try { execSync(`git clone --depth=1 --branch=${branch} ${AC_REPO_URL} ${TEMP_DIR}`, { stdio: 'inherit', - timeout: 60000 // 60 second timeout + timeout: 120000 // 120 second timeout });
50-56
: Improve error detection logicThe error detection relies on string matching which could be fragile. The
error.message
property might not contain the expected strings depending on the execution context.} catch (error) { - if (error.message.includes('Authentication failed')) { + if (error.message?.includes('Authentication failed') || error.message?.includes('could not read Username')) { throw new Error(`Authentication failed for ${AC_REPO_URL}. Please ensure you have access to the repository and your git credentials are configured properly.`); - } else if (error.message.includes('timeout')) { + } else if (error.code === 'ETIMEDOUT' || error.message?.includes('timeout')) { throw new Error(`Git clone operation timed out. Please check your network connection and try again.`); } else { throw new Error(`Failed to clone repository: ${error.message}`); } }
119-124
: Potential data loss: Consider backing up existing filesThe script removes existing files without creating backups. Consider adding a backup mechanism or at least a dry-run option to preview changes.
// Remove existing files in target directory (except index.mdx) try { const existingFiles = await fs.readdir(AC_DOCS_TARGET); for (const file of existingFiles) { if (file !== 'index.mdx') { + // Consider creating a backup directory + // const backupPath = join(AC_DOCS_TARGET, '.backup', new Date().toISOString()); + // await fs.mkdir(backupPath, { recursive: true }); + // await fs.rename(join(AC_DOCS_TARGET, file), join(backupPath, file)); await fs.rm(join(AC_DOCS_TARGET, file), { recursive: true, force: true }); console.log(`Removed existing file: ${file}`); } }
195-197
: Improve cross-platform compatibility checkThe current approach for detecting direct script execution may not work correctly on Windows systems where
process.argv[1]
might have different path separators.// Run the script if called directly -if (import.meta.url === `file://${process.argv[1]}`) { +if (import.meta.url === pathToFileURL(process.argv[1]).href) { main(); }Additionally, import
pathToFileURL
at the top:-import { fileURLToPath } from 'url'; +import { fileURLToPath, pathToFileURL } from 'url';package.json (1)
14-14
: Consider parameterizing the branch argumentThe script hardcodes the
main
branch. Consider making this configurable via npm script arguments or environment variables for better flexibility.- "update-ac-manual": "node scripts/update-ac-manual.js main" + "update-ac-manual": "node scripts/update-ac-manual.js"Then users can specify the branch:
npm run update-ac-manual -- feature-branch
docs/en/ui/cli_tools/ac/administrator-command-reference.md (1)
20-29
: Add language identifiers to fenced code blocksAll command-line code blocks should specify
bash
orsh
as the language identifier for proper syntax highlighting.For example, update line 20:
-``` +```bash # Drain a node for maintenance ac adm drain NODE_NAMEApply similar changes to lines 41, 52, 63, 74, 88, and 112.
Also applies to: 41-44, 52-55, 63-66, 74-80, 88-104, 112-115
docs/en/ui/cli_tools/ac/managing-cli-profiles.md (2)
59-64
: Add language identifier to output code blocksThe output examples should use the
text
orplaintext
language identifier to differentiate them from executable commands.Line 59:
-``` +```text CURRENT SESSION PLATFORM USER CLUSTERSLine 110:
-``` +```text You are currently in namespace "my-app-dev".Also applies to: 110-116
190-190
: Security: Remove or mask JWT token in exampleThe example contains what appears to be a partial JWT token. Even though it's truncated, it's best practice to use obviously fake examples like
<TOKEN>
orxxx...
to avoid any confusion.- token: eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9... + token: <JWT_TOKEN_HERE>docs/en/ui/cli_tools/ac/configuration.md (2)
24-24
: Consider consistent command prompt styleThe documentation uses
$
prompts inconsistently. Either show them with expected output or remove them entirely for consistency across the documentation.Either remove the
$
prompts:- $ ac completion bash > ac_bash_completion + ac completion bash > ac_bash_completionOr show the expected output where relevant.
Also applies to: 29-29, 73-73, 87-87, 92-92, 97-97
76-76
: Format bare URL properlyThe bare URL in the example should be wrapped in angle brackets or formatted as inline code.
- - `<platform-url>`: The base URL of the ACP platform (e.g., https://acp.prod.example.com) + - `<platform-url>`: The base URL of the ACP platform (e.g., `https://acp.prod.example.com`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.gitignore
(1 hunks)docs/en/ui/cli_tools/ac/ac-and-kubectl-usage.md
(1 hunks)docs/en/ui/cli_tools/ac/administrator-command-reference.md
(1 hunks)docs/en/ui/cli_tools/ac/configuration.md
(1 hunks)docs/en/ui/cli_tools/ac/developer-command-reference.md
(1 hunks)docs/en/ui/cli_tools/ac/extending-with-plugins.md
(1 hunks)docs/en/ui/cli_tools/ac/getting-started.md
(1 hunks)docs/en/ui/cli_tools/ac/index.mdx
(1 hunks)docs/en/ui/cli_tools/ac/managing-cli-profiles.md
(1 hunks)docs/en/ui/cli_tools/index.mdx
(1 hunks)docs/en/ui/cli_tools/violet.mdx
(1 hunks)package.json
(1 hunks)scripts/update-ac-manual.js
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/en/ui/cli_tools/ac/configuration.md
24-24: Dollar signs used before commands without showing output
(MD014, commands-show-output)
29-29: Dollar signs used before commands without showing output
(MD014, commands-show-output)
73-73: Dollar signs used before commands without showing output
(MD014, commands-show-output)
76-76: Bare URL used
(MD034, no-bare-urls)
87-87: Dollar signs used before commands without showing output
(MD014, commands-show-output)
92-92: Dollar signs used before commands without showing output
(MD014, commands-show-output)
97-97: Dollar signs used before commands without showing output
(MD014, commands-show-output)
docs/en/ui/cli_tools/ac/extending-with-plugins.md
17-17: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/en/ui/cli_tools/ac/administrator-command-reference.md
20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
63-63: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
88-88: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/en/ui/cli_tools/ac/getting-started.md
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/en/ui/cli_tools/ac/managing-cli-profiles.md
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/en/ui/cli_tools/ac/developer-command-reference.md
17-17: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Gitleaks (8.27.2)
docs/en/ui/cli_tools/ac/managing-cli-profiles.md
[high] 190-190: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (4)
docs/en/ui/cli_tools/ac/ac-and-kubectl-usage.md (1)
23-23
: Confirm “automatic routing” behavior.Please verify that auto‑routing of global resources to the global cluster is implemented across all listed commands and note any exceptions.
docs/en/ui/cli_tools/ac/extending-with-plugins.md (1)
21-23
: Confirm plugin name discovery for kubectl‑ prefixed binaries.Docs state files starting with
kubectl-
are recognized by ac. Please confirm ac scans bothac-
andkubectl-
prefixes (and document search PATH/dirs if applicable).docs/en/ui/cli_tools/violet.mdx (1)
9-9
: Target doc exists — no action required.
docs/en/extend/upload_package.mdx found.docs/en/ui/cli_tools/ac/getting-started.md (1)
1-295
: Check for unlabeled code fences (MD040). Search docs/en/ui/cli_tools/ac/getting-started.md for lines that are exactly three backticks and add language identifiers (e.g., three backticks followed by the language name likebash
oryaml
) to any unlabeled fences. Suggested command: grep -nE '^[[:space:]]```[[:space:]]$' docs/en/ui/cli_tools/ac/getting-started.md
``` | ||
# Update pod 'foo' with the annotation 'description' and the value 'my frontend' | ||
# If the same annotation is set multiple times, only the last value will be applied | ||
ac annotate pods foo description='my frontend' | ||
|
||
# Update a pod identified by type and name in "pod.json" | ||
ac annotate -f pod.json description='my frontend' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Add languages to bare fenced code blocks (MD040).
A few blocks still use ``` without a language. Please mark as bash/yaml/json/text as appropriate.
To find and fix remaining fences without a language:
Also applies to: 45-55, 73-76
🏁 Script executed:
#!/bin/bash
# List lines starting a fence without language
rg -nP '^\s*```(?!\w)' docs/en/ui/cli_tools/ac/developer-command-reference.md
Length of output: 82
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="docs/en/ui/cli_tools/ac/developer-command-reference.md"
if [ ! -f "$file" ]; then
echo "FILE_NOT_FOUND: $file"
exit 0
fi
echo "Searching for fenced code blocks without language in: $file"
if command -v rg >/dev/null 2>&1; then
rg -n -U -C2 '^[[:space:]]*```[[:space:]]*$' "$file" || echo "NO_MATCHES"
else
grep -n -E -C2 '^[[:space:]]*```[[:space:]]*$' "$file" || echo "NO_MATCHES"
fi
Length of output: 28109
Add language identifiers to all fenced code blocks in docs/en/ui/cli_tools/ac/developer-command-reference.md (MD040)
Multiple fenced blocks in this file are missing language tags — add "bash" for CLI examples and "yaml"/"json" where appropriate. Affected examples include (non-exhaustive): 18–38, 46–64, 72–75, 83–102, 110–116, 124–133, 141–147, 155–169, 181–209, 217–220, 228–234, 242–248, 256–259, 267–279, 287–334. Run rg -n -U -C2 '^[[:space:]]```[[:space:]]$' docs/en/ui/cli_tools/ac/developer-command-reference.md to list all occurrences and fix the opening fences.
🤖 Prompt for AI Agents
In docs/en/ui/cli_tools/ac/developer-command-reference.md around lines 18 to
334, several fenced code blocks are missing language identifiers; update each
opening fence from ``` to include the appropriate language (e.g., ```bash for
CLI examples, ```json for JSON snippets, ```yaml for YAML) for the ranges called
out (18–38, 46–64, 72–75, 83–102, 110–116, 124–133, 141–147, 155–169, 181–209,
217–220, 228–234, 242–248, 256–259, 267–279, 287–334); run the provided rg
command to list every bare fence, then edit each opening ``` to the correct
```bash/```json/```yaml tag so Markdownlint MD040 is satisfied and examples
render with proper syntax highlighting.
weight: 10 | ||
--- | ||
|
||
# ACP CLI (oc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix product/CLI name: "oc" → "ac".
This page introduces ACP CLI, but the H1 says "(oc)". It should be "(ac)".
Apply this diff:
-# ACP CLI (oc)
+# ACP CLI (ac)
📝 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.
# ACP CLI (oc) | |
# ACP CLI (ac) |
🤖 Prompt for AI Agents
In docs/en/ui/cli_tools/ac/index.mdx around line 5, the H1 currently reads "ACP
CLI (oc)" but should read "ACP CLI (ac)"; update the header text to replace
"(oc)" with "(ac)" so the product/CLI name is correct.
# Wait for the pod "busybox1" to be deleted, with a timeout of 60s, after having issued the "delete" command | ||
ac delete pod/busybox1 | ||
ac wait --for=delete pod/busybox1 --timeout=60s | ||
``` No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个文件有点儿太长了,估计翻译可能会有问题,能拆一下吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个目前没办法拆,这个文档里面没有下一级分组了,是按照字母顺序生成的 reference 文档,如果翻译超上下文了我先手动生成下翻译吧
Summary by CodeRabbit
Documentation
Chores
Style