fix(cli): generate --llms-full from live schemas - #474
Conversation
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5789da8956
ℹ️ 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".
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2e63a0a1d
ℹ️ 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".
…/codex/fix-llms-full-manifest
|
@codex review Please review the current head, |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59ae515c1f
ℹ️ 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".
|
@codex security review Please review the current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 025bf950a1
ℹ️ 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".
mldangelo-oai
changed the title
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5ad985147
ℹ️ 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".
kmbroai
left a comment
kmbroai
left a comment
There was a problem hiding this comment.
Critical review
Reviewed head c5ad985147f5eaba53682cd816fd60c5c9e6f50d.
Recommendation: useful discovery improvement, with one remaining documentation correction; no new execution blocker found. Deriving accepted flags and constraints from live schemas is preferable to maintaining a second handwritten CLI reference. The Markdown adapter correctly leaves the machine-readable schemas intact.
Remaining accuracy gap
The existing comment about bulk-scan's hint still applies. It documents that CSV mode requires --output-dir, but not the reverse restriction: bulk-scan --output-dir /tmp/results without a CSV is rejected before the interactive wizard starts. Scoped bulk-scan --llms-full omits the root operating guide. Add the reverse dependency to the command-owned guidance. This is an incomplete reference, not evidence that the runtime validation itself is broken.
Necessity and simplification
The new module does more than render documentation: it also owns result-format rules, global-option parsing, and reconstruction of safe validation messages. Shared runtime/reference rules are worthwhile, but keep those ownership boundaries visible. In particular, humanValidationMessage depends on Incur's human-output shape; when the dependency changes, it must fail closed to the existing redacted message rather than assume arbitrary output is schema-owned. The current fallback does that.
I would not add a general command-dependency DSL to fix the remaining hint. Use command-owned guidance for the few cross-option rules, and schemas for type/value constraints. The adapter should stay a compatibility layer for the pinned Incur omissions, not grow into a competing CLI framework. Its comments should remain tied to those upstream omissions so it can eventually be removed.
The previously reported equals-form global options, repeated-value constraints, and patch selector guidance are addressed at this head; I am not treating old comments as unresolved without checking the current code.
Verification
Ran cli-manifest.test.ts and cli.test.ts: 157 passed, 0 failed, covering scoped discovery, structured schema parity, global-option forms, and existing CLI behavior. Linux, Bun 1.3.14 / Node 22.13.1 with cached dependencies. No published-package smoke or native Windows run was performed.
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa81998336
ℹ️ 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".
| ...commands.map((command) => { | ||
| const restrictions = commandResultRestrictions(command.name ?? ""); | ||
| return [ | ||
| Skill.generate(cli.name, [command]).replace(/^#/gmu, "###"), |
There was a problem hiding this comment.
For the scoped scan-components --llms-full reference, this renders --component, --components-file, and --auto as independently optional and includes no command guidance, while the live refinement in cli.ts rejects every invocation that does not select exactly one of them. Because scoped output deliberately omits the README operating guide, an agent following this reference can construct commands that are guaranteed to fail; include the refinement through shared command guidance or another schema-derived source.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
| hint: | ||
| "--linear-team or CODEX_SECURITY_LINEAR_TEAM is required. " + | ||
| "--linear-assignee requires --linear-api-key or CODEX_SECURITY_LINEAR_API_KEY. " + | ||
| "If both --linear-project and --project are set, they must select the same project.", |
There was a problem hiding this comment.
These destination requirements are attached only to publish scan, so publish check --llms-full still presents --linear-team, --linear-assignee, and both project selectors without their dependencies. The publish check handler calls the same publicationDestination helper at cli.ts:2561, which rejects a missing team, an assignee without an API key, or conflicting project selectors; reuse this guidance for both publication commands so the scoped check reference does not advertise guaranteed-invalid combinations.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a13f0a7390
ℹ️ 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".
| for (const [key, label] of CONSTRAINT_LABELS) { | ||
| if (property[key] !== undefined) { | ||
| details.push(`${label}: ${property[key]}.`); |
There was a problem hiding this comment.
When an agent consults import github --llms-full, the --github-alert item schema contains the pattern ^[1-9]\d*$, but this renderer only emits enums, numeric bounds, and length bounds. The generated reference therefore presents arbitrary nonempty strings as valid alert numbers even though the live schema rejects them; include pattern constraints or schema-derived equivalent guidance so the schema remains the source of truth.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
| hint: | ||
| "--linear-team or CODEX_SECURITY_LINEAR_TEAM is required. " + | ||
| "--linear-assignee requires --linear-api-key or CODEX_SECURITY_LINEAR_API_KEY. " + | ||
| "If both --linear-project and --project are set, they must select the same project.", |
There was a problem hiding this comment.
For publish scan --llms-full, the positional scan directory, --scan, and --scan-dir are rendered as independent inputs while this hint only describes Linear destination requirements. An agent can therefore supply --scan together with either directory form even though the handler unconditionally rejects that combination, or combine --csv with any scan input and receive another guaranteed failure; include these source-selection rules in the scoped reference from the same source used by the handler.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a31c7d2ea
ℹ️ 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".
| hint: [ | ||
| ...Object.values(PUBLICATION_SOURCE_REQUIREMENTS), | ||
| ...Object.values(PUBLICATION_DESTINATION_REQUIREMENTS), | ||
| ].join(" "), |
There was a problem hiding this comment.
For publish scan --llms-full, this hint states that a Linear team is always required but omits that --csv is valid only with --to cloud; the handler at lines 2207–2230 also rejects Linear options and --skip-existing for Cloud. Consequently, a Cloud user following the generated reference may add the advertised team option and construct a command guaranteed to fail. Derive destination-specific guidance from the same rules enforced by the handler.
AGENTS.md reference: sdk/typescript/AGENTS.md:L14-L16
Useful? React with 👍 / 👎.
| hint: | ||
| "--path, --diff, and --working-tree are mutually exclusive. " + | ||
| "Deep-scan settings require --mode deep. --patch-severity and --create-pr require --patch, and --patch cannot be combined with --dry-run. " + | ||
| "Use --json for scan results; " + | ||
| "--dry-run checks local inputs without verifying authentication or model access.", |
There was a problem hiding this comment.
When an agent requests scan --llms-full, the operating guide is omitted and this hint does not include the refinements at lines 2863–2871: --head requires --diff, and --base requires --working-tree. The option descriptions only say what each ref is used for, so the scoped reference still permits apparently valid commands such as scan . --head HEAD that are unconditionally rejected; include these dependencies in the command guidance.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
| hint: | ||
| "Use issue text or files, saved finding identifiers with optional --scan, Linear selectors, or --resume-pr. " + | ||
| "Saved findings cannot be combined with Linear selectors. --severity and --create-pr require saved findings; " + | ||
| "--linear-filter requires --linear-project, and --linear-api-key requires a Linear selector. " + | ||
| "--resume-pr cannot be combined with other patch inputs or options.", |
There was a problem hiding this comment.
For patch --llms-full, this guidance advertises multiple saved finding identifiers without stating that they must all resolve to one saved scan. If identifiers from different scans are supplied without --scan, selectSavedFindings rejects them at lines 4744–4763, so an invocation constructed from the reference is guaranteed to fail. Document the one-scan constraint here and in verify-fix, which uses the same selector.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffa4bc5fd2
ℹ️ 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".
| expect(Number(scopedCount.trim())).toBeLessThan(Number(count.trim())); | ||
|
|
||
| const limited = await invoke(["--llms-full", "--token-limit", "1"]); | ||
| expect(limited).toContain("[truncated: showing tokens 0–1 of "); |
There was a problem hiding this comment.
Fresh evidence after the earlier wording assertions were removed is this remaining literal assertion on Incur's generated truncation message: a harmless renderer copy change would fail the suite even though the following length assertion already verifies that token limiting works. Assert only the stable truncation behavior instead of the exact Markdown sentence.
AGENTS.md reference: sdk/typescript/AGENTS.md:L32-L36
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1135bb4aa5
ℹ️ 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".
| }), | ||
| hint: | ||
| "Use issue text or files, saved finding identifiers with optional --scan, Linear selectors, or --resume-pr. " + | ||
| "Saved findings cannot be combined with Linear selectors. --severity and --create-pr require saved findings; " + |
There was a problem hiding this comment.
Fresh evidence after the earlier dependency fix is the merged direct-input branch at lines 4081–4131: patch "issue" --create-pr and Linear-backed patches snapshot the current tree and create a pull request without selecting saved findings. This hint incorrectly says --create-pr requires saved findings, so the generated patch --llms-full reference discourages valid supported commands; limit that requirement to --severity.
AGENTS.md reference: sdk/typescript/AGENTS.md:L14-L16
Useful? React with 👍 / 👎.
| .default([]) | ||
| .describe("Linear issue identifier or URL; repeat for more issues."), | ||
| .describe( | ||
| "Linear issue identifier or URL; repeat for more issues; cannot be combined with --linear-project or saved findings.", |
There was a problem hiding this comment.
For verify-fix --llms-full, this description invites repeating --linear-issue but does not state that each issue must be distinct. Supplying the same issue twice, such as --linear-issue SEC-1 --linear-issue SEC-1, produces duplicate imported IDs and is unconditionally rejected by the uniqueness check at lines 3763–3766, so include that constraint in the scoped reference.
AGENTS.md reference: sdk/typescript/AGENTS.md:L14-L16
Useful? React with 👍 / 👎.
|
@codex review Please review current head |
|
@codex review Please review head Focused CLI-manifest tests: 24 passed. SDK TypeScript check, plugin bundle generation, package-script syntax, changed-file formatting, and git diff --check passed. The full suite is left to CI. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dad7dc8aa5
ℹ️ 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".
| ...commands.map((command) => { | ||
| const restrictions = commandResultRestrictions(command.name ?? ""); | ||
| return [ | ||
| Skill.generate(cli.name, [command]).replace(/^#/gmu, "###"), |
There was a problem hiding this comment.
For classify-severity --llms-full, the scoped output omits the operating guide and renders --scan and --scan-dir as independently optional, although the live handler at cli.ts:3276-3282 rejects both a missing selector and supplying both selectors. An agent relying on this reference can therefore construct commands guaranteed to fail; include the exactly-one requirement in schema-derived command guidance.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
| ...commands.map((command) => { | ||
| const restrictions = commandResultRestrictions(command.name ?? ""); | ||
| return [ | ||
| Skill.generate(cli.name, [command]).replace(/^#/gmu, "###"), |
There was a problem hiding this comment.
For dedupe --llms-full, the scoped reference renders both --scan and --workflow-id as optional, so an invocation containing only the required --findings-url appears complete even though the handler at cli.ts:3372-3381 rejects it unless one of those selectors is provided. Because scoped output omits the README examples, include this at-least-one requirement in the command guidance.
AGENTS.md reference: sdk/typescript/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
3 participants
Summary
Generate
--llms-fullMarkdown from the installed CLI's live schemas. Rootdiscovery includes the packaged operating guide; scoped references stay focused
on the selected command or group. Structured JSON manifests retain their shape.
Changes
numeric/length bounds, repeated-value constraints, and regex patterns.
Incur's token count, limit, and offset controls.
exclusivity, publication destination requirements, and conflicting publication
sources. Both
publish scanandpublish checkreceive the relevant guidance.infofilter validator and metadata-field list; theshared validator still enforces the same rule from the output schema.
smoke path. Keep validation guidance schema-owned without echoing rejected values.
Testing
Merge conflict resolution at
dad7dc8aa5f9c9d58a504c01d2df1b46918c2253againstmainat6750642c94edce8d60112f816631de05dc10efd6:Earlier validation recorded for prior heads:
Current main integration on
c1fdb71243c3310f4e2f65dc88347a307ceb5274:Local full-suite and installed-package checks were not repeated after this merge because the validation host repeatedly exhausted disk space. Fresh CI is requested for the new head; its outcome is separate from human approval.
Risk and rollout
This changes Markdown emitted by the existing
--llms-fullflag and makesrelated validation guidance more specific. It adds no command, flag, supported
environment variable, dependency, state migration, or plugin-version change.
Command acceptance and existing publication error messages are unchanged by
the guidance fixes. Explicit JSON discovery remains structured; scoped Markdown
omits the full operating guide.
No package is published or released by this PR. Cross-platform CI and human
review remain separate merge requirements.
Public disclosure review
The update and public PR surface have been reviewed. Existing automated comments
contain access-restricted report links, so the second attestation remains unchecked.