fix(#5268): config-new-project refuses the choices config-set refuses, and honors --dry-run

[0xdhx]

Fix PR

Using the wrong template?
— Enhancement: use enhancement.md
— Feature: use feature.md

---

Linked Issue

Required. This PR will be auto-closed if no valid issue link is found.

Fixes #5268

The linked issue must have the confirmed-bug label. If it doesn't, ask a maintainer to confirm the bug before continuing.

---

What was broken

config-new-project wrote almost any choices it was given into the new project's .planning/config.json and reported created: true (the one value it checked was ship.pr_body_sections, in the builder). An unknown key, or a value config-set refuses for the same key (for example a mode placeholder the new-project workflow left unfilled), landed in the config with no error. --dry-run was accepted, ignored, and the file was written anyway.

What this fix does

Choices are now checked with config-set's own key and value rules before anything is written, and a choice config-set refuses fails with the same error text; the three intended differences are listed below. The check runs before the existing-config check, so a refused choice fails even when config.json already exists. --dry-run prints the config that would be created and writes nothing; any other flag, and a second positional argument, are refused.

After the fix, the issue's reproduction:

$ gsd-tools query config-new-project '{"bogus_key":"x","mode":"nonsense"}' --dry-run
Error: Unknown config key: "bogus_key". Valid keys: …
$ gsd-tools query config-new-project '{"mode":"nonsense"}'
Error: Invalid mode 'nonsense'. Valid values: interactive, yolo
$ ls .planning
ls: cannot access '.planning': No such file or directory

Root cause

Two gaps in one verb:

1. buildNewProjectConfig spreads the choices verbatim (...choices) and cmdConfigNewProject never checked a key or a value. The value rules existed only inline in cmdConfigSet (L920-L1179), so no other verb could apply them.
2. routeConfigNewProject forwarded only args[1] (L839-L851), so every flag was dropped without a word.

The change

• src/config.cts
  • validateConfigValue(cwd, kp, parsedValue, val) is cmdConfigSet's per-key value block, moved into a function with its body unchanged except for one added rule: a project_code that is not a string is refused (config-set always stores the typed text, bug: config-set silently coerces values — context_window Infinity persisted as null while CLI reports Infinity (output ≠ disk) #1581, so the rule never fires there). cmdConfigSet calls it. assertKnownConfigKey is the same extraction for the key check and its error text.
  • validateNewProjectChoices walks the nested choices to the dotted leaf paths config-set addresses ({"workflow":{"research":true}} is checked as workflow.research) and runs each leaf through those two functions, then through the same prototype-pollution refusal config-set's setter applies. It returns the checked choices as a new object and leaves its input as given. An object is read as a section whenever settings live under its key: a static section, the top level of a dynamic key family, or a setting that is also a parent (review.max_prompt_tokens_per_reviewer, agent_tools), whose entries are then checked one by one as config-set checks each entry's own key.
  • cmdConfigNewProject parses and checks the choices first, then the existence check, then builds. With dryRun it prints { dry_run, would_create, path, config } and returns; secret keys are masked in that preview as config-set and config-get mask them.
  • planNewProjectConfig builds the config and returns the user defaults file's depth to granularity rewrite (L356-L363) as data, writing nothing. cmdConfigNewProject persists it only after config.json is written, so a --dry-run, a refused payload or a failed write leaves the defaults file as it was. buildNewProjectConfig keeps its other caller's contract (ensureConfigFile): plan, persist, return.
• gsd-core/bin/gsd-tools.cjs: routeConfigNewProject gets a closed flag set, --dry-run and --raw, following todo complete (#4096). A flag is a dash or two followed by a letter; any other argument, -1 included, is the choices JSON, which is the one positional argument wherever the flag sits.
• docs/CLI-TOOLS.md: the verb was not documented; it now is, with --dry-run and the three intended differences from config-set below.

Same defect class, folded in

Several inputs took the same unvalidated write path and are closed by the same walk:

Input	Before	Now
Choices that parse as JSON but are not an object ('["mode"]', '"yolo"')	spread into the config as index keys "0", "1", …	refused
An empty key ({"":{"mode":"yolo"}})	written as a section named ""	refused
A key whose dot spells a deeper path ({"workflow.research":false})	written as one literal key that no reader resolves, beside the untouched nested default	refused, with the nested form in the message
An empty object under a name that is not a section ({"bogus":{}}, at any depth)	written	refused with config-set's own unknown-key error; under a real section ({"workflow":{}}) it sets nothing
A real section given a non-object ({"workflow":"x"})	merged into the default section as index keys ("0":"x")	refused as a section, unless the key is itself a setting (agent_tools), where the setting's own rule applies as on config-set
A null choice ({"mode":null})	written as a literal null (and ship.pr_body_sections: null refused by the builder, where config-set accepts and clears it)	treated as not chosen: its key is still checked, and it takes the value it would have had without the choice; under __proto__, prototype or constructor it is refused (third difference below)

The dotted-key rule is not a blanket ban, because one key family carries a dot inside an id: phase_commit_docs is a map keyed by the literal phase id, and {"phase_commit_docs":{"2.1":true}} is the shape its resolver reads. So a dot is refused as structure unless one dynamic key family admits both the path cut at the dot and the whole path, which is what a dot inside a single id looks like to the schema. "2.1" under phase_commit_docs is kept and written as given; "workflow.research" at the top level, "fallow.scope" under code_quality, "max_prompt_tokens_per_reviewer.codex" under review and "blocks.workflow" under claude_md_assembly are refused. All five are pinned by test.

Three intended differences from config-set

The first two follow from the input being JSON rather than typed text. All three are documented in docs/CLI-TOOLS.md:

• Values are JSON-typed and never coerced. config-set workflow.research true parses the text true to a boolean; the choice "research":"true" is a string and is refused. Pinned by test for two booleans and two numbers.
• A setting with per-entry settings under it is checked entry by entry. For review.max_prompt_tokens_per_reviewer, {"codex":5000} is accepted and {"a":1} is refused, where config-set, given the whole object, stores it unchecked. agent_tools is walked the same way, so an invalid selector is refused as config-set agent_tools.<selector> refuses it.
• A null choice under __proto__, prototype or constructor is refused. For a key that is otherwise known ({"model_overrides":{"constructor":null}}), the prototype-pollution guard refuses it. config-set model_overrides.constructor null reaches that guard only when the config has a model_overrides section; without one its unset finds nothing to remove and exits 0. The default config has agent_skills and lacks model_overrides, agent_tools and features, so the difference can hold after creation. config-set is left as it is. Pinned by test for agent_skills, against config-set run on a default config.

Census: the flag set

The route now enumerates its flags, so every flag that can reach it has to be in the set or be refused on purpose. The dispatcher consumes each global before routing; measured on this branch, each with config-new-project '{}':

Flag	Reaches the route?	Result
--raw (either position)	no, spliced	created
--cwd <p>, --cwd=<p>	no, spliced	created in <p>
--pick <field>	no, spliced	created (--pick created; an absent field exits 1 after creating, as on every verb)
--project-dir <p>	no, spliced	created
--ws <name>, --ws=<name>	no, consumed by workstream resolution	created in the workstream
--json-errors, --exit-contract=<v>	no, spliced	created
--default <v>	no, spliced	created
--help, -h, --h, --usage	no, handled before routing	usage
--version, -v	no, refused before routing	Unknown flag: …, unchanged
--dry-run	yes	preview, nothing written
any other dash-and-letter flag that reaches the route (--force)	yes	Unknown flag for config-new-project: <flag>
-1, -	yes, as the choices argument	refused as choices (expected a JSON object, Invalid JSON)
a second --json-errors (--json-errors --json-errors)	yes: the dispatcher splices only the first copy	Unknown flag for config-new-project: --json-errors, as todo complete refuses it
a second --raw (--raw --raw)	yes, for the same reason	accepted, since --raw is in the set

--raw stays in the set for the reason todo complete gives, a guard against that splice ever moving, and because a repeated --raw reaches the route today.

Out of scope, stated

• Values config-set does not validate. model_profile, granularity, parallelization and commit_docs are in the workflow template, and config-set accepts any value for them (config-set model_profile bogus succeeds), so an unfilled placeholder in one of those four still gets through both verbs. The issue asks for parity and leaves config-set behavior out of scope. Because the rules are now one function, adding a rule for any of them fixes both verbs. The same holds for agent_tools, which has no value rule on either verb.
• Sibling verbs that drop --dry-run. config-ensure-section --dry-run and config-set-model-profile <p> --dry-run both write. Same class, different verbs; not touched here.
• The user defaults file. Its contents are not held to the choice rules, as before (out of scope per the triage brief); only the builder's own ship.pr_body_sections check reads them. They are read only to build a new config, so a problem in them is not reported when the config already exists. The merge itself is also unchanged: a choice in a section outside the six deep-merged ones (review, say) still replaces that whole section from the defaults file, which predates this PR. Because the builder checks ship.pr_body_sections after the merge, a null choice for it leaves an invalid value from the defaults file to be refused at creation, where config-set ship.pr_body_sections null on an existing config clears the key.
• config-set and a dot inside a phase id. config-set phase_commit_docs.2.1 true writes {"phase_commit_docs":{"2":{"1":true}}}, a shape the resolver does not read; config-new-project writes the "2.1" key it does read. That is config-set's own path splitting, untouched here.
• A failed write's residue. When the config write itself fails, the shared platformWriteSync helper can leave a partial config.json, and a retry then reports already_exists. The helper is byte-identical on the base; this PR does not touch it.

Testing

How I verified the fix

A describe block in tests/config.test.cjs with 95 tests, all but one through the CLI (the fault-injected config write runs in-process), and a property section at the end of the same file:

• Parity table, 18 rows. Each row is an input config-set refuses: unknown and misspelled keys, mode, context, context_window, workflow.* enums, booleans and ranges, git.create_tag, hooks.context_warning_threshold, plan_review.source_grounding_authority, statusline.state_format, and agent_skills.__proto__. Each test first runs config-set as a positive control, then asserts config-new-project fails with the identical error string and that .planning/ was not created.
• Forbidden segments with null, 3 rows. agent_skills.__proto__, .prototype and .constructor set to null: config-set on a default config refuses each (the positive control), and config-new-project fails with the identical error and creates nothing.
• Boundaries. The four numeric rules the parity rows touch, each at limit−1, limit and limit+1 on every end it has (hooks.context_warning_threshold −1/0/1/99/100/101, workflow.security_asvs_level 0–4, context_window and workflow.drift_threshold 0/1/2), each row asserting config-set's verdict before its own; and the positional count at zero, one and two.
• Property-based (fast-check, both verbs in-process): (a) a payload of one to five leaves drawn from fourteen keys is accepted exactly when config-set accepts every leaf, a refusal carries config-set's error for one of them, and each accepted leaf appears in the dry-run preview exactly as config-set --dry-run reports it; (b) one unknown leaf inserted anywhere refuses the payload and writes nothing. At the shared seed, 200 runs of (a) give 75 accepted and 125 refused; the test asserts both verdicts occur.
• The rest: one bad key refuses the whole payload; normalized values are written normalized; non-object JSON, empty keys, structural dotted keys, empty objects under unknown names, non-object sections, unsupported flags and a second positional are refused with nothing written; null choices give the config a {} payload gives; agent_tools selectors are checked; JSON-typed strings are refused where config-set coerces; --dry-run (before and after the choices) prints what a real run then writes, and with --raw prints the text form would create (dry run), writing nothing either way; it masks secrets exactly as config-set --dry-run does, and refuses invalid choices; the user defaults file is migrated only by a creation that wrote its config (fault-injected write, dry run and refused payloads leave it byte-identical); an existing config does not hide a refused choice.
• The shipped callers. The choices template is read out of workflows/new-project.md and workflows/new-project/steps/auto-mode-config.md, checked for a leftover placeholder, filled, and must be accepted with every chosen leaf written as chosen and every other key left at its default.

Negative controls. The 44 tests of the PR's first commit (e7179dff5), run against a build of the base: 40 fail, and the four that pass (the two template cases, the existing-config dry run and the decimal phase id) pin behavior that must not move. In the review round, I ran each new row against the build before its fix, where it failed, and reverted each fix alone at the final tree, which turned its named rows red. In the 2026-10-14 round, moving the null skip above the segment guard turned the three forbidden-segment rows red and left the file's other 287 tests passing, and the file's other 193 tests passed on that head and on its base (433a43445). After the 2026-10-18 rebase onto 755bf26f0, which replays every commit unchanged, the file passes 290 of 290 at this head, and every earlier commit of the PR passes it at its own tree.

Run locally on this branch: npm run lint:ci, then the unit, integration, security, install, slow and qa suites (the Detect test scope selector reports full_matrix: true for this diff).

Regression test added?

• Yes — added a test that would have caught this bug
• No — explain why:

Platforms tested

• macOS
• Windows (including backslash path handling)
• Linux
• N/A (not platform-specific)

Locally on Linux. In this PR's CI, the macOS conformance leg runs tests/config.test.cjs; the Windows legs run tests/config.platform.test.cjs instead, so the new block is not exercised on Windows.

Runtimes tested

• Claude Code
• Antigravity
• OpenCode
• Other: ___
• N/A (not runtime-specific)

---

Checklist

• Issue linked above with Fixes #NNN — PR will be auto-closed if missing
• Linked issue has the confirmed-bug label
• Fix is scoped to the reported bug — no unrelated changes included
• Regression test added (or explained why not)
• All existing tests pass (npm test)
• .changeset/ fragment added if this is a user-facing fix (npm run changeset -- --type Fixed --pr <NNN> --body "...") — or no-changelog label applied
• No unnecessary dependencies added

Breaking changes

Inputs that used to be written are now refused. That is the fix, but a caller relying on the old leniency will see an error where it saw created: true:

• an unknown or misspelled key, or a value outside a key's allowed set, including a string where the key takes a boolean or a number ({"workflow":{"research":"true"}}); ship.pr_body_sections was already checked;
• choices that are not a JSON object, structural dotted keys (a dot inside a phase id under phase_commit_docs is kept), empty objects under names that are not sections, and sections given a non-object (except a key that is itself a setting, such as agent_tools);
• an invalid agent_tools selector, and a project_code that is not a string;
• an unsupported flag such as --force, and a second positional argument.

A null choice now leaves its setting unchosen instead of writing null, without an error, except under __proto__, prototype or constructor, where it is refused. A refused choice now fails even when config.json already exists, where it used to report already_exists. --dry-run no longer writes. Two accepted inputs now produce a different config: review.default_reviewers is written normalized, as config-set writes it (["Codex","codex","gemini"] becomes ["codex","gemini"]), and a section whose choices are all null or empty no longer replaces that section from the user defaults with {}. The success output and the existing-config result for valid choices (created: false) are unchanged, and both shipped workflow templates produce the same config, pinned by test.

[trek-e]

Review of head 2281bd7127ab — changes requested

Summary. The fix is on the failing path: config-new-project now refuses unknown keys, out-of-enum values and unsupported flags through validators extracted from config-set, and --dry-run writes nothing (all confirmed by running the head CLI). What blocks approval: the repo's two mandatory test rules are not met for the new validator (no property test, no accept-side boundary rows), validation is skipped entirely when a config already exists, and "the same rules as config-set" is not true for a few inputs. Major 2, Minor 8, Nit 9.

Classification and gates. #5268 carries bug + confirmed-bug (RULESET.CONTRIB.CLASSIFY.fix=requires confirmed-bug before implementation (legacy 'confirmed' label is back-compat only for duplicate-sweep exemption, not a valid implementation gate)). Fixes #5268, conventional title, Fix template present, not draft, no competing PR. Changeset .changeset/calm-heron-checks.md: type Fixed, pr: 5284, bold lead; docs/CLI-TOOLS.md documents the verb. No shipped workflow, agent or command changed (no compact-content baseline or Emitted-Drift-Ack-Growth requirement); no references/ or workflows/ file added.

Acceptance criteria, #5268 Agent Brief (verbatim)

1. "Unknown key in choices: non-zero exit, Unknown config key error, no config file created." — Met on the create path; not met when config.json already exists (Minor 1).
2. "mode outside interactive/yolo (and any other value config-set rejects for that key): non-zero exit with the same message, no file created." — Met for the 18 table rows; Partial for the rest (Minor 2) and on the existing-config path.
3. "Unsupported flags are refused, or --dry-run prints the config and writes nothing." — Met (both).
4. "The full set of keys the new-project workflow template emits … is still accepted and produces the same config as today." — Met (tests fill both shipped templates).
5. "Existing-config idempotency (created: false) and defaults from the user defaults file are unchanged." — Met for idempotency; see Minor 7 for the reordered defaults-file write.
   Out of scope per the Brief: "config-set behavior; validation of user-defaults file contents; changing the template keys."

Reproduction (built head CLI in a throwaway git init repo, no test runner)

Command	Result
config-new-project '{"mode":"nonsense"}' --dry-run (no config)	Error: Invalid mode 'nonsense'. Valid values: interactive, yolo
same, with .planning/config.json present (dry-run and real)	exit 0, {"created": false, "reason": "already_exists"}
config-set mode nonsense	Error: Invalid mode 'nonsense'…
config-set review.max_prompt_tokens_per_reviewer '{"a":1}'	accepted (updated: true)
config-new-project '{"review":{"max_prompt_tokens_per_reviewer":{"a":1}}}' --dry-run	Unknown config key: "review.max_prompt_tokens_per_reviewer.a"
config-new-project '{"ship":{"pr_body_sections":null}}' --dry-run	refused (config-set accepts null and clears)
config-new-project '{"mode":null}' --dry-run	accepted
config-new-project -1	Unknown flag for config-new-project: -1

Findings

Major 1 — no fast-check property for the new validator [verified] validateNewProjectChoices is a recursive walk over arbitrary nested JSON with dotted-key, empty-section and dynamic-family logic, and tests/config.test.cjs adds table rows only (RULESET.TESTS.property-based-testing, a MUST for parsers and contracts of this kind). The code-review Standards pass cites the rule as violated; the adversarial pass rated the same gap Minor, the delegated pass Major; both ratings are kept. Suggested property: for any nested object built from valid leaf paths, acceptance equals the fold of config-set verdicts on the leaves, and one injected unknown leaf is always refused with nothing written.

Major 2 — boundary rows assert only the refused side [verified] The parity rows (context_window 0, security_asvs_level 9, drift_threshold 0, hooks.context_warning_threshold 0) never include the accepted-at-limit and refused-just-outside pair (RULESET.TESTS.boundary-coverage: limit-1, limit, limit+1). E.g. hooks.context_warning_threshold 0/1/99/100/101 and security_asvs_level 1-3 against 4; the flag and positional counts (0, 1, 2 positionals, flag before and after) also lack the n-1/n/n+1 shape.

Minor 1 — invalid choices are not refused when config.json already exists [verified: table above] cmdConfigNewProject returns already_exists before parsing or validating, so a bad key or bad mode exits 0, and --dry-run gives no preview. The Brief's idempotency line and its "non-zero exit" lines both hold on different paths; the ordering is not settled in the Brief. Fix: validate before the existence check, or document and test the short-circuit.

Minor 2 — "same key and value rules as config-set" is overstated [verified: table above; adversarial sweep 1,875 key×value cases found 14 divergences, 11 of them the CLI-string coercion] Real rule divergences: an object value for review.max_prompt_tokens_per_reviewer is accepted by config-set and refused here (the walker treats it as a section); ship.pr_body_sections: null is accepted by config-set and refused here; mode: null is accepted here and written, where config-set <key> null clears. The PR text and docs/CLI-TOOLS.md say the rules are the same. Fix: align, or enumerate the intended exceptions in the docs, and pin the null behaviour with a test.

Minor 3 — the documented string-for-boolean break is not pinned [verified] {"workflow":{"research":"true"}} is refused here while the same text through config-set is accepted (JSON-typed values, no coercion). The PR lists it under Breaking changes, but no row locks it.

Minor 4 — the issue's coupled command-aliases entry is not addressed [verified diff: no file under that path changed; judgment on applicability] Issue #5268, "Coupled (must not miss)": "command-aliases entry". The diff touches only .changeset, docs/CLI-TOOLS.md, gsd-tools.cjs, src/config.cts, tests/config.test.cjs. If no alias applies (the alias tables list the state, verify, init, phase, phases, validate, roadmap and eval families, with none for config-*), say so in the PR.

Minor 5 — the template test is coupled to workflow prose [verified/judgment] tests/config.test.cjs extracts the payload with /config-new-project '(\{.*\})' from gsd-core/workflows/*.md and fills a hand-maintained TEMPLATE_PLACEHOLDERS list; a new placeholder spelling fails with a parse error, and the order-dependent true|false replacement can mis-fill. Assert after filling that no [/| placeholder text remains.

Minor 6 — --raw and the dry-run text output are untested [verified] --raw is in the closed flag set (gsd-core/bin/gsd-tools.cjs) but no test passes it, and would create (dry run) is not asserted. One --raw --dry-run row covers both.

Minor 7 — scope beyond the brief [judgment, Standards and Spec axes] RULESET.PR-SCOPE.one-concern-per-pr=split unrelated changes into separate PRs. Beyond the Brief the PR adds non-object-choices refusal, empty-key and dotted-key refusal (with a DYNAMIC_KEY_PATTERNS carve-out), silent deletion of empty sections ({"bogus":{}} is dropped, not refused), a second-positional refusal, and moves the depth→granularity write to the user defaults file until after validation with a new options.persistDefaultsMigration parameter. The Brief says "produces the same config as today" and "defaults from the user defaults file are unchanged". Whether this is one concern is the maintainer's call; the finding stays.

Minor 8 — validateNewProjectChoices mutates its input [judgment] It rewrites normalized values and deletes empty sections in place while named validate*; return a normalized copy or rename it.

Nit 1 [judgment] validateConfigValue (about 300 lines after the move) documents "Exits non-zero through error()" and also returns the normalized value — two contracts in one function.
Nit 2 [verified] The dry-run masking test only covers empty defaults; with {"brave_search":"SECRETVAL"} the preview prints ****TVAL (as config-set does) and nothing pins masking or the absence of the plaintext.
Nit 3 [verified] -1 is reported as "Unknown flag" rather than "expected a JSON object"; any --prefixed token is treated as a flag.
Nit 4 [judgment] {"workflow":"x"} (a string for a section) reports Unknown config key "workflow", which misdescribes the problem.
Nit 5 [judgment] The flag/positional parsing in routeConfigNewProject duplicates routeConfigSet's ("mirroring routeConfigSet"), and the secret-mask loop in the dry-run preview is a third copy.
Nit 6 [judgment] Names: val/parsedValue/value for one concept; pendingDefaultsMigration holds serialized JSON, not a migration; persistDefaultsMigration is the negation of dryRun.
Nit 7 [judgment] The '--raw' entry in CONFIG_NEW_PROJECT_KNOWN_FLAGS is documented as a guard against a splice moving; it is dead today.
Nit 8 [judgment] buildNewProjectConfig's docstring says it "does NOT write any files" while it now ends with a deferred platformWriteSync; return the pending write as data and let the caller persist it.
Nit 9 [verified] docs/CLI-TOOLS.md does not mention that the dry-run preview masks secrets, and the PR template ticks Linux only although the matrix covers Windows and macOS.

Considered, not findings: no exploitable issue (see the security row); the already-supplied changeset has no (#5268) trailing backlink, which is a convention and not required; model_overrides.<agent> and agent_skills.<type> object values are not walked (same as config-set; a __proto__ key inside one is written as an inert own key and the old code accepted the same input).

Passes run

Pass	Result
/diagnose	run; CLI probes of the built head above
/root-cause-analysis	run; extracting shared validators addresses the cause (rules lived inline in cmdConfigSet), but the existence check still precedes validation (Minor 1)
/qa-test-architect	run; Majors 1 and 2, Minors 3, 5, 6
/skills-from-the-artificer (bugfix-review)	run; block below
/code-review	run as two parallel sub-agents: Standards (Majors 1 and 2 as rule violations, Minor 7, Nits 5-8) and Spec (Minors 1, 4, 7, Nit 4)
/security-review	run: identification sub-agent found no issue above 80% confidence (prototype-pollution, masking, symlinked defaults file, argv all checked by running the CLI)
Isolated adversarial pass	run by a fresh agent with no prior context; found Minors 1, 2 and Nits 2, 3 independently

Not run (Memtrace is unavailable): the Deterministic Review Pass (detect_changes, find_cross_module_issues, find_code_review_issues, find_yaml_rule_matches, review_github_pr), preflight_check, risk tiering (find_central_symbols, find_bridge_symbols), the recorded-decision check (recall_decision, governing_rules) and the co-change gate. None is reported clean. I did not execute any test (local node --test is not permitted); the Stryker rows are SKIPPED on this PR, so the 80% mutation threshold was not measured.

Laws that apply

Subject: PR #5284 makes config-new-project validate its choices with the key and value rules extracted from config-set, refuse unsupported flags and honor --dry-run.

• hyrums-law — fired: string-typed booleans, dotted keys, extra flags and extra positionals that were accepted now exit non-zero, under a Fixed changeset; and mode: null / ship.pr_body_sections: null behave differently from config-set (src/config.cts). Action: Minors 2 and 3.
• postels-law — fired: the edge is deliberately strict (the right direction), but the strictness differs between the two verbs for null, objects and coercion, so callers cannot rely on one contract. Action: Minor 2.
• zawinskis-law — fired: the change absorbs input-normalization concerns (silent empty-section deletion, defaults-file migration reordering) that the issue did not ask for. Action: Minor 7.
• goodharts-law — fired: a table of refused-side rows passes whatever the walker does on accepted limits, so green does not show parity. Action: Majors 1 and 2.
  Considered and cleared: kerckhoffs-principle (no guard or secret mechanism changed; masking behavior unchanged), kernighans-law (the long function is a pure move).

CI. The matrix executed on this head: 32 SUCCESS, 5 SKIPPED (auto-merge, Stryker matrix, Stryker mutation score, test (inert CI), Publish emitted-baseline artifact), none failing or pending; the branch is BEHIND next by one commit (a rebase is a CI condition, not a finding). I am not approving: Majors 1 and 2 and the Minors need a push.

CodeGraph Evidence

Memtrace unavailable: monthly quota exhausted and the server disconnected, so get_impact, get_symbol_context, recall_decision and find_code_review_issues were not run, and the recorded-decision check was not evaluated.

• codegraph_impact buildNewProjectConfig: 5 symbols (buildNewProjectConfig, cmdConfigNewProject, ensureConfigFile, cmdConfigEnsureSection, cmdConfigSetModelProfile); the signature change is an optional trailing argument and the three other callers see the later defaults-file write.
• codegraph_callers cmdConfigNewProject: dispatched from routeConfigNewProject in gsd-core/bin/gsd-tools.cjs; branch-new symbols (validateNewProjectChoices, validateConfigValue, assertKnownConfigKey) are absent from the main-checkout index, so the head CLI was executed instead.
• codegraph_node (file mode): cmdConfigSet region on base, confirming the value rules lived inline there; codegraph_explore on the alias tables, for Minor 4.
• Finding: the blast radius is the config verbs only; the risk is contract drift between config-new-project and config-set, not a wide break.

Verdict

Changes requested. Blocker 0, Major 2, Minor 8, Nit 9.

[0xdhx]

Thanks for the review. Every finding is addressed in 14 commits on top of the reviewed head 2281bd712; the head is now 9a68a06fb. Two are declined below with their reasons (Minor 4, Nit 7). The sweep turned up two more issues, both fixed. Before pushing, I had an independent model review the round in passes; its findings are folded into the commits below.

Majors

Finding	Commit	What changed
Major 1, no property test	53124548d	A property section at the end of tests/config.test.cjs runs both verbs in-process. It's in this file, not its own, because of lint-test-file-count. Each case draws 1 to 5 leaves from 14 keys, using values each key's rule accepts, values it refuses (for every key that has a rule), and null. (a) A payload is accepted exactly when config-set accepts every leaf; a refusal carries config-set's error for one of them; each accepted non-null leaf shows up in the dry-run preview exactly as config-set --dry-run reports it. (b) One unknown leaf inserted anywhere refuses the payload and writes nothing. At seed 42, 200 runs of (a) give 75 accepted and 125 refused, and the test asserts both outcomes occur. The fd capture uses suppressFdAsync (#4448). Against the build before the null fix, (a) fails on ship.pr_body_sections: null.
Major 2, boundary rows	187129139	The four numeric rules in the parity table each get rows at limit−1, limit and limit+1 on every end they have: hooks.context_warning_threshold −1/0/1/99/100/101, security_asvs_level 0–4, and context_window and drift_threshold 0/1/2. Each row checks config-set's verdict before its own. Positional count 0 (no choices, and --dry-run alone) joins the existing 1 and 2. Moving the ASVS upper bound to 4 turns the =4 row red.

Minors

Finding	Commit	What changed
Minor 1, existing config	99b95f73d	Choices are checked before the existence check, so a refused choice now fails even when config.json exists, with or without --dry-run. Valid choices on an existing config still report already_exists.
Minor 2, parity	791b08ac5, 68e015e4a, docs	null: a null choice is now treated as not chosen. Its key is still checked, and it takes the value it would have had without the choice. That is the creation-time counterpart of config-set <key> null, not an exact equal: with a user default set, config-set falls back to the loader's default instead. Per-entry settings: kept strict, and docs/CLI-TOOLS.md documents it as an intended difference. {"codex":5000} is accepted and {"a":1} refused, where config-set stores the whole object. agent_tools is now walked the same way (68e015e4a), so an invalid selector is refused as config-set agent_tools.<selector> refuses it; it had been taken whole. Coercion: documented as the other intended difference and pinned (Minor 3). The docs now list both differences instead of saying "the same rules".
Minor 3, string-for-boolean	20831c0c7	Pinned. "true" for workflow.research, "false" for git.create_tag, and "5" and "2" for context_window and security_asvs_level, are refused, while config-set accepts the same text (asserted).
Minor 4, command-aliases	none	Declined: the entry already exists. src/command-aliases.cts has config-new-project (aliases: [], mutation: true) in NON_FAMILY_COMMAND_ALIASES, and nothing about it changes. The verb still mutates without --dry-run, as config-set does with the same flag.
Minor 5, template coupling	20831c0c7	After filling, the test checks the payload's strings for a leftover "[…]" or a bare a|b|c alternation and fails by name. With the "[selected]" mapping removed, both template rows fail with that message.
Minor 6, --raw	20831c0c7	A --raw --dry-run row asserts the output is would create (dry run) and that nothing is written.
Minor 7, scope	c9fecafde	Empty sections: an empty object is no longer dropped silently. Under an unknown name it is refused with config-set's own error; under a real section it sets nothing. The rest: each addition guards an Acceptance Criterion. A dotted key writes a literal key no reader resolves, and a non-object payload writes index keys "0", "1", …: both are unvalidated keys reaching the config by another route (AC1). A second positional is an ignored input, like the ignored flag (AC3). The defaults-file rewrite is a write --dry-run must not make (AC3). The one-concern call is yours; I can split any of these out on request.
Minor 8, mutation	f967b3246	validateNewProjectChoices returns the checked choices as a new object and leaves its input as given.

Nits

Nit	Commit	Disposition
1, two contracts	f967b3246	The docstring states one contract: it returns the value to persist, or refuses.
2, preview masking	20831c0c7	Pinned: {"brave_search":"SECRETVAL"} previews exactly what config-set --dry-run prints, and the plaintext appears nowhere.
3, -1 as a flag	611d658d3	Fixed. A flag is now a dash or two followed by a letter, so -1 gets "expected a JSON object" and - gets "Invalid JSON".
4, section given a string	c9fecafde	A real section given a non-object is refused with "it is a section, so its value must be an object". The exception is a key that is itself a setting (agent_tools), where that setting's own rule applies, as on config-set.
5, duplication	f967b3246	The comment wrongly cited routeConfigSet, which has no closed flag set (it only reads --dry-run); it now cites todo complete (#4096). I haven't extracted helpers: the three masking sites have different shapes, and extraction would widen a diff Minor 7 already questions.
6, names / 8, deferred write	f967b3246, 73ff14e55	buildNewProjectConfig is split. planNewProjectConfig writes nothing and returns { config, defaultsRewrite }; persistDefaultsMigration and pendingDefaultsMigration are gone. cmdConfigNewProject persists the rewrite only after config.json is written (73ff14e55), so a failed write leaves the defaults file as it was. ensureConfigFile keeps its contract.
7, dead --raw	none	Kept: it's the sibling convention, since TODO_COMPLETE_KNOWN_FLAGS carries --raw with the same comment.
9, docs / platforms	20831c0c7, body	docs/CLI-TOOLS.md now says the preview masks secrets. The platforms section now says what I ran locally (Linux) and what CI runs: the macOS leg runs tests/config.test.cjs, but the Windows legs run tests/config.platform.test.cjs, so the new block is not exercised on Windows.

Found in the sweep

• 95cfdd9f0: {"project_code":7} was written as the number 7. config-set always stores project_code as the text typed (bug: config-set silently coerces values — context_window Infinity persisted as null while CLI reports Infinity (output ≠ disk) #1581), and its readers treat it as a string (an as string cast or a typeof string check). validateConfigValue now refuses a non-string project_code; through config-set the rule never fires.
• 68e015e4a: the agent_tools selector bypass described under Minor 2.

Verification

• Reversion controls: each of the round's seven fix commits (99b95f73d, 791b08ac5, c9fecafde, 95cfdd9f0, 73ff14e55, 611d658d3, 68e015e4a) fails a named test when its source change alone is reverted, with the tests kept.
• Required checks, run locally at 9a68a06fb: lint, plugin validation, docs and changeset lint, both detectors, security, default-flip-documentation, the QA loop walk, the targeted lane and all three Ubuntu shards pass; the hosted, metadata-only, macOS and Windows checks were not run locally. Shard 2 of an earlier capture went red when a watchdog killed one chunk under host load ~250 (0 assertion failures); it passed in full when re-run under normal load.
• Per commit: every commit from the base to 9a68a06fb was checked out, built and run through tests/config.test.cjs; each exits 0, and no test that runs at the final tree is skipped at any commit that has it, so the sequence is bisectable.
• CI on this PR: 50 check-run registrations on 9a68a06fb as of 09:54Z, 45 SUCCESS and 5 SKIPPED, none running. The Stryker mutation matrix did not run for this push, so this is not a full-matrix result.

[trek-e]

Review of head 9a68a06fb071 - substance clean, awaiting rebase

Summary. Every finding from the 2026-10-10 review is resolved or declined with a reason I verified on disk. Root cause (value rules inline in cmdConfigSet, flags dropped by the router, existence check before validation) is fixed at the source via the shared assertKnownConfigKey / validateConfigValue.

Prior findings. Major 1 (property): two fast-check properties, both outcomes asserted. Major 2 (boundaries): limit-1/limit/limit+1 rows present. Minors 1, 2, 3, 5, 6, 8 and Nits 1-4, 6, 8, 9: fixed and pinned. Minor 4: src/command-aliases.cts:754 already carries config-new-project; decline accepted. Nit 7: matches the todo complete convention (gsd-tools.cjs:3200); accepted.

Open for the maintainer. Minor 7 (scope: dotted-key, non-object, empty-section, second-positional and defaults-rewrite changes ride along); the author justifies each against the ACs and leaves the one-concern call to you. Nit 5 (third masking copy) left as is.

Nits. (1) Masking loop duplicated in the dry-run preview. (2) New test block is not exercised by the Windows legs (disclosed).

State. BEHIND next by 42 commits; merge-tree is clean. git fetch origin && git rebase origin/next, resolve nothing expected, push. No failing checks at this head; Stryker rows SKIPPED.

Memtrace Evidence

The Memtrace MCP server was unavailable for this review (it failed to connect: memtrace not found in PATH), so no graph-backed check ran. None of the four checks below is a clean result.

• get_impact: not run. Blast radius comes from reading the diff and grepping callers.
• get_symbol_context: not run. Callers and callees of the changed symbols were checked by grep.
• recall_decision: not run. The recorded-decision gate did not execute, so this does not show that no decision governs the change. ADRs and CONTEXT.md predicates were checked by grep instead.
• find_code_review_issues: not run. The deterministic review pass did not run. A manual diff review stood in for it, so the findings above are judgment, not graph facts.

[0xdhx]

Thanks. Rebased onto next at 433a43445: the 16 commits replay unchanged (git range-diff marks all 16 =). Three commits sit on top, and the head is now f1f36fb06.

Item	Commit	What changed
Rebase	—	Onto 433a43445. next has since moved; none of the newer commits touches this PR's five files.
Nit 1, masking loop	b00f1cdea	The preview loop now calls maskIfSecret from secrets.cjs, which is exactly the loop's body (isSecretKey, then maskSecret). Output is unchanged. This corrects my round-2 reply, which said the masking sites had different shapes: the preview's did not. The other sites in config.cts mask one key's value (config-set also masks that key's previous value), each inside an output shape of its own, and stay as they are.
Nit 2, Windows legs	none	No change. tests/config.test.cjs is on the macOS conformance list, not the Windows one; the body says so under Platforms tested.
Minor 7, scope	none	Your call, as before.

Found before pushing

A review of this round, before the push, refuted a claim the docs made: that config-new-project differs from config-set only in the two documented ways. A null choice under __proto__, prototype or constructor is a third. config-new-project '{"model_overrides":{"constructor":null}}' is refused by the prototype-pollution guard. config-set model_overrides.constructor null reaches that guard only when the config already has a model_overrides section: without one, _unsetNestedValue returns before its leaf guard and the command exits 0 with nothing removed. The default config has agent_skills but not model_overrides, agent_tools or features, so the difference can hold after creation.

104b4ac99 leaves config-set alone (out of scope for #5268), adds three rows that pin the new-project side for agent_skills against config-set run on a default config, and documents it in docs/CLI-TOOLS.md as the third intended difference. The PR body now lists three.

The same review turned up two config-set behaviours this PR does not touch; both are now in the body under Out of scope:

• config-set phase_commit_docs.2.1 true writes {"phase_commit_docs":{"2":{"1":true}}}, a shape the resolver does not read; config-new-project writes the "2.1" key it does read.
• With an invalid ship.pr_body_sections in the user defaults file, a null choice for it is refused at creation, while config-set ship.pr_body_sections null on an existing config clears the key.

I can file the first as its own issue if you want it tracked.

A correction to my round-2 reply

Its Verification listed default-flip-documentation among the checks that passed locally. That run had no PR event payload, so the script skipped. This round it ran against the PR body and reports no default value changed.

Verification

• Reversion control: with the null skip in validateNewProjectChoices moved above the segment guard, the three new rows fail and the other 287 tests in tests/config.test.cjs pass. With the maskIfSecret call replaced by the unmasked value, three named masking tests fail.
• Per commit: every commit from the base to 104b4ac99 was checked out, built and run through tests/config.test.cjs. Each exits 0, and no test that runs at the final tree is skipped at any commit that has it, so the sequence is bisectable.
• Required checks, run locally at 104b4ac99: lint, plugin validation, docs and changeset lint, both detectors, security, default-flip-documentation, the QA loop walk, the targeted lane and all three Ubuntu shards exit 0. The hosted, metadata-only, macOS and Windows checks were not run locally.
• Newer next: in a trial merge of b00f1cdea with 671aa6f0f (fix(#4840): register the context monitor only on hosts that install a metrics writer #5389, the first commit next gained after the rebase), tests/config.test.cjs passed 287/287, and fix(#4840): register the context monitor only on hosts that install a metrics writer #5389's added and modified tests passed.
• CI: the run on 104b4ac99 failed one job, conformance test (windows-latest, 24, shard 1/3). Its only file, tests/codex-config-agents.test.cjs, passed every test, then the child process did not exit before the 600 s chunk timeout. That file reaches none of the code this PR changes, and the same shard passed on next at 51f0c042e. f1f36fb06 is an empty commit to re-run it: 44 check-run registrations on f1f36fb06 as of 19:54Z, 39 SUCCESS and 5 SKIPPED, none running. The Stryker mutation matrix did not run, so this is not a full-matrix result.

[trek-e]

Review (re-review) — fix(#5268): config-new-project refuses the choices config-set refuses, and honors --dry-run

Head reviewed: f1f36fb0630ce010432bb674a63ead50f73b5d5f (fresh gh pr view this turn: mergeable MERGEABLE, mergeStateStatus BEHIND, reviewDecision CHANGES_REQUESTED, not draft). Merge base 433a43445; origin/next is now f7efbf110.

Summary

Re-review of trek-e's CHANGES_REQUESTED reviews (head 2281bd712, then 9a68a06fb). The second review already found every first-round finding resolved or declined with a verified reason and left Minor 7 (scope) to the maintainer. Since 9a68a06fb the PR was rebased onto 433a43445 and gained three commits: b00f1cdea (mask preview via maskIfSecret, closes the second review's Nit 1), 104b4ac99 (test + docs for a third config-set difference), f1f36fb06 (empty commit to re-run CI). No new defect in the delta. Remaining items are the carried maintainer-call scope finding and two nits.

Classification & gate compliance

• Track: linked #5268 is confirmed-bug (per trek-e's earlier verified read) -> Bug-Fix track. PR label is review: changes requested only (state label, not a classification label).
• Body: Fixes #5268, Fix template, not draft. Title fix(#5268): ... conventional.
• Changeset .changeset/calm-heron-checks.md: type Fixed, pr: 5284, bold lead, body ends at the period. Fixed needs no docs/ update; docs/CLI-TOOLS.md is updated anyway (and in the delta).
• No references/ or workflows/ file added, so no INVENTORY drift. No shipped agent/workflow/command markdown changed.
• Files changed (5): .changeset/calm-heron-checks.md, docs/CLI-TOOLS.md, gsd-core/bin/gsd-tools.cjs, src/config.cts, tests/config.test.cjs.
• Branch fix/... name not re-verified; head ref not shown in the fetched JSON (not a finding).

Must-Have Acceptance Checklist (#5268 Agent Brief, as quoted in the earlier review)

1. Unknown key: non-zero exit, Unknown config key, no config created. PASS (create path and, since 3b8478e8a, existing-config path).
2. mode outside interactive/yolo and any value config-set rejects: same message, no file. PASS (18 parity rows plus 3 forbidden-segment null rows, the latter new in 104b4ac99).
3. Unsupported flags refused or --dry-run prints and writes nothing. PASS (both; -1/- now reported as choices).
4. Workflow template keys still accepted, same config as today. PASS (tests fill both shipped templates and assert leftover-placeholder absence).
5. Existing-config idempotency and user-defaults unchanged. PASS for idempotency with valid choices; defaults-file rewrite now deferred until after the config write (see Minor 7).

Root-cause / functional verdict

Root cause (one sentence): cmdConfigNewProject spread caller choices into the config without the key/value rules that existed only inline in cmdConfigSet, and routeConfigNewProject dropped every flag silently. The fix extracts assertKnownConfigKey / validateConfigValue and runs them over the dotted leaves before any write, and closes the flag set. One owner for the rules: cmdConfigSet calls the same functions. Root-cause fix, not symptom handling.

Delta check:

• b00f1cdea src/config.cts:714: maskIfSecret(key, value) replaces isSecretKey then maskSecret. maskIfSecret is exported by src/secrets.cts (INVENTORY.md:721 lists it; src/init.cts:39 already imports it). Behavior-preserving; three masking tests named in the commit cover it.
• 104b4ac99: three rows (agent_skills.__proto__|prototype|constructor = null) assert the config-set positive control against a seeded default config first, then identical error text and nothing written (tests/config.test.cjs ~813-834). Specific refusal verdict, persisted-artifact check via assertNothingWritten. Docs text added at docs/CLI-TOOLS.md ~627-635 documents the third difference; consistent with the PR body.
• f1f36fb06: empty commit, no content.

Blast radius

CodeGraph (earlier review, not re-run for the delta since it touches one call site): buildNewProjectConfig has 5 impacted symbols (cmdConfigNewProject, ensureConfigFile, cmdConfigEnsureSection, cmdConfigSetModelProfile); the other callers see the deferred defaults-file write only. The delta changes one masking call and adds tests/docs. Risk is contract drift between the two config verbs, now documented.

Co-change completeness

src/config.cts + gsd-core/bin/gsd-tools.cjs + test + docs + changeset all present. src/command-aliases.cts already carries config-new-project (earlier Minor 4 declined, accepted by trek-e). Generated .cjs outputs are built in CI; no drift check red.

Memtrace Evidence

• get_impact: UNAVAILABLE — Memtrace MCP failed to connect (ENOENT).
• get_symbol_context: UNAVAILABLE — Memtrace MCP failed to connect (ENOENT).
• recall_decision: UNAVAILABLE — Memtrace MCP failed to connect (ENOENT). The recorded-decision gate did not execute; this is not a clean result.
• find_code_review_issues: UNAVAILABLE — Memtrace MCP failed to connect (ENOENT). The deterministic review pass did not run.
  Substituted evidence: git show of the three delta commits; git grep maskIfSecret refs/prrev/5284 (export documented in docs/INVENTORY.md:721, second consumer src/init.cts:2390); the prior reviews' CodeGraph impact of buildNewProjectConfig (5 symbols). No local test was run (node --test blocked; CI is the executor).

Test quality

Unchanged from the second review's verdict: two fast-check properties with both-outcome assertions, limit-1/limit/limit+1 rows, config-set positive control before each refusal, persisted-artifact assertions. Delta rows meet the same bar. No source-grep, no wall-clock assertions in the delta. The fail-first claim for the new rows (null skip moved above segment guard turns exactly three rows red) is the author's reversion control; I did not re-execute it.

CI (head f1f36fb06)

Latest-per-name collapse: 39 success, 5 skipped, 0 failing, 0 pending. action_required runs for this head: none. Skipped rows include the Stryker matrix and mutation score (not executed, so no mutation result; as the author states, this is not a full-matrix result). The prior Windows shard hang on 104b4ac99 was re-run green at this head; the author attributes it to an unrelated runner hang and I am only reporting that the re-run is green, not judging the cause. Branch is BEHIND next (merge-base 433a43445, next at f7efbf110); a rebase is a CI condition, not a finding: git fetch origin && git rebase origin/next.

Findings

• Minor 7 (scope beyond the Brief) — withdrawn: the maintainer accepted the extra refusals as in scope.
• Nit A (deterministic fact): f1f36fb06 is an empty commit titled ci: re-run after an unrelated Windows runner hang, with no issue scope; it adds history noise. Fix: drop it at the next rebase (git rebase origin/next with it dropped, or re-run CI from the Actions UI instead).
• Nit B (carried, disclosed): the new describe block in tests/config.test.cjs is not exercised by the Windows conformance legs (they run tests/config.platform.test.cjs); disclosed in the PR body. Judgment whether a platform-leg wiring is wanted.

Verdict

Changes requested under zero tolerance (nit-level only): all prior Majors and Minors are resolved and verified; the delta is clean; CI is green at f1f36fb06 (39 success, 5 skipped, 0 action_required). Substance is otherwise clean; before merge: drop the empty commit, rebase on origin/next, (Minor 7 scope question was ruled in scope by the maintainer).

[0xdhx]

Thanks. Rebased onto next at 755bf26f0; the head is now f329c43c8.

Item	Commit	What changed
Nit A, empty commit	none	Dropped. f1f36fb06 had its parent's tree and is gone from the branch; the other 18 commits replay unchanged (git range-diff marks all 18 =).
Rebase	none	101 commits. Four touch this PR's files (aeabede8e, e92fe7a63 and 5ae76464d in gsd-tools.cjs, 10ac3a7a9 in docs/CLI-TOOLS.md), none of them in this PR's hunks, and the replay had no conflict.
Nit B, Windows legs	none	No change; reason below.

This round has no fix commits, so there is no reversion control to report.

Nit B

Most of the block's tests reach the defaults store and the API-key files through os.homedir(): newProject() passes homeSandboxEnv(dir), which points HOME and USERPROFILE at the same temp dir, and the harness blanks GSD_HOME. On Windows os.homedir() reads USERPROFILE, so it resolves to the same sandbox there as HOME does on Linux and macOS. The few tests that seed or inspect the defaults file directly set GSD_HOME to a temp dir instead, which resolves the same way everywhere. The validation under test has no OS branch. gen-platform-conformance-tier.cjs also finds no platform signal in tests/config.test.cjs, though its check reads the file's text and does not follow the helper. If you want the block on the Windows legs anyway, I can move it into tests/config.platform.test.cjs.

Body

Three corrections. The negative-controls line cited the first commit as 2fbab96d4, its hash before the previous rebase; it is now e7179dff5. The flag census now has rows for a repeated global flag: the dispatcher splices only the first copy, so a second --json-errors reaches the route and is refused, as todo complete refuses it, and a second --raw is accepted because --raw is in the set. And the testing list said --raw --dry-run prints what a real run writes; it prints the text form would create (dry run), which is what its test asserts.

Verification

• The rebase: the tests that read this PR's two base-touched files (142 test files before the rebase, 143 after) were run at both ends: no test went from pass to fail. Every test file next added or modified since 433a43445 was run in a trial merge: no failures; three tests skip on this host (two macOS-only, one needing an en_US.UTF-8 locale), all from bug(ci): fixed time bounds in the macOS and Windows conformance tiers fail intermittently and red next #5473's validate-commit work.
• tests/config.test.cjs: 290 of 290 at this head. Every commit of the PR was checked out, built and run through it: each exits 0, and no test that runs at the head is skipped at any commit that has it, so the sequence stays bisectable.
• Locally at f329c43c8: lint:ci, plugin validation, docs and changeset lint, both detectors, security, default-flip-documentation, the QA loop walk, the targeted lane and all three Ubuntu shards (shard 1 with integration, security, install and slow) exit 0. The hosted, metadata-only, macOS and Windows checks were not run locally.
• CI: 50 check-run registrations on f329c43c8 as of 03:41Z, 45 SUCCESS and 5 SKIPPED, none running. The Stryker mutation matrix did not run, so this is not a full-matrix result.

[trek-e]

Review (re-review 3) - fix(#5268): config-new-project refuses the choices config-set refuses, and honors --dry-run

Head reviewed: f329c43c882b7872f5ae377977e55ad8427fb88e (fresh gh pr view this turn: mergeable MERGEABLE, mergeStateStatus BEHIND, reviewDecision CHANGES_REQUESTED). Merge base 755bf26f0; origin/next is 0dd184555, 4 commits ahead. git merge-tree of head with origin/next is clean.

Summary

Third review. Prior on-record reviews are trek-e's at 2281bd712, 9a68a06fb and f1f36fb06. The last one left two nits: Nit A (empty commit f1f36fb06) and Nit B (new test block not on the Windows legs). Since f1f36fb06 the author rebased onto 755bf26f0 and dropped the empty commit. Nit A is resolved. Nit B is declined with a reason. The author states git range-diff marks all 18 remaining commits =. I confirmed the PR's own diff against 755bf26f0 touches exactly the five PR files and matches the reviewed content (no hunk in src/config.cts, gsd-core/bin/gsd-tools.cjs, docs/CLI-TOOLS.md or the changeset differs from what the prior review read). The three other PR comments are all from the author; none is an unanswered question.

Classification & gate compliance

• Linked #5268 labels: bug, confirmed-bug -> Bug-Fix track. PR carries only the state label review: changes requested.
• Fixes #5268 present (GitHub closingIssuesReferences resolves it). Title fix(#5268): ... conventional. Not a draft. Fix template used. Branch fix/5268-config-new-project-validate-choices matches <prefix>/NNN-slug.
• Changeset .changeset/calm-heron-checks.md: type Fixed, pr: 5284, bold lead, ends at the period. Fixed needs no docs update; docs/CLI-TOOLS.md documents the verb anyway.
• No references/ or workflows/ file added: no INVENTORY drift. No agent/workflow/command markdown changed.
• One concern: the dotted-key, non-object, empty-section and second-positional refusals were ruled in scope by the maintainer in the previous review.
• ADR gate: no ADR governs this verb that I could find; recall_decision unavailable (see Memtrace Evidence).

Must-Have Acceptance Checklist (#5268 Agent Brief, verbatim)

1. "Unknown key in choices: non-zero exit, Unknown config key error, no config file created." PASS. validateNewProjectChoices runs assertKnownConfigKey per leaf before planNewProjectConfig/any write.
2. "mode outside interactive/yolo (and any other value config-set rejects for that key): non-zero exit with the same message, no file created." PASS. validateConfigValue is the extracted cmdConfigSet body; both verbs call it, so the error text is the same code path. 18 parity rows + 3 forbidden-segment rows.
3. "Unsupported flags are refused, or --dry-run prints the config and writes nothing." PASS (both): closed flag set in routeConfigNewProject; dryRun returns before mkdirSync/platformWriteSync.
4. "The full set of keys the new-project workflow template emits ... is still accepted and produces the same config as today." PASS per the tests that fill both shipped templates.
5. "Existing-config idempotency (created: false) and defaults from the user defaults file are unchanged." PASS for valid choices. The defaults-file depth->granularity rewrite now runs after the config write (persistDefaultsRewrite), which keeps buildNewProjectConfig behaviour for ensureConfigFile.
   Out of scope per Brief: config-set behavior, validation of user-defaults contents, template keys. None changed.

Root-cause / functional verdict

Root cause, one sentence: cmdConfigNewProject spread caller choices into the config with none of the key/value rules that existed only inline in cmdConfigSet, and routeConfigNewProject forwarded only args[1], dropping every flag. The fix is at the source: the key gate (assertKnownConfigKey) and the value block (validateConfigValue) are extracted and called by both verbs, so there is one owner for the rules and no parallel validator. Not special-casing.

Independent read of the diff:

• Prototype-pollution: Object.keys on JSON.parse output includes an own __proto__ key. Every path that would assign checked[key] for such a key either errors in assertKnownConfigKey (unknown key) or in the segment guard before assignment. checked is only assigned after the guard. No pollution path found.
• Order of checks matches config-set (key, value, then setter's segment guard).
• --dry-run with existing config.json reports already_exists with no preview; documented in docs/CLI-TOOLS.md.
• Subprocesses: none added.

Blast radius

buildNewProjectConfig keeps its signature and contract (plan, persist, return); ensureConfigFile, cmdConfigEnsureSection, cmdConfigSetModelProfile callers see only the deferred defaults write. cmdConfigSet now calls the two extracted functions with identical bodies. cmdConfigNewProject gains an optional fourth parameter (additive). Generated gsd-core/bin/lib/config.cjs is gitignored (.gitignore:329), built in CI. No exported-surface removal.

Co-change completeness

Present: src/config.cts, gsd-core/bin/gsd-tools.cjs, tests, docs/CLI-TOOLS.md, changeset. config-new-project already has its entry in src/command-aliases.cts (accepted by trek-e in review 2). The 4 commits on next since the merge base (.changeset/brave-search-no-redirect.md, .github/workflows/test.yml, src/commands.cts and CI diagnostics) touch none of this PR's files.

Memtrace Evidence

• get_impact: UNAVAILABLE - Memtrace MCP failed to connect (ENOENT).
• get_symbol_context: UNAVAILABLE - Memtrace MCP failed to connect (ENOENT).
• recall_decision: UNAVAILABLE - Memtrace MCP failed to connect (ENOENT). The recorded-decision gate did not execute; this is not a clean result.
• find_code_review_issues: UNAVAILABLE - Memtrace MCP failed to connect (ENOENT). The deterministic review pass did not run; the findings below are manual-diff judgment unless marked as fact.
  Substituted evidence: gh pr diff-equivalent git diff 755bf26f0 refs/prrev/5284 for all five files; git merge-tree against origin/next; compare API f1f36fb06...f329c43c8 (rebase only, 119 files from next, PR files identical per range-diff); git ls-tree/check-ignore for the generated file; PR-comment and review history. No CodeGraph call was needed beyond the diff because the changed symbols and callers are all in src/config.cts. No local test run (node --test blocked; CI is the executor).

Test quality

Unchanged from review 2: two fast-check properties with both-outcome assertions, limit-1/limit/limit+1 rows for the four numeric rules and the positional count, config-set positive control before each refusal, assertion on the persisted artifact (assertNothingWritten, defaults file byte-identical). No wall-clock assertions. The readFileSync hits in the diff read config.json, the defaults file and workflows/*.md templates (not a source .cjs/.js/.ts), and .split('.') is on key paths: no local/no-source-grep exposure, no CRLF-split exposure. Fail-first for the new rows rests on the author's reversion controls, which I did not re-execute.

CI (head f329c43c8)

Latest-per-name collapse of 50 registrations (37 names): 32 success, 5 skipped, 0 failing, 0 pending. Skipped: auto-merge, Publish emitted-baseline artifact, Stryker (${{ matrix.name }}), Stryker mutation score (changed files only), test (inert CI). Stryker did not execute, so this is not a mutation-matrix result. action_required runs for this head: none. macOS conformance, Windows conformance shards 1-3, ubuntu test shards 1-3, Required tests, Coverage gate, lint-tests, docs-lint, changeset-lint, Base branch health all success.

Findings

• Nit B (carried, judgment): tests/config.test.cjs (the new describe block, ~800 lines) is not run by the Windows conformance legs, which run tests/config.platform.test.cjs. The author's reason (no OS branch in the validation, HOME/USERPROFILE both sandboxed) is plausible but unverified, and the author's own note says the platform-tier generator "reads the file's text and does not follow the helper". Fix: move the platform-sensitive subset (defaults-file and API-key-file cases that go through os.homedir()) into tests/config.platform.test.cjs, or confirm the decline.
• Nit C (deterministic fact): docs/CLI-TOOLS.md:627 is a 120-column line inside a hard-wrapped bullet whose neighbours wrap near 100 (... refused, where \config-set` stores any object given to the setting whole. `review.max_prompt_tokens_per_reviewer` also`). Fix: rewrap that bullet.
• State, not a finding: branch is BEHIND next by 4 commits and merge-tree is clean. git fetch origin && git rebase origin/next, push.

Verdict

Changes requested under zero tolerance (nit-level only). Substance is clean: all Majors and Minors from reviews 1 and 2 are resolved, Nit A is resolved, CI is green at f329c43c8 with Stryker skipped, no action_required runs. Remaining: Nit B (maintainer's call), Nit C, and a rebase onto origin/next.