fix(cli): improve init diagnostics and starter output - #776
Open
mldangelo wants to merge 1 commit into
Open
Conversation
Refusing to overwrite is init's advertised behavior, so surface it as a sentence naming the file instead of a raw EEXIST errno. Write the starter with default permissions. The file holds no credentials and is meant to be committed, so 0600 only broke shared checkouts and containers running as another user. JSON starters cannot carry the commented settings a YAML starter shows. Print that guidance instead of leaving the file's only content a $schema key, and point at the YAML template. Search upward from the starter for an installed @openai/codex-security so hoisted workspaces get an editor hint that resolves, falling back to the invocation directory when nothing is installed yet.
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #742 (base
mdangelo/codex/project-configuration), so it can be reviewed in parallel and will retarget tomainwhen that lands. Four follow-ups to theinitcommand added in #742, all cosmetic except the last.init's advertised behavior, so it now reads as a sentence naming the file rather than a raw errno. Before:codex-security: EEXIST: file already exists, open '/…/codex-security.yaml'. After:codex-security: /…/codex-security.yaml already exists. Edit it, or select it with --config codex-security.yaml.0o600. The file holds no credentials and is meant to be committed; the private mode only broke shared checkouts and containers running as another user.init file.jsonpreviously produced a file whose entire content was a$schemakey. It now prints that guidance to stderr and points at the YAML template. The file itself is unchanged.@openai/codex-securityso hoisted workspaces get an editor hint that resolves, falling back to the invocation directory when nothing is installed yet. This is the one behavioral change; the docs previously recorded the old behavior as a limitation and are updated to match.projectConfigStarternow returns{ contents, notes }so the "what guidance does this format need" decision stays next to the YAML comment block. It has one call site and is not exported fromindex.ts.Testing
bun test ./tests-ts/cli-project-config.test.ts: 52 passed, no failures. Three new cases cover the umask permissions (compared against a reference write, so the assertion holds under any umask), the JSON guidance, and the hoisted-workspace$schema. The existing overwrite case now also asserts the message and thatEEXISTno longer appears.bun testacrosscli-project-config,project-config,sdk-project-config,deep-config,cli,skeleton: 279 passed, 1 failed. The failure isskeleton.test.ts"keeps machine-wide policy changes out of parallel and experimental runs", which fails identically onmainin this environment (Bun.YAML.parseon bun 1.3.3 mis-parsing the workflowon:key) and is unrelated to this change.pnpm run typesandprettier --check: pass.node_moduleshoisted to the root and the starter written topackages/app/: the secondinitprints the new sentence and exits 2, the file is-rw-r--r--, the JSON starter prints both guidance lines to stderr, and the nested starter emits$schema: ../../node_modules/@openai/codex-security/schemas/project-config.schema.json.Risk and rollout
No shared types, resolution logic, or precedence rules change, and nothing in #742 depends on
projectConfigStarter's shape. The permissions change makes a previously owner-only file world-readable subject to umask; that matches how tracked configuration is normally created and the file is documented as never holding credential values.The
$schemasearch reads the filesystem duringinitonly. Existing behavior is preserved when nothing is installed, which is the common case for a first run beforenpm install.Public disclosure review