Repository navigation
docs(skill): add the repo-owned ocx operating skill - #2788
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds a repo-owned ChangesOCX skill contract and generated surface
CLI reference and repository integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI_CAPABILITIES as CLI capability registry
participant SURFACE_GENERATOR as surface generator
participant MANAGEMENT_SURFACE as committed management surface
participant SKILL_TESTS as skill tests
CLI_CAPABILITIES->>SURFACE_GENERATOR: provide capability definitions
SURFACE_GENERATOR->>MANAGEMENT_SURFACE: render capability-to-route documentation
SKILL_TESTS->>MANAGEMENT_SURFACE: compare committed output with renderer
SKILL_TESTS->>CLI_CAPABILITIES: validate documented command invocations
Suggested reviewers: ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
|
✅ Deterministic PR hygiene checks passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1cf660c23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Check the generated ocx skill surface is current | ||
| run: bun run skill:surface:check |
There was a problem hiding this comment.
Include skill files in the CI path filters
The new check is skipped when a pull request changes only skills/ocx/**: the gates job requires needs.changes.outputs.ci == 'true', but the ci filter at lines 176–193 does not include skills/**; the push trigger omits it as well. Consequently, direct edits to the generated surface or invalid commands added to the handwritten references can receive a green aggregate check without running either this step or tests/skill-ocx.test.ts. Add the skill directory to both path allowlists.
AGENTS.md reference: AGENTS.md:L184-L191
Useful? React with 👍 / 👎.
| That is the machine-readable index of every verb, the routes it drives, its flags, and whether it | ||
| mutates. Read it first rather than guessing a command name — it is generated from the same table | ||
| that generates the CLI help, so it cannot describe a verb that does not exist. |
There was a problem hiding this comment.
Scope the capabilities index to its actual subset
Do not describe this as an index of every verb: CAPABILITIES currently contains only 26 entries and is explicitly marked incomplete in src/cli/capabilities.ts, while CLI_COMMANDS includes commands such as ready, doctor, and access, and even the recipes rely on missing operations such as provider add/test/set-default. An agent following this instruction will therefore fail to discover large parts of the supported CLI; either expand the capability table or state precisely that it covers only the declared management subset.
Useful? React with 👍 / 👎.
| ```bash | ||
| ocx ready --json # is it up at all? | ||
| ocx status --json # is it the build you think, on the port you think? | ||
| ocx doctor --json # what is structurally wrong |
There was a problem hiding this comment.
Remove the unsupported JSON flag from the doctor recipe
ocx doctor has no JSON mode: runDoctor in src/cli/doctor.ts:956-1008 recognizes only its repair/recovery flags, silently ignores --json, and prints the normal multiline human report. An agent executing this ostensibly JSON-based diagnostic sequence will receive non-JSON output and fail when parsing it, so use bare ocx doctor or implement and document a real JSON envelope.
Useful? React with 👍 / 👎.
| `accounts[]` with `id`, `email`, `plan`, `paused`, `selected`, `priority`, and `needsReauth`. Quota | ||
| appears only under `--quota`. | ||
|
|
||
| `paused` and `selected` are independent — a paused-but-selected account still receives requests. |
There was a problem hiding this comment.
Correct the paused-account routing guidance
A paused account does not continue receiving requests merely because it is still displayed as selected. The pause route clears thread bindings and selects a fallback (src/codex/auth-api.ts:1507-1511), and the routing path explicitly excludes paused accounts, as verified by tests/codex-routing.test.ts:375-391. This guidance can make operators believe a successful pause is ineffective; explain that the two display flags may coexist while paused accounts remain ineligible for routing.
Useful? React with 👍 / 👎.
| `storage cleanup`, `storage trash restore`, and `storage policy run` exit 2 without `--yes` and send | ||
| no mutating request at all. So an accidental invocation is a no-op, not a partial delete. |
There was a problem hiding this comment.
Keep cleanup preview out of the exit-2 group
storage cleanup intentionally succeeds without --yes: cleanup() calls the preview endpoint, prints the preview, and returns normally at src/cli/storage.ts:88-97, yielding exit 0. Only storage trash restore and storage policy run reject missing confirmation with exit 2. Grouping cleanup with them contradicts the preview recipe and will cause automation to treat a successful preview as a usage failure.
Useful? React with 👍 / 👎.
c1cf660 to
88b90ad
Compare
617bab1 to
00a12c9
Compare
88b90ad to
cafdfb0
Compare
00a12c9 to
160b9a0
Compare
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
…surface map Every prior phase widened what an agent can do; this makes it discoverable without reading source. The capability-to-route map is generated from src/cli/capabilities.ts and a test fails if the committed copy drifts, because a hand-maintained surface map is a second description of the CLI free to diverge from the first. A command-existence test caught a documented ocx request-history that never existed, plus three wrong argument shapes. Every recipe was executed against a live proxy rather than transcribed.
The test already catches drift, but as a byte comparison in a test log. A named step tells the contributor which command fixes it.
cafdfb0 to
4ea5804
Compare
160b9a0 to
1bbd6b9
Compare
…scan Closes the unit. The rebase onto dev was a no-op -- dev never moved off 50e9556 -- so the ancestry was proved link by link across all nine pairs instead of the step being quietly skipped. Three CI failures, three distinct causes, each fixed in the lowest branch that showed it so the restack carries the fix upward rather than leaving the stack red below the fix. The part worth reading is the audit. I had scanned every changed file with the hygiene gate's own helper, got zero matches, and concluded the empty_catch failure was purely a prose match. The reviewer disproved that: the regex does not match a catch body containing only a comment, and this stack had added one. The scan was sound and the conclusion was wrong. It also caught that my first sync-cache fix reported failed refreshes as deliberate skips. Both are recorded rather than smoothed over, including the earlier phases' 429-driven subagent substitution ending here -- this reviewer was a real dispatched explorer. WP9MSG && git log --oneline -1
4ea5804 to
368d85b
Compare
리뷰 · 우선순위 58 / 80이 PR은 ocx 스택이 넓힌 CLI를 에이전트가 소스 안 읽고도 쓰게 하는 운영 스킬입니다. 베이스는 구성은 다섯 장입니다. 생성 표면을 테스트로 잠근 선택이 핵심입니다. 동의 경계도 분명합니다. star를 사용자 대신 하지 말라고 적고, 라인 skills/ocx/SKILL.md - 방향·exit·동의. star/gh 우회 금지 문장. 라인 scripts/generate-ocx-skill-surface.ts - capability → 01_management_surface.md 생성기. 라인 skills/ocx/references/01_management_surface.md - 생성물. 손편집 금지, check가 잠금. 라인 tests/skill-ocx.test.ts - 바이트 일치·커맨드 존재·부정 문장 스킵·gh api 금지를 잠금. 라인 docs-site/.../reference/cli.md - exit 표·preview-first·스택 동작 변화. 영어 우선. 라인 .github/workflows/ci.yml / package.json - skill:surface:check 스텝·스크립트 등록. 경로 base - #2787 위. gui-parity와 capability 스택이 먼저여야 스킬이 가리키는 명령이 실재합니다. 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Cleanup without --yes is a preview that exits 0, API failures still print stderr not an error envelope, and doctor has no --json mode. Co-authored-by: Cursor <cursoragent@cursor.com>
docs(skill): add the repo-owned ocx operating skill
docs(skill): add the repo-owned ocx operating skill
Summary
Every prior phase in this stack widened what an agent can do with
ocx. This one makes itdiscoverable without reading source, by adding a repo-owned operating skill.
Repo-owned rather than in
$CODEX_HOME/skills, because it describes this repository's CLIcontract and has to version with it.
The surface map is generated, and a test enforces it
01_management_surface.mdis rendered from the capability table byscripts/generate-ocx-skill-surface.ts.tests/skill-ocx.test.tsasserts the committed file matchesregeneration byte for byte, and CI runs
bun run skill:surface:checkas a named step so the failuretells you the fix instead of showing a byte diff.
Hand-writing it would recreate the defect this whole stack removed: a second description of the CLI
surface, free to drift from the first.
Driven red both ways — adding a comment to a capability leaves the check green (renders no output),
changing one
summarystring fails it.The command-existence gate found a real error
One test extracts every
ocx <command>presented as a command across all five pages and asserts eachexists in
CLI_COMMANDS. It immediately failed onocx request-history— a command the designdoc's recipe section named and which does not exist. The route-decision view is
ocx logs explain <request-id>.Checking the rest against source found three more wrong shapes:
access key add --label Xaccess key create <name>(positional)access key remove --id Xaccess key remove <id> --yes(positional)provider default Xprovider set-default XA skill documenting a command nobody can run is worse than no skill: an agent tries it, gets
Unknown command, and concludes the tool is broken.The extractor needed a second pass. Scanning raw prose flagged "driving ocx programmatically" and
the sentence "there is no
ocx request-historycommand" — a line whose entire purpose is to say thecommand does not exist. It now reads only code spans and fenced blocks, skipping negated lines, and a
companion test asserts it still finds eight real commands so narrowing it could not make the main
assertion vacuous.
Recipes were executed, not transcribed
ready,status,logs --jsonl,logs explain,access key list,storage report,storage cleanup --percent 1,inspect star,inspect pacing,inspect client-config,integration native list, andagent request-user-inputwere all run against a live proxy, and thefield names in
02_json_shapes.mdwere read off those responses.Consent
SKILL.mdstates plainly not to star the repository on the user's behalf, and names the workaroundsit forbids —
gh, a raw HTTP call, a minted session. Three tests hold that line, including one thatfails if any page contains
gh api. The failure mode being guarded is a skill that mentions aboundary and then supplies the workaround anyway.
Docs-site
reference/cli.mdgains the exact exit-code table (0/2/4/5/1), the preview-first rule forstorage cleanup, an agent-orientation section, and a behavior-change list for this stack:doctor/sync-cacheexiting non-zero, 404→4 and 409→5, position-independent--json,logs --modelactually filtering, andocx storagegaining subcommands with its bare form unchanged.Changes are additive, so the seven translated locales are less complete but do not contradict the
English source.
Verification
Checklist
Summary by CodeRabbit
New Features
ocxCLI skill documentation, including command discovery, JSON output, recipes, safety guidance, and failure handling.Bug Fixes
Documentation
ocxskill documentation.