* feat(#3243): sync installed codex .toml model/effort to the passive posture Implements ADR-2313 D7, and owns the Codex .toml typed IR that Phase 1's review assigned to this phase. The IR exists for a structural reason, not tidiness: this phase has to PARSE these files, and a parser kept bug-compatible with a separate renderer is the generative-fix-divergence shape this epic already dealt with once for the model predicate. So Phase 2's parsing MOVES here rather than being copied — agent-install-check now imports it, and its test file passing unchanged is the proof the extraction altered nothing. The load-bearing property is byte-identical round-trip: render(parse(x)) === x. Without it a sync silently reformats a user's file — line endings, key order, BOM, trailing newline — turning a two-line repair into a whole-file diff in their dotfile repo. The IR keeps original lines and removes targeted ones rather than reconstructing from parsed fields, which is what makes that property hold. It also reconciles a real contradiction between Phase 2 and ADR-2313. An unterminated developer_instructions block: the reader excludes the rest of the file, deliberately failing toward a false positive, because misreading prose as a pin only wastes a user's time. The writer must refuse, because proceeding on a malformed document rewrites it. A false positive is the safe direction for a reader and the dangerous one for a writer. So the parse reports the fact and the two consumers branch on it — one parse, one truth, two policies, instead of two parsers that agree today. The sync leaves a legal real-Codex pin and its coupled effort untouched, reported skipped rather than synced; strips a stale Anthropic or tier model and an orphaned effort; keeps dry-run as the default; refuses any file whose parse fails; and skips symlinks exactly as the Claude path already did. The Claude path itself is byte-identical. PARSE_REASON.NO_HEADER from the ADR's illustrative snippet is deliberately not implemented — a missing header is legal, not an error, so it would be a dead enum member that the enum-lock test then pins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3243): preserve per-line endings and make the codex write atomic Two findings from an isolated review, both in the write path. BLOCKER: mixed line endings broke the byte-identical round-trip. `eol` was a single whole-file flag and split(/\r?\n/) discarded each line's own terminator, so render re-joined with ONE style and normalized every line — even with zero strips performed. A file with one CRLF line and the rest LF came back fully converted. That falsified the A14 guarantee, violated the design's "must not silently rewrite every line", and made the CONTEXT.md glossary claim wrong. It was untested because A12 and B15 only cover PURE CRLF; no mixed-ending fixture existed anywhere. Fixed by keeping each line's terminator alongside its content, so render is a plain concatenation and a strip removes only the target line and its own terminator. `eol` survives as informational metadata that render never reads. Seven fixtures added for the paths nothing exercised: mixed endings unmodified and with a strip, a lone \r, a file ending on the block's closing ''' with no newline, multiple trailing newlines, a BOM-only file, and an empty file. MINOR, but it contradicted this phase's own contract: the write was in-place open-truncate, so a failure between truncate and completion leaves a truncated .toml — exactly what ADR-2313 says must never happen. The Codex path now writes a sibling temp file and renames over the target, which is atomic on one filesystem, with cleanup on failure. It uses the repo's existing retryRenameSync rather than a hand-rolled rename, and deliberately NOT platformWriteSync, whose normalizeContent would mangle the very CRLF and trailing-newline bytes the round-trip property exists to preserve. The Claude path keeps its in-place write untouched. It has the same shape, but changing it is not this phase's business and its tests must stay byte-identical. B20 previously mocked writeFileSync to throw BEFORE touching anything, so it proved nothing about a mid-write failure — its passing comment was true only because of how the mock was built. It now performs a real truncated write wherever writeFileSync is called, catching both the naive direct-to-target path and the new temp path, and asserts the target is byte-identical afterwards with no stray temp file left. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3243): preserve the trailing-newline state when stripping a last line Caught by B17, one of this phase's own tests — the suite working, not a test problem. Content is reconstructed as the concatenation of lines[k] + terminators[k], so a file with no trailing newline has '' as its last terminator. removeLine spliced out both arrays at the same index, which is right for a middle line but wrong for the last one: it dropped the empty terminator and left the PREVIOUS line's newline in place. A file ending `...\nmodel = "sonnet"` with no trailing newline came back as `...\n`, gaining a newline the user never wrote. The new last line now inherits the removed line's terminator, so a removal leaves the file exactly as if that line had never been written. Removing the only line yields an empty file rather than a stray terminator. Both stripModel and stripReasoningEffort funnel through the one removeLine, confirmed rather than assumed, so a single fix covers both — including the row-B7 shape where a stale model and its orphaned effort are removed in sequence and the second removal targets the last line. Two of the four new cases are honestly not red-first and say so in their comments: removing a last line that HAS a trailing newline only exposes the bug under mixed EOL, since uniform files coincidentally have equal terminators on both sides; and removing the only line already degenerated correctly through Array.slice. They are kept as guards for the new branch rather than dressed up as catches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(#3243): document the codex repair path and close the loop How-to: the Codex-400 entry added in Phase 2 told users to re-run the installer, because that was the only repair available then. It now leads with `effort sync` and keeps the reinstall as the alternative, with the reason to prefer one — a reinstall regenerates the agent files wholesale, so anyone who hand-edited theirs loses those edits. Detect, preview, apply is now one continuous path in one place. Reference: docs/COMMANDS.md had no `effort sync` entry at all — the same gap `validate agents` had in Phase 2, found the same way. The entry documents BOTH runtimes, because the command genuinely forks on runtime and describing only the new half would misdescribe it. The write flag is `--apply`. The design doc and test matrix both said `--no-dry-run` throughout, which does not exist — verified against the actual arg parser in gsd-tools.cjs before writing. Documenting a flag that does not exist is worse than documenting nothing, because it fails at the moment someone needs it. Both surfaces state that only the targeted lines are removed and every other byte is preserved. That is a user-visible guarantee rather than an implementation note: it is the difference between a two-line diff and a reformatted file in someone's dotfile repo, it is what the IR's round-trip property exists to deliver, and writing it down makes it a contract a future change has to break knowingly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3243): inherit the trailing-newline state, not the line ending style My previous rule was subtly wrong and this phase's own test caught it. "The new last line inherits the removed line's terminator" copies the removed line's STYLE as well as its presence. A26 uses mixed endings on purpose — line one terminated \r\n, the model line terminated \n — so inheriting silently rewrote line one's ending to \n. That is precisely the defect class the mixed-EOL blocker fix existed to eliminate, reintroduced one layer down by the fix for it. The correct rule inherits the EMPTINESS only. If the removed line had no terminator, the new last line loses its own, preserving "this file has no trailing newline". Otherwise the new last line keeps its own terminator: it is already a newline, and already the right style for that line. A26's assertion moved too, and that deserves saying plainly rather than burying: it previously encoded my wrong rule. Changing a test to match the implementation is usually the mistake, so it was checked from first principles instead — a file whose first line ends \r\n and whose last line ends \n, with that last line removed entirely, must be the first line with its own \r\n intact. The new expectation is what the user's file should actually look like; the old one was wrong. A29 adds the interaction nothing covered: the compounding case (strip a stale model, then its orphaned effort, the second removal landing on the last line) with non-uniform endings either side. The two fixes meet there and nothing exercised the meeting point. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(#3243): drop the phantom trailing line from the IR representation Root cause, not another patch on the removal rule. Three consecutive fixes there each surfaced the next issue, which was the signal that the data model was wrong. splitPreservingTerminators left a phantom empty final entry for any file ending in a newline: "a\nb\n" became lines ['a','b','']. So for the common case the real last content line was NOT the last array element, removeLine's isLastLine check never matched it, and every rule I gave was reasoning about the wrong element. What hid it: render was already a plain concatenation, so a phantom empty line with an empty terminator contributes nothing to the output. A14's byte-identical round-trip could never have caught it — the defect is byte-neutral until a removal shifts the index arithmetic under it. That is worth recording, because "the round-trip test is green" was exactly the reassurance that kept the search pointed elsewhere. The representation is now 1:1 — terminators[i] follows lines[i] and may be '' — with no phantom, verified across empty, no-trailing-newline, trailing-newline, blank-line and mixed-CRLF inputs. render stays a plain concat and needs no special cases. With the phantom gone the removal rule is correct as stated and finally applies to the genuinely last element. Consumers checked rather than assumed: the block-range detector and header scanner are agnostic to array shape, and Phase 2's reader uses its own independent split, so tests/agent-install-check.test.cjs is untouched and still passes unchanged. One test expectation was wrong and is corrected rather than quietly adjusted: A18 asserted a 7-element terminators array whose trailing '' was the phantom itself. It now asserts the six real terminators, which is what the invariant lines.length === terminators.length requires. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(#3243): backfill changeset pr number (#3296) --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/gentle-wasps-wave.md
Normal file
5
.changeset/gentle-wasps-wave.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Added
|
||||
pr: 3296
|
||||
---
|
||||
**`effort sync` now repairs stale Codex `.toml` files without a reinstall** — on a `codex` install it strips a `model` pin that Codex rejects (a tier alias or a `claude-*` id) and an orphaned `model_reasoning_effort`, so agents fall back to the always-available session model. An explicit real-Codex pin is left alone. It is a **dry run by default** — pass `--apply` to write — and only the offending lines are removed: line endings, BOM, comments, key order, and any keys you added by hand are preserved byte-for-byte, so a repair is a two-line diff rather than a reformatted file. A file that cannot be parsed is refused and reported, never partially rewritten, and writes are atomic. Pairs with `validate agents`, which detects the same drift. The `claude` path is unchanged. (#3243)
|
||||
1
.gitignore
vendored
1
.gitignore
vendored
@@ -240,6 +240,7 @@ build/
|
||||
/gsd-core/bin/lib/onboard-projection.cjs
|
||||
/gsd-core/bin/lib/agent-command-router.cjs
|
||||
/gsd-core/bin/lib/agent-install-check.cjs
|
||||
/gsd-core/bin/lib/codex-agent-toml.cjs
|
||||
/gsd-core/bin/lib/task-command-router.cjs
|
||||
/gsd-core/bin/lib/validate-command-router.cjs
|
||||
/gsd-core/bin/lib/workstream-inventory.cjs
|
||||
|
||||
@@ -203,6 +203,9 @@ Leaf module owning the **static** model tables and the closed vocabularies deriv
|
||||
### Model Resolver Module
|
||||
Module owning model and effort resolution policy: resolves the model, runtime tier, planning granularity, reasoning effort, and fast-mode for a given agent by reading project config and resolving against the model profiles and catalog (`resolveModelInternal`, `resolveModelPolicy`, `resolveTierEntry`, `resolveModelForTier`, `resolveGranularityInternal`, `resolveEffortInternal`, `resolveFastModeInternal`, `resolveEffortForTier`, `nextEffort`, `assertValidGranularityOverride`). Depends only on leaf modules (`config-loader` for `loadConfig`, `configuration` for defaults, `model-profiles` and `model-catalog` for the static tables) — no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2f (#888) — the final core.cts decomposition step; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. **`CLAUDE_AGENT_ALIASES` no longer lives here** — it moved down to the Model Catalog Module (#3241, ADR-2313 Phase 1) so the Agent Install Check and Codex-sync surfaces can consume the alias rule without taking a `config-loader` dependency this module would have dragged with it; it is still **re-exported** from here, so existing importers (`bin/install.js`, `tests/codex-config.test.cjs`) are unaffected and a parity test asserts both modules expose the same set. Source of truth: `gsd-core/bin/lib/model-resolver.cjs` (generated from `src/model-resolver.cts`).
|
||||
|
||||
### Codex Agent TOML Module
|
||||
A **genuine leaf** (node builtins only) owning the typed IR for `~/.codex/agents/<agent>.toml` (#3243, ADR-2313 Phase 3). It is a **document model, not a policy** — it knows how to parse/render/strip the two keys the posture owns (`model`, `model_reasoning_effort`); it does NOT know which `model` values are illegal for Codex (that predicate, `isAnthropicFlavoredModel`, stays in the Model Catalog Module and the caller decides what to strip). `parseCodexAgentToml(content) → {ok:true,doc} | {ok:false,reason}` is the STRICT half: `PARSE_REASON.UNTERMINATED_BLOCK` when the `developer_instructions` block is opened but never closed, because a writer that proceeds on a malformed document risks rewriting it. `renderCodexAgentToml(doc)` round-trips **byte-identically** for an unmodified doc — the load-bearing property that stops a sync from silently reformatting a user's file — by keeping the original `lines` array (never re-derived) plus the detected `eol`/BOM/trailing-newline metadata, and rejoining rather than reconstructing. `stripModel`/`stripReasoningEffort` remove exactly one targeted line, re-indexing the block range and the sibling key's line index; every other line (comments, hand-added keys, the prompt block, line endings) is untouched. `scanTomlLines`/`stripBOM`/`findDeveloperInstructionsBlockRange`/`unquoteTomlValue` are the LENIENT reader primitives — moved here (not copied) from the Agent Install Check Module (#3242, Phase 2), which still imports and calls them directly, unchanged in behavior: an unterminated block falls back to "rest of file is inside the block" rather than failing, because misreading prompt prose as a pin is only a false positive. One block-range detector (`findDeveloperInstructionsBlockRange`, now carrying a `terminated` flag the strict parser reads and the lenient scanner ignores) serves both policies, so the reader and the writer can never silently diverge on where the block ends. Consumed by the Codex `.toml` sync (Commands Module's `cmdEffortSyncCodex`, ADR-2313 D7) and the Agent Install Check Module's `checkCodexModelPosture`. Source of truth: `gsd-core/bin/lib/codex-agent-toml.cjs` (generated from `src/codex-agent-toml.cts`). See Agent Install Check Module, Model Catalog Module, ADR-2313.
|
||||
|
||||
### Package Identity Module [Planned]
|
||||
Single seam owning GSD's published-package coordinates so a repoint/rename is a one-line change instead of a tree-wide sweep. Source of truth is `package.json`; values are *derived*, not re-typed: `packageName` (`.name` → `@opengsd/gsd-core`), `binName` (`Object.keys(.bin)[0]` → `gsd-core`), `repoSlug` (parsed from `.repository.url` → `open-gsd/gsd-core`), plus derived `changelogRawUrl` and `manualInstallCommand({ scope, runtime })`. Generated `.cjs` per ADR-457 (generated-single-source); shipped under `gsd-core/bin/lib/`. Three consumer worlds: **Node** consumers `require()` it at runtime (worker, `check-latest-version.cjs`, `bin/install.js`); the **bash launcher** snippet receives the literal injected by `scripts/sync-runtime-launcher.cjs` at sync time; **prose/help** literals (`update.md`, installer help) carry a committed copy. A drift-guard lint (`scripts/lint-package-identity-drift.cjs`, sibling to `check:alias-drift`) fails CI on any raw package/repo literal outside `package.json`, the generated module, and the value-checked materialization sites — this is what keeps the seam real (`two adapters`, not one). Replaces the contradictory pair it consolidates: the runtime-broken `require('../package.json').name` in `hooks/gsd-check-update-worker.js` (#378, resolves to `undefined` post-install) and the hardcoded constant in `check-latest-version.cjs` (#2992). _Avoid_: "package name string", "the npm name" (when you mean the seam). See ADR-457 and Installer Module.
|
||||
|
||||
|
||||
@@ -1738,6 +1738,41 @@ node gsd-tools.cjs roadmap upgrade --convention milestone-prefixed --apply # ap
|
||||
|
||||
## State Management Commands
|
||||
|
||||
### `effort sync`
|
||||
|
||||
Re-align installed agent files with your current effort and model configuration, without a full reinstall.
|
||||
|
||||
**Prerequisites:** GSD installed for a runtime
|
||||
**Produces:** A structured change report; writes only with `--apply`
|
||||
|
||||
```bash
|
||||
node gsd-tools.cjs effort sync # dry run — reports, writes nothing
|
||||
node gsd-tools.cjs effort sync --apply # write the changes
|
||||
```
|
||||
|
||||
| Flag | Description |
|
||||
|------|-------------|
|
||||
| `--apply` | Write the changes. **Omitted is a dry run** — the default reports and touches nothing |
|
||||
| `--dry-run` | Explicit dry run (the default) |
|
||||
| `--runtime <name>` | Override the runtime instead of reading it from config |
|
||||
| `--config-dir <path>` | Point at a specific runtime config directory |
|
||||
|
||||
**On `claude`** it re-syncs the `effort:` frontmatter of installed `gsd-*.md` agents.
|
||||
|
||||
**On `codex`** it repairs `.toml` files that drift from the passive model posture ([ADR-2313](adr/2313-codex-passive-model-posture.md)) — the counterpart to the detection that [`validate agents`](#validate-agents) performs:
|
||||
|
||||
| Situation | What happens |
|
||||
|---|---|
|
||||
| `model` pins a tier alias or a `claude-*` id | the `model` line is removed, so the agent inherits the session model |
|
||||
| `model_reasoning_effort` with no `model` | the orphaned effort line is removed ([#838](https://github.com/open-gsd/gsd-core/issues/838)) |
|
||||
| `model` pins a real Codex id | **left untouched**, reported `skipped` — an explicit pin is yours to keep |
|
||||
| the file cannot be parsed | **refused and reported** — never partially rewritten |
|
||||
| the file is a symlink | skipped, as on the Claude path |
|
||||
|
||||
Only the targeted lines are removed. Line endings, BOM, key order, comments, blank lines, and any keys GSD does not itself emit are preserved byte-for-byte, so a repair shows up as a two-line diff rather than a reformatted file. Writes are atomic — the file is either its old contents or its new ones, never a partial write.
|
||||
|
||||
---
|
||||
|
||||
### `validate agents`
|
||||
|
||||
Check that the GSD agents are installed for the active runtime — and, on Codex, that the installed `.toml` files satisfy the passive model posture.
|
||||
|
||||
@@ -339,6 +339,7 @@
|
||||
"clock.cjs",
|
||||
"clusters.cjs",
|
||||
"code-review-flags.cjs",
|
||||
"codex-agent-toml.cjs",
|
||||
"command-aliases.cjs",
|
||||
"command-arg-projection.cjs",
|
||||
"command-roster.cjs",
|
||||
|
||||
@@ -458,6 +458,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`.
|
||||
| `clock.cjs` | Injectable clock seam (now/sleep) for deterministic lock testing |
|
||||
| `clusters.cjs` | Skill cluster definitions for the runtime surface module (ADR-0011 Phase 2) |
|
||||
| `code-review-flags.cjs` | Typed flag parser for `/gsd-code-review`; exports `parseCodeReviewFlags(argv)` (→ `{ fix, all, auto, depth, files }`) and `resolveCodeReviewWorkflow(flags)` (→ `'code-review.md' \| 'code-review-fix.md'`); canonical dispatch seam for `--fix`/`--all`/`--auto` routing |
|
||||
| `codex-agent-toml.cjs` | Typed IR (genuine leaf) for `~/.codex/agents/<agent>.toml` — `parseCodexAgentToml`/`renderCodexAgentToml` round-trip byte-identically; `stripModel`/`stripReasoningEffort` remove exactly one targeted line; `scanTomlLines`/`stripBOM`/`findDeveloperInstructionsBlockRange`/`unquoteTomlValue` are the lenient reader primitives moved here from `agent-install-check.cjs` (#3242 Phase 2); consumed by the Codex `.toml` sync (`commands.cjs cmdEffortSyncCodex`, ADR-2313 D7, #3243) |
|
||||
| `command-aliases.cjs` | Alias/subcommand metadata for manifest-backed family routers |
|
||||
| `commonjs-marker.cjs` | Ownership-guarded `{"type":"commonjs"}` marker used to pin GSD's staged `.js` scripts to CommonJS; exports `classifyMarker` (absent/gsd-owned/foreign, fail-closed), `ensureCommonJsMarker`, and `removeCommonJsMarker` so install and uninstall share one predicate and never touch a user-authored `package.json` (#2544) |
|
||||
| `command-arg-projection.cjs` | Typed flag and positional argument projection helpers shared across command-family routers |
|
||||
|
||||
@@ -300,13 +300,32 @@ One thing the check deliberately does not inspect: an agent file that is a **sym
|
||||
rather than followed, matching how the effort sync treats them. If you symlink your agent configs,
|
||||
verify those targets by hand.
|
||||
|
||||
**Fix:** re-run the installer. Current versions write no model at all, so agents inherit the session
|
||||
model:
|
||||
**Fix — repair in place, without a reinstall.** Preview what would change:
|
||||
|
||||
```bash
|
||||
node gsd-tools.cjs effort sync
|
||||
```
|
||||
|
||||
That is a dry run; it writes nothing. When the reported changes look right, apply them:
|
||||
|
||||
```bash
|
||||
node gsd-tools.cjs effort sync --apply
|
||||
```
|
||||
|
||||
It removes only the offending `model` / `model_reasoning_effort` lines. Everything else — your line
|
||||
endings, comments, key order, and any keys you added by hand — is preserved byte-for-byte, so the
|
||||
result is a two-line diff rather than a reformatted file. An explicit real-Codex pin is left alone,
|
||||
and a file it cannot parse is refused and reported rather than partially rewritten.
|
||||
|
||||
**Or re-run the installer**, which rewrites the agent files wholesale. Current versions write no
|
||||
model at all, so agents inherit the session model:
|
||||
|
||||
```bash
|
||||
npx @opengsd/gsd-core@latest --codex --global
|
||||
```
|
||||
|
||||
Prefer the sync if you have hand-edited your `.toml` files — a reinstall regenerates them.
|
||||
|
||||
If you are on an **API-key** account and genuinely want a pinned model, name a real Codex model id
|
||||
per agent instead — see [How to configure model profiles](configure-model-profiles.md#codex-does-not-do-tier-routing--pin-explicitly-instead).
|
||||
|
||||
|
||||
@@ -200,6 +200,8 @@ export default tseslint.config(
|
||||
'gsd-core/bin/lib/onboard-projection.cjs',
|
||||
'gsd-core/bin/lib/agent-command-router.cjs',
|
||||
'gsd-core/bin/lib/agent-install-check.cjs',
|
||||
// ADR-2313 Phase 3 (#3243): tsc-generated runtime artifact — lint the src/codex-agent-toml.cts source.
|
||||
'gsd-core/bin/lib/codex-agent-toml.cjs',
|
||||
'gsd-core/bin/lib/task-command-router.cjs',
|
||||
'gsd-core/bin/lib/validate-command-router.cjs',
|
||||
'gsd-core/bin/lib/workstream-inventory.cjs',
|
||||
|
||||
@@ -21,6 +21,12 @@ import { getDirName, NO_LOCAL_CONFIG_DIR_SENTINEL } from './runtime-name-policy.
|
||||
// consume it without dragging model-resolver's config-loader chain into a
|
||||
// pure read/verify surface.
|
||||
import { isAnthropicFlavoredModel } from './model-catalog.cjs';
|
||||
// #3243 — the Codex `.toml` block-range/BOM/scan primitives moved into the typed
|
||||
// IR module (Phase 3), which this reader now imports rather than defining
|
||||
// locally. Behavior is unchanged: scanTomlLines/stripBOM here are the exact
|
||||
// same lenient functions that used to live in this file — see
|
||||
// codex-agent-toml.cts's module header for the reader/writer reconciliation.
|
||||
import { stripBOM, scanTomlLines } from './codex-agent-toml.cjs';
|
||||
|
||||
interface AgentsInstalledResult {
|
||||
agents_installed: boolean;
|
||||
@@ -70,93 +76,6 @@ function truncatePostureValue(value: string): string {
|
||||
return value.length > 64 ? `${value.slice(0, 64)}…` : value;
|
||||
}
|
||||
|
||||
// The `developer_instructions` block is a TOML multi-line literal string
|
||||
// (`developer_instructions = '''...'''`) that `generateCodexAgentToml` always
|
||||
// emits after the header fields. Prompt prose inside that block discusses models
|
||||
// constantly, so a `model = ...`-shaped line inside it must never be read as a
|
||||
// live pin — but the block can legally appear anywhere in the file (a
|
||||
// hand-reordered agent can move `model` after it), and another key's *value* can
|
||||
// legally contain the literal text `developer_instructions = '''` (e.g. a
|
||||
// `description` field quoting it) without that being the real block opener. So
|
||||
// instead of truncating the file at the first textual occurrence of the marker
|
||||
// anywhere in the content, this locates the block by its anchored line-start
|
||||
// opener (`^\s*developer_instructions\s*=\s*'''`, never a mid-line/mid-value
|
||||
// match) and its closing `'''` line, and excludes only the lines between them —
|
||||
// every other line in the file, before AND after the block, is scanned.
|
||||
//
|
||||
// If no opener is found, nothing is excluded (the whole file is scanned). If the
|
||||
// block is unterminated (no closing `'''` before EOF — a malformed file), the
|
||||
// rest of the file is treated as inside the block: that is the safe direction,
|
||||
// since misreading prompt prose as a pin past a malformed block is only a
|
||||
// false positive (wastes a user's time), while the alternative — scanning past
|
||||
// an unterminated block — risks hiding a real pin inside unclosed prose. The
|
||||
// emitter always uses `'''` (a TOML literal string), never a `"""` basic
|
||||
// multi-line string, so only `'''` is treated as the block delimiter here.
|
||||
function findDeveloperInstructionsBlockRange(lines: string[]): { start: number; end: number } {
|
||||
const openIndex = lines.findIndex((line) => /^\s*developer_instructions\s*=\s*'''/.test(line));
|
||||
if (openIndex === -1) {
|
||||
return { start: -1, end: -1 };
|
||||
}
|
||||
const afterOpenMarker = lines[openIndex].replace(/^\s*developer_instructions\s*=\s*'''/, '');
|
||||
if (afterOpenMarker.includes("'''")) {
|
||||
// Same-line block: developer_instructions = '''one line'''
|
||||
return { start: openIndex, end: openIndex };
|
||||
}
|
||||
for (let i = openIndex + 1; i < lines.length; i++) {
|
||||
if (lines[i].includes("'''")) {
|
||||
return { start: openIndex, end: i };
|
||||
}
|
||||
}
|
||||
return { start: openIndex, end: lines.length - 1 };
|
||||
}
|
||||
|
||||
// Strips a leading UTF-8 BOM (U+FEFF), which fs.readFileSync(..., 'utf8') does not
|
||||
// strip on its own, and unwraps a TOML basic/literal string value's surrounding
|
||||
// quotes so `model = "sonnet"` yields `sonnet`, not `"sonnet"`.
|
||||
function stripBOM(content: string): string {
|
||||
return content.charCodeAt(0) === 0xfeff ? content.slice(1) : content;
|
||||
}
|
||||
|
||||
function unquoteTomlValue(rawValue: string): string {
|
||||
const trimmed = rawValue.trim();
|
||||
const quoted = trimmed.match(/^"([^"]*)"/) ?? trimmed.match(/^'([^']*)'/);
|
||||
return quoted ? quoted[1] : trimmed;
|
||||
}
|
||||
|
||||
interface HeaderScanResult {
|
||||
model: string | null;
|
||||
hasReasoningEffort: boolean;
|
||||
}
|
||||
|
||||
// Line-oriented scan of every line OUTSIDE the `developer_instructions` block
|
||||
// (see findDeveloperInstructionsBlockRange). Full-key-name anchoring —
|
||||
// `^([A-Za-z_][\w]*)\s*=` for a bare key, or `^"([^"]*)"\s*=` / `^'([^']*)'\s*=`
|
||||
// for TOML's legal quoted-key forms, normalized to the same key name — means
|
||||
// `model_verbosity` / `model_reasoning_effort` never satisfy a `model` probe,
|
||||
// and vice versa; `#`-prefixed lines (after trimming leading whitespace) are
|
||||
// treated as comments, never live pins.
|
||||
function scanTomlLines(content: string): HeaderScanResult {
|
||||
const lines = content.split(/\r?\n/);
|
||||
const { start, end } = findDeveloperInstructionsBlockRange(lines);
|
||||
let model: string | null = null;
|
||||
let hasReasoningEffort = false;
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
if (start !== -1 && i >= start && i <= end) continue;
|
||||
const trimmed = lines[i].trim();
|
||||
if (trimmed === '' || trimmed.startsWith('#')) continue;
|
||||
const match = trimmed.match(/^(?:"([^"]*)"|'([^']*)'|([A-Za-z_][\w]*))\s*=\s*(.*)$/);
|
||||
if (!match) continue;
|
||||
const key = match[1] ?? match[2] ?? match[3];
|
||||
const rawValue = match[4];
|
||||
if (key === 'model') {
|
||||
model = unquoteTomlValue(rawValue);
|
||||
} else if (key === 'model_reasoning_effort') {
|
||||
hasReasoningEffort = true;
|
||||
}
|
||||
}
|
||||
return { model, hasReasoningEffort };
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the agents directory for the given runtime.
|
||||
*
|
||||
|
||||
391
src/codex-agent-toml.cts
Normal file
391
src/codex-agent-toml.cts
Normal file
@@ -0,0 +1,391 @@
|
||||
/**
|
||||
* Codex Agent TOML — typed IR for `~/.codex/agents/<agent>.toml` (#3243, ADR-2313).
|
||||
*
|
||||
* A genuine leaf: node builtins only. This is a **document model**, not a policy —
|
||||
* it knows how to parse/render/strip two known keys (`model`,
|
||||
* `model_reasoning_effort`) from a Codex agent `.toml`. It does NOT know which
|
||||
* `model` values are illegal for Codex (that predicate — Anthropic-flavored
|
||||
* detection — stays in `model-catalog.cts`; callers decide what to strip).
|
||||
*
|
||||
* Moved here (not copied) from `agent-install-check.cts` (#3242, Phase 2), which
|
||||
* wrote the hard half: block-range detection, BOM stripping, TOML value
|
||||
* unquoting, and the lenient header scan. That module's behavior is UNCHANGED —
|
||||
* it imports `stripBOM`/`scanTomlLines` from here and its regression suite
|
||||
* (`tests/agent-install-check.test.cjs`) is the proof.
|
||||
*
|
||||
* ── The reconciliation (40-design.md) ──────────────────────────────────────
|
||||
*
|
||||
* Phase 2's reader and this phase's writer disagree on how to handle an
|
||||
* unterminated `developer_instructions` block, deliberately:
|
||||
*
|
||||
* - The READER (`scanTomlLines`, used directly by `checkCodexModelPosture`)
|
||||
* stays LENIENT: an unterminated block still excludes "the rest of the
|
||||
* file" from the header scan (findDeveloperInstructionsBlockRange's
|
||||
* existing fallback), because misreading prompt prose as a pin is only a
|
||||
* false positive — it wastes a user's time, nothing more.
|
||||
* - The WRITER (`parseCodexAgentToml`, used by the Codex sync) is STRICT: an
|
||||
* unterminated block makes the whole document `{ok:false}`, because a
|
||||
* writer that proceeds on a malformed document risks rewriting it.
|
||||
*
|
||||
* One block-range detector, two call sites, two policies — never two detectors
|
||||
* that could silently drift from each other.
|
||||
*/
|
||||
|
||||
/** Frozen reason enum for a failed {@link parseCodexAgentToml}. */
|
||||
export const PARSE_REASON = Object.freeze({
|
||||
UNTERMINATED_BLOCK: 'unterminated_block',
|
||||
});
|
||||
|
||||
export type ParseReason = (typeof PARSE_REASON)[keyof typeof PARSE_REASON];
|
||||
|
||||
/**
|
||||
* The typed IR. Carries the original `lines` (never re-tokenized once parsed)
|
||||
* plus the detected BOM/trailing-newline metadata so {@link renderCodexAgentToml}
|
||||
* can reproduce the source **byte-identically** when nothing was stripped (the
|
||||
* load-bearing round-trip property — see 50-test-matrix.md row A14), even when
|
||||
* the source mixes line-ending styles (`\r\n`, `\n`, a lone `\r`) within one
|
||||
* file. `stripModel`/`stripReasoningEffort` remove a targeted line AND its own
|
||||
* terminator from `lines`/`terminators`; every other line's content is
|
||||
* untouched, and so is its own terminator — EXCEPT when the removed line was
|
||||
* the file's last line AND the removed line had no terminator (the source had
|
||||
* no trailing newline), in which case the new last line's terminator is
|
||||
* cleared to `''` too (see {@link removeLine}) so the file's trailing-newline
|
||||
* status is preserved rather than silently changed. When the source DID end
|
||||
* with a newline, the new last line keeps its OWN terminator unchanged —
|
||||
* copying the removed line's terminator there would corrupt a mixed-EOL
|
||||
* source by silently changing the new last line's own ending style.
|
||||
*/
|
||||
export interface CodexAgentDoc {
|
||||
/** Content lines (BOM-stripped, terminator-free). Paired 1:1 by index with `terminators`. */
|
||||
lines: string[];
|
||||
/**
|
||||
* Each line's OWN terminator (`'\r\n'`, `'\n'`, `'\r'`, or `''` for a line
|
||||
* with none — the last line of a source with no trailing newline). Never
|
||||
* collapsed to one whole-file style: {@link renderCodexAgentToml} rejoins
|
||||
* `lines[i] + terminators[i]` so a mixed-EOL source round-trips exactly.
|
||||
*/
|
||||
terminators: string[];
|
||||
/**
|
||||
* The line-ending style that appears in the source, informational only —
|
||||
* `renderCodexAgentToml` does NOT use this field (it rejoins each line with
|
||||
* its own `terminators[i]` instead). Recorded as `'\r\n'` if any `\r\n`
|
||||
* appears anywhere in the source, else `'\n'`, purely for callers that want
|
||||
* a human-readable summary (e.g. logging); never a rendering input.
|
||||
*/
|
||||
eol: '\n' | '\r\n';
|
||||
/** Whether the source began with a UTF-8 BOM (U+FEFF). */
|
||||
hadBOM: boolean;
|
||||
/** Whether the source ended with a line terminator (any of `\r\n`/`\n`/`\r`). */
|
||||
trailingNewline: boolean;
|
||||
/** The `developer_instructions = '''...'''` block's line range, or `{start:-1,end:-1}` if absent. */
|
||||
blockRange: { start: number; end: number };
|
||||
/** The resolved `model` value (last occurrence outside the block), or null. */
|
||||
model: string | null;
|
||||
/** Line index of the `model` key, or null if absent. */
|
||||
modelLineIndex: number | null;
|
||||
/** The resolved `model_reasoning_effort` value (last occurrence outside the block), or null. */
|
||||
reasoningEffort: string | null;
|
||||
/** Line index of the `model_reasoning_effort` key, or null if absent. */
|
||||
reasoningEffortLineIndex: number | null;
|
||||
}
|
||||
|
||||
export type ParseCodexAgentTomlResult =
|
||||
| { ok: true; doc: CodexAgentDoc }
|
||||
| { ok: false; reason: ParseReason };
|
||||
|
||||
// The UTF-8 BOM codepoint, spelled as an escape rather than the literal
|
||||
// character so the source file never carries an invisible codepoint.
|
||||
const BOM_CHAR = String.fromCharCode(0xfeff);
|
||||
|
||||
// Strips a leading UTF-8 BOM (U+FEFF), which fs.readFileSync(..., 'utf8') does not
|
||||
// strip on its own, and unwraps a TOML basic/literal string value's surrounding
|
||||
// quotes so `model = "sonnet"` yields `sonnet`, not `"sonnet"`.
|
||||
export function stripBOM(content: string): string {
|
||||
return content.charCodeAt(0) === 0xfeff ? content.slice(1) : content;
|
||||
}
|
||||
|
||||
export function unquoteTomlValue(rawValue: string): string {
|
||||
const trimmed = rawValue.trim();
|
||||
const quoted = trimmed.match(/^"([^"]*)"/) ?? trimmed.match(/^'([^']*)'/);
|
||||
return quoted ? quoted[1] : trimmed;
|
||||
}
|
||||
|
||||
// The `developer_instructions` block is a TOML multi-line literal string
|
||||
// (`developer_instructions = '''...'''`) that `generateCodexAgentToml` always
|
||||
// emits after the header fields. Prompt prose inside that block discusses models
|
||||
// constantly, so a `model = ...`-shaped line inside it must never be read as a
|
||||
// live pin — but the block can legally appear anywhere in the file (a
|
||||
// hand-reordered agent can move `model` after it), and another key's *value* can
|
||||
// legally contain the literal text `developer_instructions = '''` (e.g. a
|
||||
// `description` field quoting it) without that being the real block opener. So
|
||||
// instead of truncating the file at the first textual occurrence of the marker
|
||||
// anywhere in the content, this locates the block by its anchored line-start
|
||||
// opener (`^\s*developer_instructions\s*=\s*'''`, never a mid-line/mid-value
|
||||
// match) and its closing `'''` line, and excludes only the lines between them —
|
||||
// every other line in the file, before AND after the block, is scanned.
|
||||
//
|
||||
// If no opener is found, nothing is excluded (the whole file is scanned) and
|
||||
// `terminated` is trivially true. If the block IS opened but never closed before
|
||||
// EOF (malformed file), `terminated` is false: the lenient reader (scanTomlLines)
|
||||
// still treats the rest of the file as inside the block (the safe direction for
|
||||
// a reader — see module header comment); the strict writer (parseCodexAgentToml)
|
||||
// reads `terminated` and refuses instead. The emitter always uses `'''` (a TOML
|
||||
// literal string), never a `"""` basic multi-line string, so only `'''` is
|
||||
// treated as the block delimiter here.
|
||||
export function findDeveloperInstructionsBlockRange(
|
||||
lines: string[],
|
||||
): { start: number; end: number; terminated: boolean } {
|
||||
const openIndex = lines.findIndex((line) => /^\s*developer_instructions\s*=\s*'''/.test(line));
|
||||
if (openIndex === -1) {
|
||||
return { start: -1, end: -1, terminated: true };
|
||||
}
|
||||
const afterOpenMarker = lines[openIndex].replace(/^\s*developer_instructions\s*=\s*'''/, '');
|
||||
if (afterOpenMarker.includes("'''")) {
|
||||
// Same-line block: developer_instructions = '''one line'''
|
||||
return { start: openIndex, end: openIndex, terminated: true };
|
||||
}
|
||||
for (let i = openIndex + 1; i < lines.length; i++) {
|
||||
if (lines[i].includes("'''")) {
|
||||
return { start: openIndex, end: i, terminated: true };
|
||||
}
|
||||
}
|
||||
return { start: openIndex, end: lines.length - 1, terminated: false };
|
||||
}
|
||||
|
||||
/** Header-scan result shared by the lenient reader ({@link scanTomlLines}). */
|
||||
export interface HeaderScanResult {
|
||||
model: string | null;
|
||||
hasReasoningEffort: boolean;
|
||||
}
|
||||
|
||||
interface HeaderLineInfo {
|
||||
model: string | null;
|
||||
modelLineIndex: number | null;
|
||||
reasoningEffort: string | null;
|
||||
reasoningEffortLineIndex: number | null;
|
||||
}
|
||||
|
||||
// Line-oriented scan of every line OUTSIDE the `developer_instructions` block
|
||||
// (see findDeveloperInstructionsBlockRange). Full-key-name anchoring —
|
||||
// `^([A-Za-z_][\w]*)\s*=` for a bare key, or `^"([^"]*)"\s*=` / `^'([^']*)'\s*=`
|
||||
// for TOML's legal quoted-key forms, normalized to the same key name — means
|
||||
// `model_verbosity` / `model_reasoning_effort` never satisfy a `model` probe,
|
||||
// and vice versa; `#`-prefixed lines (after trimming leading whitespace) are
|
||||
// treated as comments, never live pins. Shared by both scanTomlLines (the
|
||||
// lenient reader, boolean-only for reasoning effort) and parseCodexAgentToml
|
||||
// (the strict writer, which also needs the effort's value and both keys' line
|
||||
// indices so stripModel/stripReasoningEffort can remove exactly one line).
|
||||
function scanHeaderLines(lines: string[], blockStart: number, blockEnd: number): HeaderLineInfo {
|
||||
let model: string | null = null;
|
||||
let modelLineIndex: number | null = null;
|
||||
let reasoningEffort: string | null = null;
|
||||
let reasoningEffortLineIndex: number | null = null;
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
if (blockStart !== -1 && i >= blockStart && i <= blockEnd) continue;
|
||||
const trimmed = lines[i].trim();
|
||||
if (trimmed === '' || trimmed.startsWith('#')) continue;
|
||||
const match = trimmed.match(/^(?:"([^"]*)"|'([^']*)'|([A-Za-z_][\w]*))\s*=\s*(.*)$/);
|
||||
if (!match) continue;
|
||||
const key = match[1] ?? match[2] ?? match[3];
|
||||
const rawValue = match[4];
|
||||
if (key === 'model') {
|
||||
model = unquoteTomlValue(rawValue);
|
||||
modelLineIndex = i;
|
||||
} else if (key === 'model_reasoning_effort') {
|
||||
reasoningEffort = unquoteTomlValue(rawValue);
|
||||
reasoningEffortLineIndex = i;
|
||||
}
|
||||
}
|
||||
return { model, modelLineIndex, reasoningEffort, reasoningEffortLineIndex };
|
||||
}
|
||||
|
||||
/**
|
||||
* The LENIENT reader entry point (Phase 2, moved verbatim in behavior). Never
|
||||
* fails: an unterminated block falls back to "rest of file is inside the
|
||||
* block" via {@link findDeveloperInstructionsBlockRange}'s own fallback.
|
||||
* `content` is expected already BOM-stripped (callers pass `stripBOM(raw)`).
|
||||
*/
|
||||
export function scanTomlLines(content: string): HeaderScanResult {
|
||||
const lines = content.split(/\r?\n/);
|
||||
const { start, end } = findDeveloperInstructionsBlockRange(lines);
|
||||
const { model, reasoningEffort } = scanHeaderLines(lines, start, end);
|
||||
return { model, hasReasoningEffort: reasoningEffort !== null };
|
||||
}
|
||||
|
||||
// Splits `content` into `{lines, terminators}` where `terminators[i]` is the
|
||||
// terminator that FOLLOWS `lines[i]` (`'\r\n'`, `'\r'`, `'\n'`, or `''` for a
|
||||
// line with none — only possible as the file's last line). The two arrays are
|
||||
// always the same length and there is NEVER a phantom trailing entry: a
|
||||
// source ending in a terminator (the common case) yields exactly as many
|
||||
// lines as it has content lines, not one more. `render` is then a plain
|
||||
// `lines[i] + terminators[i]` concatenation with no special-casing of "the
|
||||
// last line" — see `renderCodexAgentToml`.
|
||||
//
|
||||
// `String#split` with a capturing group interleaves the delimiters into the
|
||||
// result array — `"a\r\nb\nc".split(/(\r\n|\r|\n)/)` yields
|
||||
// `["a","\r\n","b","\n","c"]` — so even indices are line content and odd
|
||||
// indices are that line's terminator. When `content` ends WITH a terminator,
|
||||
// `split` appends one extra empty-string element after the last real
|
||||
// terminator (e.g. `"a\n".split(...)` → `["a","\n",""]`); that trailing `""`
|
||||
// is not a real line, it is `split`'s "nothing after the last delimiter"
|
||||
// marker, so the loop below stops before consuming it instead of recording it
|
||||
// as a phantom empty final line (the defect this replaced — see A29: a doc
|
||||
// with a phantom last element made every removal rule reason about the wrong
|
||||
// element for any trailing-newline-terminated file, the common case). `\r\n`
|
||||
// is tried before the bare `\r` alternative so a CRLF is never misread as a
|
||||
// lone-CR line followed by an empty LF-terminated line.
|
||||
function splitPreservingTerminators(content: string): { lines: string[]; terminators: string[] } {
|
||||
if (content === '') return { lines: [], terminators: [] };
|
||||
const parts = content.split(/(\r\n|\r|\n)/);
|
||||
const lastIndex = parts.length - 1;
|
||||
const lines: string[] = [];
|
||||
const terminators: string[] = [];
|
||||
for (let i = 0; i < parts.length; i += 2) {
|
||||
if (i === lastIndex && parts[i] === '') break; // split's post-terminator marker, not a real line
|
||||
lines.push(parts[i]);
|
||||
terminators.push(parts[i + 1] ?? '');
|
||||
}
|
||||
return { lines, terminators };
|
||||
}
|
||||
|
||||
/**
|
||||
* The STRICT parse entry point (Phase 3, the writer's half of the
|
||||
* reconciliation). Returns `{ok:false, reason:UNTERMINATED_BLOCK}` rather than
|
||||
* guessing when the `developer_instructions` block is opened but never closed.
|
||||
* On success, `doc` carries enough (the original `lines`/`terminators`, BOM/
|
||||
* trailing-newline flags, and the two resolved values with their line indices)
|
||||
* for {@link renderCodexAgentToml} to reproduce the source byte-identically —
|
||||
* including a source with mixed line-ending styles — and for
|
||||
* {@link stripModel}/{@link stripReasoningEffort} to remove exactly one line
|
||||
* and its own terminator.
|
||||
*/
|
||||
export function parseCodexAgentToml(content: string): ParseCodexAgentTomlResult {
|
||||
const hadBOM = content.charCodeAt(0) === 0xfeff;
|
||||
const stripped = stripBOM(content);
|
||||
// Informational only — see CodexAgentDoc.eol's docstring. Never used by
|
||||
// renderCodexAgentToml.
|
||||
const eol: '\n' | '\r\n' = stripped.includes('\r\n') ? '\r\n' : '\n';
|
||||
const trailingNewline = /(\r\n|\r|\n)$/.test(stripped);
|
||||
const { lines, terminators } = splitPreservingTerminators(stripped);
|
||||
const { start, end, terminated } = findDeveloperInstructionsBlockRange(lines);
|
||||
if (start !== -1 && !terminated) {
|
||||
return { ok: false, reason: PARSE_REASON.UNTERMINATED_BLOCK };
|
||||
}
|
||||
const { model, modelLineIndex, reasoningEffort, reasoningEffortLineIndex } = scanHeaderLines(lines, start, end);
|
||||
const doc: CodexAgentDoc = {
|
||||
lines,
|
||||
terminators,
|
||||
eol,
|
||||
hadBOM,
|
||||
trailingNewline,
|
||||
blockRange: { start, end },
|
||||
model,
|
||||
modelLineIndex,
|
||||
reasoningEffort,
|
||||
reasoningEffortLineIndex,
|
||||
};
|
||||
return { ok: true, doc };
|
||||
}
|
||||
|
||||
/**
|
||||
* Renders `doc` back to a string. For an unmodified doc this is
|
||||
* byte-identical to the original `parseCodexAgentToml` input (matrix row
|
||||
* A14) — it never re-derives line content, only rejoins each line with its
|
||||
* OWN recorded terminator (`terminators[i]`, never the whole-file `eol`) and
|
||||
* re-prepends a BOM if one was present. This is a plain concatenation of the
|
||||
* surviving `[line, terminator]` pieces, so a source with mixed `\r\n`/`\n`/
|
||||
* lone-`\r` line endings round-trips exactly, and a strip
|
||||
* ({@link stripModel}/{@link stripReasoningEffort}) removes only the target
|
||||
* line and its own terminator — every other line's ending is untouched.
|
||||
*/
|
||||
export function renderCodexAgentToml(doc: CodexAgentDoc): string {
|
||||
let body = '';
|
||||
for (let i = 0; i < doc.lines.length; i++) {
|
||||
body += doc.lines[i] + (doc.terminators[i] ?? '');
|
||||
}
|
||||
return doc.hadBOM ? BOM_CHAR + body : body;
|
||||
}
|
||||
|
||||
// Removes exactly one line (by index) — AND its own terminator — from
|
||||
// `doc.lines`/`doc.terminators`, re-indexing the block range and the OTHER
|
||||
// key's line index so a subsequent strip/render still sees a consistent doc.
|
||||
// Never touches any other line's content or terminator.
|
||||
//
|
||||
// The one exception is when `index` names the file's LAST line: a plain
|
||||
// slice-out would drop the removed line's terminator but leave the
|
||||
// *previous* line's terminator standing in its place, which silently
|
||||
// invents (or drops) a trailing newline the source never had — a middle-line
|
||||
// removal never has this problem because the terminator that survives (the
|
||||
// one that WAS between the previous line and the removed one) is exactly the
|
||||
// terminator the new neighbors should have between them. For a last-line
|
||||
// removal, the file's trailing-newline-or-not status lives in whether the
|
||||
// REMOVED line's own terminator was empty (that is what `trailingNewline`
|
||||
// was computed from) — so the new last line inherits the removed line's
|
||||
// EMPTINESS only: if the removed terminator was `''`, the new last line's
|
||||
// terminator is cleared to `''` too. If the removed terminator was
|
||||
// non-empty, the source already ended with a newline and the new last line
|
||||
// already has the right one (its OWN, unchanged) — overwriting it with the
|
||||
// removed line's terminator would silently change the new last line's own
|
||||
// ending style on a mixed-EOL source (see A26). Removing the only remaining
|
||||
// line is the degenerate case: there is no new last line, so the result is
|
||||
// the empty document.
|
||||
function removeLine(doc: CodexAgentDoc, index: number, which: 'model' | 'reasoningEffort'): CodexAgentDoc {
|
||||
const isLastLine = index === doc.lines.length - 1;
|
||||
let lines: string[];
|
||||
let terminators: string[];
|
||||
if (doc.lines.length === 1) {
|
||||
lines = [];
|
||||
terminators = [];
|
||||
} else if (isLastLine) {
|
||||
lines = doc.lines.slice(0, index);
|
||||
terminators = doc.terminators.slice(0, index);
|
||||
// Inherit the removed line's EMPTINESS, never its STYLE: if the removed
|
||||
// line had no terminator (the source had no trailing newline), the new
|
||||
// last line's terminator becomes '' too. Otherwise the source DID end
|
||||
// with a newline, and the new last line already has the right one — its
|
||||
// OWN terminator (already carried over by the slice above), which may
|
||||
// differ in style from the removed line's (a mixed-EOL source) — so it is
|
||||
// left unchanged rather than overwritten.
|
||||
if (doc.terminators[index] === '') {
|
||||
terminators[terminators.length - 1] = '';
|
||||
}
|
||||
} else {
|
||||
lines = doc.lines.slice(0, index).concat(doc.lines.slice(index + 1));
|
||||
terminators = doc.terminators.slice(0, index).concat(doc.terminators.slice(index + 1));
|
||||
}
|
||||
const reindex = (i: number | null): number | null => (i === null ? null : i > index ? i - 1 : i);
|
||||
const blockRange = { ...doc.blockRange };
|
||||
if (blockRange.start !== -1) {
|
||||
if (blockRange.start > index) blockRange.start -= 1;
|
||||
if (blockRange.end > index) blockRange.end -= 1;
|
||||
}
|
||||
return {
|
||||
...doc,
|
||||
lines,
|
||||
terminators,
|
||||
blockRange,
|
||||
model: which === 'model' ? null : doc.model,
|
||||
modelLineIndex: which === 'model' ? null : reindex(doc.modelLineIndex),
|
||||
reasoningEffort: which === 'reasoningEffort' ? null : doc.reasoningEffort,
|
||||
reasoningEffortLineIndex: which === 'reasoningEffort' ? null : reindex(doc.reasoningEffortLineIndex),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a new doc with the `model` line removed (a no-op copy if there was
|
||||
* no `model` line). Every other byte — comments, other keys, the
|
||||
* `developer_instructions` block, line endings, BOM — is untouched.
|
||||
*/
|
||||
export function stripModel(doc: CodexAgentDoc): CodexAgentDoc {
|
||||
if (doc.modelLineIndex === null) return doc;
|
||||
return removeLine(doc, doc.modelLineIndex, 'model');
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a new doc with the `model_reasoning_effort` line removed (a no-op
|
||||
* copy if there was none). Every other byte is untouched.
|
||||
*/
|
||||
export function stripReasoningEffort(doc: CodexAgentDoc): CodexAgentDoc {
|
||||
if (doc.reasoningEffortLineIndex === null) return doc;
|
||||
return removeLine(doc, doc.reasoningEffortLineIndex, 'reasoningEffort');
|
||||
}
|
||||
152
src/commands.cts
152
src/commands.cts
@@ -8,7 +8,7 @@
|
||||
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { execGit, platformWriteSync, platformReadSync, platformEnsureDir, isSpawnTimeout } from './shell-command-projection.cjs';
|
||||
import { execGit, platformWriteSync, platformReadSync, platformEnsureDir, isSpawnTimeout, retryRenameSync } from './shell-command-projection.cjs';
|
||||
import { requireSafePath, sanitizeForDisplay } from './security.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import ioMod = require('./io.cjs');
|
||||
@@ -37,7 +37,12 @@ const { resolveModelInternal, resolveTierInternal, resolveModelForTier, resolveP
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import agentCommandRouterMod = require('./agent-command-router.cjs');
|
||||
const { AGENT_FAILURE_CLASSES } = agentCommandRouterMod;
|
||||
import { renderEffortForRuntime, renderEffortArgv, RUNTIMES_WITH_FAST_MODE } from './model-catalog.cjs';
|
||||
import { renderEffortForRuntime, renderEffortArgv, RUNTIMES_WITH_FAST_MODE, isAnthropicFlavoredModel } from './model-catalog.cjs';
|
||||
// #3243 (ADR-2313 D7) — the Codex `.toml` sync's typed IR: parse/render/strip
|
||||
// primitives moved from agent-install-check.cts's Phase-2 parsing into this
|
||||
// leaf so both consumers share one block-range detector. See
|
||||
// codex-agent-toml.cts's module header for the reader/writer reconciliation.
|
||||
import { parseCodexAgentToml, renderCodexAgentToml, stripModel, stripReasoningEffort } from './codex-agent-toml.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import hostIntegrationMod = require('./host-integration.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
@@ -704,6 +709,15 @@ function cmdEffortSync(cwd: string, raw: boolean, opts?: { dryRun?: boolean; con
|
||||
const config = loadConfig(cwd);
|
||||
const runtime = opts.runtime || (config['runtime'] as string) || 'claude';
|
||||
|
||||
// ADR-2313 D7 (#3243) — Codex gets its own `.toml` sync path (strip a stale
|
||||
// Anthropic/tier `model` and an orphaned `model_reasoning_effort`, leaving a
|
||||
// legal pin untouched). Every other non-claude runtime keeps the prior
|
||||
// early-return; the claude branch below is untouched byte-for-byte.
|
||||
if (runtime === 'codex') {
|
||||
cmdEffortSyncCodex(raw, dryRun, opts.configDir);
|
||||
return;
|
||||
}
|
||||
|
||||
if (runtime !== 'claude') {
|
||||
output({ synced: 0, skipped: 0, changes: [], dry_run: dryRun, reason: `runtime '${runtime}' does not use effort: frontmatter` }, raw, '');
|
||||
return;
|
||||
@@ -767,6 +781,140 @@ function cmdEffortSync(cwd: string, raw: boolean, opts?: { dryRun?: boolean; con
|
||||
output({ synced, skipped, changes, dry_run: dryRun, agents_dir: agentsDir }, raw, synced > 0 ? 'changed' : 'ok');
|
||||
}
|
||||
|
||||
/** One `{agent, field, from}` strip reported by {@link cmdEffortSyncCodex} — `to` is always omission (`null`). */
|
||||
interface CodexEffortSyncChange {
|
||||
agent: string;
|
||||
field: 'model' | 'model_reasoning_effort';
|
||||
from: string;
|
||||
to: null;
|
||||
}
|
||||
|
||||
/** A file `parseCodexAgentToml` refused, reported rather than partially rewritten (ADR-2313 D7 row 11). */
|
||||
interface CodexEffortSyncRefusal {
|
||||
agent: string;
|
||||
file: string;
|
||||
reason: string;
|
||||
}
|
||||
|
||||
/** A write that failed mid-sync (fs fault), reported so the remaining agents still get processed. */
|
||||
interface CodexEffortSyncWriteFailure {
|
||||
agent: string;
|
||||
file: string;
|
||||
error: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* ADR-2313 D7 (#3243) — the Codex branch of `cmdEffortSync`. Strips a stale
|
||||
* Anthropic-flavored/tier `model` pin and an orphaned `model_reasoning_effort`
|
||||
* from every installed `~/.codex/agents/<agent>.toml`, leaving a legal
|
||||
* real-Codex pin (and its coupled effort) untouched. Dry-run by default; every
|
||||
* strip reported as a structured `{agent, field, from}` change; an unparseable
|
||||
* document is refused and reported, never partially rewritten (40-design.md
|
||||
* "Reconciliation" — parseCodexAgentToml is the STRICT half of the reader/
|
||||
* writer split). Result shape is additive over the claude branch's
|
||||
* `{synced, skipped, changes, dry_run, agents_dir}` — `refused` and
|
||||
* `write_failures` are new fields, never a reshape of the existing ones.
|
||||
*/
|
||||
function cmdEffortSyncCodex(raw: boolean, dryRun: boolean, configDir?: string): void {
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/unbound-method
|
||||
const { getGlobalConfigDir } = require('./runtime-homes.cjs') as { getGlobalConfigDir(runtime: string, explicitDir?: string | null): string };
|
||||
const agentsDir = path.join(configDir || getGlobalConfigDir('codex'), 'agents');
|
||||
|
||||
if (!fs.existsSync(agentsDir)) {
|
||||
output({ synced: 0, skipped: 0, changes: [], dry_run: dryRun, agents_dir: agentsDir, reason: 'agents directory not found' }, raw, '');
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip symlinks — matches the claude branch's existing guard above (only
|
||||
// write regular files, never follow a symlink into clobbering its target).
|
||||
const files = fs
|
||||
.readdirSync(agentsDir)
|
||||
.filter(f => {
|
||||
if (!f.endsWith('.toml')) return false;
|
||||
try { return fs.lstatSync(path.join(agentsDir, f)).isFile(); } catch { return false; }
|
||||
})
|
||||
.sort();
|
||||
|
||||
const changes: CodexEffortSyncChange[] = [];
|
||||
const refused: CodexEffortSyncRefusal[] = [];
|
||||
const writeFailures: CodexEffortSyncWriteFailure[] = [];
|
||||
let synced = 0;
|
||||
let skipped = 0;
|
||||
|
||||
for (const file of files) {
|
||||
const agentName = file.replace(/\.toml$/, '');
|
||||
const filePath = path.join(agentsDir, file);
|
||||
const content = fs.readFileSync(filePath, 'utf8');
|
||||
|
||||
const parsed = parseCodexAgentToml(content);
|
||||
if (!parsed.ok) {
|
||||
// Never partially rewritten (40-design.md, ADR-2313 reader/writer
|
||||
// boundary): an unparseable document is skipped and reported, not
|
||||
// guessed at.
|
||||
skipped++;
|
||||
refused.push({ agent: agentName, file: filePath, reason: parsed.reason });
|
||||
continue;
|
||||
}
|
||||
|
||||
let doc = parsed.doc;
|
||||
const stripModelNeeded = doc.model !== null && isAnthropicFlavoredModel(doc.model);
|
||||
// #838 coupling: an orphaned effort (no model) is always stale; a stale
|
||||
// model's effort is coupled to it and strips with it. A legal pin's effort
|
||||
// (model present, not Anthropic-flavored) is left untouched (rows 4-5).
|
||||
const stripEffortNeeded = doc.reasoningEffort !== null && (stripModelNeeded || doc.model === null);
|
||||
|
||||
if (!stripModelNeeded && !stripEffortNeeded) {
|
||||
// Posture-clean, OR a legal pin (and its coupled effort) — reported
|
||||
// skipped, never synced (ADR-2313 reader/writer boundary).
|
||||
skipped++;
|
||||
continue;
|
||||
}
|
||||
|
||||
const pendingChanges: CodexEffortSyncChange[] = [];
|
||||
if (stripModelNeeded) {
|
||||
pendingChanges.push({ agent: agentName, field: 'model', from: doc.model as string, to: null });
|
||||
doc = stripModel(doc);
|
||||
}
|
||||
if (stripEffortNeeded) {
|
||||
pendingChanges.push({ agent: agentName, field: 'model_reasoning_effort', from: doc.reasoningEffort as string, to: null });
|
||||
doc = stripReasoningEffort(doc);
|
||||
}
|
||||
|
||||
if (!dryRun) {
|
||||
// Atomic publish (ADR-2313 "never partially rewritten"): write the
|
||||
// rendered TOML to a sibling tmp file, then rename it over the target.
|
||||
// Same-filesystem rename is atomic, so filePath is either the old bytes
|
||||
// or the new ones, never truncated/half-written mid-crash. Deliberately
|
||||
// NOT platformWriteSync — its normalizeContent step rewrites CRLF/
|
||||
// trailing-newline bytes, which would break the byte-identical
|
||||
// round-trip (A14) this writer must preserve. retryRenameSync (not a
|
||||
// bare fs.renameSync) carries the transient-Windows-lock retry per
|
||||
// DEFECT.WINDOWS-FS-OPS.
|
||||
const tmpPath = `${filePath}.tmp.${process.pid}`;
|
||||
try {
|
||||
fs.writeFileSync(tmpPath, renderCodexAgentToml(doc));
|
||||
retryRenameSync(tmpPath, filePath);
|
||||
} catch (err) {
|
||||
// Reported, not thrown — the remaining agents still get processed.
|
||||
// Clean up the orphaned tmp file; filePath itself was never touched.
|
||||
try { fs.unlinkSync(tmpPath); } catch { /* already gone or never created */ }
|
||||
skipped++;
|
||||
writeFailures.push({ agent: agentName, file: filePath, error: err instanceof Error ? err.message : String(err) });
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
changes.push(...pendingChanges);
|
||||
synced++;
|
||||
}
|
||||
|
||||
output(
|
||||
{ synced, skipped, changes, dry_run: dryRun, agents_dir: agentsDir, refused, write_failures: writeFailures },
|
||||
raw,
|
||||
synced > 0 ? 'changed' : 'ok',
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect the phase number for a commit from its `--files` path list.
|
||||
*
|
||||
|
||||
529
tests/codex-agent-toml.test.cjs
Normal file
529
tests/codex-agent-toml.test.cjs
Normal file
@@ -0,0 +1,529 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Behavioral tests for codex-agent-toml.cjs (#3243, ADR-2313 Phase 3).
|
||||
*
|
||||
* Module: gsd-core/bin/lib/codex-agent-toml.cjs
|
||||
* Exports: PARSE_REASON, parseCodexAgentToml, renderCodexAgentToml,
|
||||
* stripModel, stripReasoningEffort
|
||||
*
|
||||
* Spec: .gsd/phase/feat-3243-codex-toml-sync/{40-design,50-test-matrix}.md
|
||||
* Row numbers below (# A<N>) map 1:1 to 50-test-matrix.md's "A — the IR" table.
|
||||
*
|
||||
* Every fixture is hand-authored against the real `generateCodexAgentToml`
|
||||
* shape (header fields + `developer_instructions = '''...'''`), never produced
|
||||
* by calling that emitter (CONTRIBUTING's fixture-provenance rule, #2371) — a
|
||||
* fixture generated by the writer under test could only confirm what the
|
||||
* writer already believes about its own output.
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
|
||||
const {
|
||||
PARSE_REASON,
|
||||
parseCodexAgentToml,
|
||||
renderCodexAgentToml,
|
||||
stripModel,
|
||||
stripReasoningEffort,
|
||||
} = require('../gsd-core/bin/lib/codex-agent-toml.cjs');
|
||||
|
||||
// Row 18a's CRLF fixture (and A12 here) is derived at test runtime, never read
|
||||
// from a committed file: `.gitattributes` (`* text=auto eol=lf`, repo-wide) would
|
||||
// normalize any committed `\r\n` fixture back to LF on checkout, silently
|
||||
// defeating the CRLF assertion. Authoring LF content and converting it here
|
||||
// keeps the CRLF-ness under the test's control instead of git's.
|
||||
function toCrlf(lfContent) {
|
||||
return lfContent.replace(/\n/g, '\r\n');
|
||||
}
|
||||
|
||||
// ─── A1-A13 fixtures (hand-authored, #2371) ───────────────────────────────────
|
||||
|
||||
const A1_FULL = 'name = "gsd-planner"\n' +
|
||||
'description = "Plans phases for GSD milestones"\n' +
|
||||
'model = "gpt-5.6-sol"\n' +
|
||||
'model_reasoning_effort = "high"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Plan the next phase.\n' +
|
||||
"'''\n";
|
||||
|
||||
const A2_NO_HEADER_FIELDS = 'name = "gsd-plain"\n' +
|
||||
'description = "No model, no effort"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Just work.\n' +
|
||||
"'''\n";
|
||||
|
||||
const A3_DOUBLE_QUOTED_KEY = 'name = "gsd-quoted-double"\n' +
|
||||
'"model" = "sonnet"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work.\n' +
|
||||
"'''\n";
|
||||
|
||||
const A3_SINGLE_QUOTED_KEY = "name = \"gsd-quoted-single\"\n" +
|
||||
"'model' = \"sonnet\"\n" +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work.\n' +
|
||||
"'''\n";
|
||||
|
||||
const A4_VERBOSITY_ONLY = 'name = "gsd-light"\n' +
|
||||
'model_verbosity = "low"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work fast.\n' +
|
||||
"'''\n";
|
||||
|
||||
const A5_MODEL_INSIDE_BLOCK = 'name = "gsd-planner"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'When picking a plan, remember: model = "sonnet" is just prose here.\n' +
|
||||
"'''\n";
|
||||
|
||||
const A6_MODEL_AFTER_BLOCK = 'name = "gsd-reordered"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work.\n' +
|
||||
"'''\n" +
|
||||
'model = "sonnet"\n';
|
||||
|
||||
const A7_DECOY_MARKER_IN_DESCRIPTION = 'name = "gsd-decoy-marker"\n' +
|
||||
'description = "mentions developer_instructions = \'\'\' as an example string"\n' +
|
||||
'model = "sonnet"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work.\n' +
|
||||
"'''\n";
|
||||
|
||||
const A8_SAME_LINE_BLOCK = 'name = "gsd-terse"\n' +
|
||||
"developer_instructions = '''one line block'''\n";
|
||||
|
||||
const A9_UNTERMINATED = 'name = "gsd-broken"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'This block never closes.\n';
|
||||
|
||||
const A10_NO_BLOCK_AT_ALL = 'name = "gsd-noblock"\n' +
|
||||
'description = "no prompt block on this agent"\n' +
|
||||
'"model" = "sonnet"\n';
|
||||
|
||||
const A11_COMMENTED_PIN = 'name = "gsd-reviewer"\n' +
|
||||
'# model = "sonnet"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Review.\n' +
|
||||
"'''\n";
|
||||
|
||||
const A12_CRLF_SOURCE = toCrlf(
|
||||
'name = "gsd-scribe"\n' +
|
||||
'model = "sonnet"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Write a changelog entry.\n' +
|
||||
"'''\n",
|
||||
);
|
||||
|
||||
const A13_BOM_SOURCE = String.fromCharCode(0xfeff) +
|
||||
'name = "gsd-archivist"\n' +
|
||||
'model = "sonnet"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Archive.\n' +
|
||||
"'''\n";
|
||||
|
||||
// ─── A1-A13: parse behavior ────────────────────────────────────────────────
|
||||
|
||||
describe('parseCodexAgentToml: happy / boundary / hostile', () => {
|
||||
test('A1: real emitted .toml (all header fields + block) — every field recovered', () => {
|
||||
const result = parseCodexAgentToml(A1_FULL);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, 'gpt-5.6-sol');
|
||||
assert.equal(result.doc.reasoningEffort, 'high');
|
||||
assert.equal(result.doc.hadBOM, false);
|
||||
assert.equal(result.doc.eol, '\n');
|
||||
assert.equal(result.doc.trailingNewline, true);
|
||||
assert.notEqual(result.doc.blockRange.start, -1);
|
||||
});
|
||||
|
||||
test('A2: header with no model / no effort — both null', () => {
|
||||
const result = parseCodexAgentToml(A2_NO_HEADER_FIELDS);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, null);
|
||||
assert.equal(result.doc.reasoningEffort, null);
|
||||
});
|
||||
|
||||
test('A3: quoted keys ("model" = / \'model\' =) recovered as key model', () => {
|
||||
const doubleQuoted = parseCodexAgentToml(A3_DOUBLE_QUOTED_KEY);
|
||||
assert.equal(doubleQuoted.ok, true);
|
||||
assert.equal(doubleQuoted.doc.model, 'sonnet');
|
||||
|
||||
const singleQuoted = parseCodexAgentToml(A3_SINGLE_QUOTED_KEY);
|
||||
assert.equal(singleQuoted.ok, true);
|
||||
assert.equal(singleQuoted.doc.model, 'sonnet');
|
||||
});
|
||||
|
||||
test('A4: model_verbosity present, model absent — model: null (full-key anchoring)', () => {
|
||||
const result = parseCodexAgentToml(A4_VERBOSITY_ONLY);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, null);
|
||||
});
|
||||
|
||||
test('A5 (hostile, data-loss case): model = inside the block — model: null', () => {
|
||||
const result = parseCodexAgentToml(A5_MODEL_INSIDE_BLOCK);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, null);
|
||||
});
|
||||
|
||||
test('A6: model line after the block (hand-reordered) — recovered', () => {
|
||||
const result = parseCodexAgentToml(A6_MODEL_AFTER_BLOCK);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, 'sonnet');
|
||||
});
|
||||
|
||||
test('A7 (hostile): description value quoting the block marker does not hide the real model pin', () => {
|
||||
const result = parseCodexAgentToml(A7_DECOY_MARKER_IN_DESCRIPTION);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, 'sonnet');
|
||||
});
|
||||
|
||||
test('A8: same-line block — block range is one line', () => {
|
||||
const result = parseCodexAgentToml(A8_SAME_LINE_BLOCK);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.blockRange.start, result.doc.blockRange.end);
|
||||
assert.notEqual(result.doc.blockRange.start, -1);
|
||||
});
|
||||
|
||||
test('A9: unterminated block — {ok:false, reason:UNTERMINATED_BLOCK}', () => {
|
||||
const result = parseCodexAgentToml(A9_UNTERMINATED);
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.reason, PARSE_REASON.UNTERMINATED_BLOCK);
|
||||
});
|
||||
|
||||
test('A10: no developer_instructions at all — {ok:true}, whole file scanned', () => {
|
||||
const result = parseCodexAgentToml(A10_NO_BLOCK_AT_ALL);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.blockRange.start, -1);
|
||||
assert.equal(result.doc.model, 'sonnet');
|
||||
});
|
||||
|
||||
test('A11 (hostile): commented # model = "x" — model: null', () => {
|
||||
const result = parseCodexAgentToml(A11_COMMENTED_PIN);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, null);
|
||||
});
|
||||
|
||||
test('A12 (cross-platform): CRLF input parses identically; eol recorded as \\r\\n', () => {
|
||||
const result = parseCodexAgentToml(A12_CRLF_SOURCE);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, 'sonnet');
|
||||
assert.equal(result.doc.eol, '\r\n');
|
||||
});
|
||||
|
||||
test('A13 (cross-platform): BOM input parses; BOM recorded', () => {
|
||||
const result = parseCodexAgentToml(A13_BOM_SOURCE);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, 'sonnet');
|
||||
assert.equal(result.doc.hadBOM, true);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── A14 (LOAD-BEARING): round-trip byte-identity ─────────────────────────────
|
||||
//
|
||||
// Asserts on raw BYTES, never a re-parsed structure — a renderer that dropped
|
||||
// every comment or normalized whitespace would still pass a structural
|
||||
// comparison. A parse/render pair that fails this row silently reformats a
|
||||
// user's file on every sync.
|
||||
|
||||
describe('A14 (load-bearing): render(parse(x)) === x, byte-identical', () => {
|
||||
const roundTripFixtures = [
|
||||
['A1_FULL', A1_FULL],
|
||||
['A2_NO_HEADER_FIELDS', A2_NO_HEADER_FIELDS],
|
||||
['A3_DOUBLE_QUOTED_KEY', A3_DOUBLE_QUOTED_KEY],
|
||||
['A3_SINGLE_QUOTED_KEY', A3_SINGLE_QUOTED_KEY],
|
||||
['A4_VERBOSITY_ONLY', A4_VERBOSITY_ONLY],
|
||||
['A5_MODEL_INSIDE_BLOCK', A5_MODEL_INSIDE_BLOCK],
|
||||
['A6_MODEL_AFTER_BLOCK', A6_MODEL_AFTER_BLOCK],
|
||||
['A7_DECOY_MARKER_IN_DESCRIPTION', A7_DECOY_MARKER_IN_DESCRIPTION],
|
||||
['A8_SAME_LINE_BLOCK', A8_SAME_LINE_BLOCK],
|
||||
['A10_NO_BLOCK_AT_ALL', A10_NO_BLOCK_AT_ALL],
|
||||
['A11_COMMENTED_PIN', A11_COMMENTED_PIN],
|
||||
['A12_CRLF_SOURCE', A12_CRLF_SOURCE],
|
||||
['A13_BOM_SOURCE', A13_BOM_SOURCE],
|
||||
];
|
||||
|
||||
for (const [label, source] of roundTripFixtures) {
|
||||
test(`round-trips ${label} byte-identically, unmodified`, () => {
|
||||
const result = parseCodexAgentToml(source);
|
||||
assert.equal(result.ok, true, `${label} must parse ok`);
|
||||
const rendered = renderCodexAgentToml(result.doc);
|
||||
assert.equal(rendered, source, `${label} must round-trip byte-identically`);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// ─── A15/A16: round-trip after a targeted strip ───────────────────────────────
|
||||
|
||||
describe('A15/A16: round-trip after stripModel / stripReasoningEffort', () => {
|
||||
test('A15: after stripModel(), only the model line is gone — every other byte identical', () => {
|
||||
const result = parseCodexAgentToml(A1_FULL);
|
||||
assert.equal(result.ok, true);
|
||||
const stripped = stripModel(result.doc);
|
||||
const rendered = renderCodexAgentToml(stripped);
|
||||
const expected = 'name = "gsd-planner"\n' +
|
||||
'description = "Plans phases for GSD milestones"\n' +
|
||||
'model_reasoning_effort = "high"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Plan the next phase.\n' +
|
||||
"'''\n";
|
||||
assert.equal(rendered, expected);
|
||||
assert.equal(stripped.model, null);
|
||||
assert.equal(stripped.reasoningEffort, 'high', 'the effort line must survive untouched');
|
||||
});
|
||||
|
||||
test('A16: after stripReasoningEffort(), only that line is gone — every other byte identical', () => {
|
||||
const result = parseCodexAgentToml(A1_FULL);
|
||||
assert.equal(result.ok, true);
|
||||
const stripped = stripReasoningEffort(result.doc);
|
||||
const rendered = renderCodexAgentToml(stripped);
|
||||
const expected = 'name = "gsd-planner"\n' +
|
||||
'description = "Plans phases for GSD milestones"\n' +
|
||||
'model = "gpt-5.6-sol"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Plan the next phase.\n' +
|
||||
"'''\n";
|
||||
assert.equal(rendered, expected);
|
||||
assert.equal(stripped.reasoningEffort, null);
|
||||
assert.equal(stripped.model, 'gpt-5.6-sol', 'the model line must survive untouched');
|
||||
});
|
||||
|
||||
test('stripModel() on a doc with no model line is a no-op copy', () => {
|
||||
const result = parseCodexAgentToml(A2_NO_HEADER_FIELDS);
|
||||
assert.equal(result.ok, true);
|
||||
const stripped = stripModel(result.doc);
|
||||
assert.equal(renderCodexAgentToml(stripped), A2_NO_HEADER_FIELDS);
|
||||
});
|
||||
|
||||
test('stripReasoningEffort() on a doc with no effort line is a no-op copy', () => {
|
||||
const result = parseCodexAgentToml(A2_NO_HEADER_FIELDS);
|
||||
assert.equal(result.ok, true);
|
||||
const stripped = stripReasoningEffort(result.doc);
|
||||
assert.equal(renderCodexAgentToml(stripped), A2_NO_HEADER_FIELDS);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── A18-A24: mixed/edge line-ending round-trip (BLOCKER fix, per-line terminators) ──
|
||||
//
|
||||
// Regression coverage for the defect a review caught: `eol` used to be a
|
||||
// single whole-file flag and `split(/\r?\n/)` discarded every line's own
|
||||
// terminator, so `renderCodexAgentToml` re-joined with ONE style and
|
||||
// normalized every line — even when zero strips were performed. None of
|
||||
// A1-A17 exercised a MIXED-ending source (A12 is pure CRLF), so this gap
|
||||
// shipped untested. These fixtures are hand-authored (never emitted by
|
||||
// `generateCodexAgentToml`), per CONTRIBUTING's fixture-provenance rule.
|
||||
|
||||
const A18_MIXED_EOL = 'name = "gsd-mixed"\r\n' +
|
||||
'model = "sonnet"\n' +
|
||||
'description = "mixed line endings"\r\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work.\r\n' +
|
||||
"'''\n";
|
||||
|
||||
const A20_LONE_CR = 'name = "gsd-oldmac"\r' +
|
||||
'model = "sonnet"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work.\n' +
|
||||
"'''\n";
|
||||
|
||||
const A21_NO_TRAILING_AFTER_BLOCK = 'name = "gsd-terse2"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work.\n' +
|
||||
"'''"; // no trailing newline at all — last line is the block's closing '''
|
||||
|
||||
const A22_MULTI_TRAILING_NEWLINES = 'name = "gsd-multi"\n' +
|
||||
'model = "sonnet"\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work.\n' +
|
||||
"'''\n\n\n"; // three trailing newlines after the closing '''
|
||||
|
||||
const A23_BOM_ONLY = String.fromCharCode(0xfeff); // a file that is ONLY a BOM
|
||||
|
||||
const A24_EMPTY = ''; // a genuinely empty file
|
||||
|
||||
describe('A18-A24 (BLOCKER regression): mixed/edge line-ending round-trip', () => {
|
||||
test('A18: mixed \\r\\n and \\n in one file, unmodified — round-trips byte-identically', () => {
|
||||
const result = parseCodexAgentToml(A18_MIXED_EOL);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, 'sonnet');
|
||||
// Each line's own terminator is captured, never collapsed to one style.
|
||||
// No phantom trailing '' entry: the file has exactly 6 content lines, and
|
||||
// `terminators[i]` is the terminator that follows `lines[i]`, so the
|
||||
// arrays are the same length as the file's real line count.
|
||||
assert.deepEqual(result.doc.terminators, ['\r\n', '\n', '\r\n', '\n', '\r\n', '\n']);
|
||||
const rendered = renderCodexAgentToml(result.doc);
|
||||
assert.equal(rendered, A18_MIXED_EOL, 'mixed-EOL source must round-trip byte-identically, unmodified');
|
||||
});
|
||||
|
||||
test('A19: mixed endings + a stale pin stripped — the pin\'s line (and only its own terminator) goes, every other line keeps its original terminator', () => {
|
||||
const result = parseCodexAgentToml(A18_MIXED_EOL);
|
||||
assert.equal(result.ok, true);
|
||||
const stripped = stripModel(result.doc);
|
||||
const rendered = renderCodexAgentToml(stripped);
|
||||
const expected = 'name = "gsd-mixed"\r\n' +
|
||||
'description = "mixed line endings"\r\n' +
|
||||
"developer_instructions = '''\n" +
|
||||
'Work.\r\n' +
|
||||
"'''\n";
|
||||
assert.equal(rendered, expected, 'the model line (and its own \\n) must be gone; every surviving line keeps its own original terminator');
|
||||
assert.equal(stripped.model, null);
|
||||
});
|
||||
|
||||
test('A20: a lone \\r (old-Mac style) somewhere in the file — preserved, not upgraded to \\r\\n or collapsed to \\n', () => {
|
||||
const result = parseCodexAgentToml(A20_LONE_CR);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, 'sonnet');
|
||||
assert.equal(result.doc.terminators[0], '\r', 'the lone CR must be recorded exactly, not merged with the following line');
|
||||
const rendered = renderCodexAgentToml(result.doc);
|
||||
assert.equal(rendered, A20_LONE_CR, 'must round-trip byte-identically, unmodified');
|
||||
});
|
||||
|
||||
test('A21: last line is the block\'s closing \'\'\' with no trailing newline — round-trips byte-identically', () => {
|
||||
const result = parseCodexAgentToml(A21_NO_TRAILING_AFTER_BLOCK);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.trailingNewline, false);
|
||||
const rendered = renderCodexAgentToml(result.doc);
|
||||
assert.equal(rendered, A21_NO_TRAILING_AFTER_BLOCK);
|
||||
});
|
||||
|
||||
test('A22: multiple trailing newlines — every one preserved, round-trips byte-identically', () => {
|
||||
const result = parseCodexAgentToml(A22_MULTI_TRAILING_NEWLINES);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.model, 'sonnet');
|
||||
const rendered = renderCodexAgentToml(result.doc);
|
||||
assert.equal(rendered, A22_MULTI_TRAILING_NEWLINES);
|
||||
});
|
||||
|
||||
test('A23: a file that is only a BOM — parses, round-trips to just the BOM', () => {
|
||||
const result = parseCodexAgentToml(A23_BOM_ONLY);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.hadBOM, true);
|
||||
assert.equal(result.doc.model, null);
|
||||
const rendered = renderCodexAgentToml(result.doc);
|
||||
assert.equal(rendered, A23_BOM_ONLY);
|
||||
});
|
||||
|
||||
test('A24: an empty file — parses, round-trips to an empty string', () => {
|
||||
const result = parseCodexAgentToml(A24_EMPTY);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.hadBOM, false);
|
||||
assert.equal(result.doc.model, null);
|
||||
const rendered = renderCodexAgentToml(result.doc);
|
||||
assert.equal(rendered, A24_EMPTY);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── A25-A28 (regression, B17): removeLine trailing-newline preservation ──────
|
||||
//
|
||||
// A stale/orphaned strip that lands on the file's LAST line must preserve
|
||||
// (not invent, not drop) the file's trailing-newline status. The defect: a
|
||||
// last-line removal used to keep the PREVIOUS line's own terminator, which
|
||||
// only happens to be correct when every terminator in the file is identical
|
||||
// (the common case, which is why A1-A24 never caught it) — it silently
|
||||
// invents a trailing newline whenever the removed line's own terminator
|
||||
// differs from the survivor's, e.g. no trailing newline at all (B17,
|
||||
// commands.test.cjs), or a mixed-EOL source (A26 below).
|
||||
|
||||
describe('A25-A28 (regression, B17): removeLine preserves trailing-newline status', () => {
|
||||
test('A25 (red pre-fix): strip a model line that is the file\'s last line, no trailing newline — still no trailing newline', () => {
|
||||
const source = 'name = "gsd-bare"\nmodel = "sonnet"'; // no trailing \n; model is the last line
|
||||
const result = parseCodexAgentToml(source);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.trailingNewline, false);
|
||||
const rendered = renderCodexAgentToml(stripModel(result.doc));
|
||||
// Pre-fix: removeLine kept the PREVIOUS line's terminator ('\n') instead
|
||||
// of the removed line's own ('') — rendered came back as
|
||||
// 'name = "gsd-bare"\n', a trailing newline the source never had.
|
||||
assert.equal(rendered, 'name = "gsd-bare"', 'the file must still have no trailing newline');
|
||||
});
|
||||
|
||||
test('A26 (red pre-fix): strip a model line that is the file\'s last line, WITH a trailing newline — still exactly one trailing newline', () => {
|
||||
// Mixed EOL is deliberate: it is the only way to distinguish the two
|
||||
// candidate fixes. removeLine must inherit the removed line's
|
||||
// EMPTINESS (trailing-newline-or-not), never its STYLE — the survivor's
|
||||
// OWN terminator is already the right style for the survivor; copying
|
||||
// the removed line's terminator onto it would silently change the
|
||||
// survivor's own ending, which is exactly the class of defect the
|
||||
// mixed-EOL round-trip guarantee (A14) exists to prevent.
|
||||
const source = 'name = "gsd-bare"\r\nmodel = "sonnet"\n'; // name: CRLF: model (last): LF
|
||||
const result = parseCodexAgentToml(source);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.trailingNewline, true);
|
||||
const rendered = renderCodexAgentToml(stripModel(result.doc));
|
||||
// Pre-fix (style-inheriting variant): rendered came back as
|
||||
// 'name = "gsd-bare"\n' — the removed line's own LF terminator, which
|
||||
// silently changed the survivor's ending from CRLF to LF. Correct: the
|
||||
// survivor keeps its OWN CRLF terminator unchanged; the file still ends
|
||||
// with exactly one trailing newline, in the survivor's original style.
|
||||
assert.equal(rendered, 'name = "gsd-bare"\r\n', 'exactly one trailing newline must survive, on the new last line, in ITS OWN style');
|
||||
});
|
||||
|
||||
test('A27: strip the only line in a file — empty result', () => {
|
||||
// Already correct pre-fix (JS Array#slice(1) on a length-1 array is [],
|
||||
// so the old generic slice degenerated to the right answer here) —
|
||||
// included as a regression guard for the new length===1 branch, not
|
||||
// because it was red before the fix.
|
||||
const source = 'model = "sonnet"'; // one line, no trailing newline
|
||||
const result = parseCodexAgentToml(source);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.lines.length, 1);
|
||||
const stripped = stripModel(result.doc);
|
||||
assert.deepEqual(stripped.lines, []);
|
||||
assert.deepEqual(stripped.terminators, []);
|
||||
assert.equal(renderCodexAgentToml(stripped), '');
|
||||
});
|
||||
|
||||
test('A28 (red pre-fix, row B7 shape): stale model plus its orphaned effort, effort is the last line, no trailing newline — both counts correct', () => {
|
||||
const source = 'name = "gsd-stale"\nmodel = "opus"\nmodel_reasoning_effort = "medium"'; // no trailing \n
|
||||
const result = parseCodexAgentToml(source);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.trailingNewline, false);
|
||||
// Sequential removal, same order commands.test.cjs's syncCodex applies:
|
||||
// model first (a middle line — unaffected by the fix), then the now-last
|
||||
// orphaned effort line (the fix's compounding case).
|
||||
const afterModel = stripModel(result.doc);
|
||||
const afterBoth = stripReasoningEffort(afterModel);
|
||||
// Pre-fix: the second removal (now-last-line) kept the intermediate
|
||||
// survivor's terminator instead of the removed effort line's own,
|
||||
// rendering 'name = "gsd-stale"\n' — a trailing newline the source
|
||||
// never had.
|
||||
assert.equal(renderCodexAgentToml(afterBoth), 'name = "gsd-stale"', 'both lines gone AND no invented trailing newline');
|
||||
assert.equal(afterBoth.model, null);
|
||||
assert.equal(afterBoth.reasoningEffort, null);
|
||||
});
|
||||
|
||||
test('A29 (regression, B7 shape, mixed EOL): stale model plus its orphaned effort, effort is the last line, mixed line endings — survivor keeps its OWN terminator, not the removed effort\'s', () => {
|
||||
// Exercises the two fixes' interaction: stripModel is a middle-line
|
||||
// removal (untouched by the last-line fix), then stripReasoningEffort
|
||||
// lands on the now-last line. Deliberately non-uniform EOL either side
|
||||
// of the second removal (survivor 'name' is CRLF-terminated, the
|
||||
// removed effort line is LF-terminated) so a style-inheriting bug and
|
||||
// the correct emptiness-inheriting behavior render DIFFERENT bytes.
|
||||
const source = 'name = "gsd-stale"\r\nmodel = "opus"\nmodel_reasoning_effort = "medium"\n';
|
||||
const result = parseCodexAgentToml(source);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.doc.trailingNewline, true);
|
||||
assert.deepEqual(result.doc.terminators, ['\r\n', '\n', '\n']);
|
||||
const afterModel = stripModel(result.doc);
|
||||
// Middle-line removal: splice only, no terminator fiddling — survivor
|
||||
// keeps its own '\r\n', the effort line keeps its own '\n'.
|
||||
assert.deepEqual(afterModel.terminators, ['\r\n', '\n']);
|
||||
const afterBoth = stripReasoningEffort(afterModel);
|
||||
// If the bug were still present (inheriting the removed effort line's
|
||||
// OWN '\n'), this would render 'name = "gsd-stale"\n' — silently
|
||||
// downgrading the survivor's CRLF to LF. Correct: the survivor's own
|
||||
// '\r\n' is unchanged; the file still ends with exactly one trailing
|
||||
// newline.
|
||||
assert.equal(renderCodexAgentToml(afterBoth), 'name = "gsd-stale"\r\n', 'survivor keeps its OWN terminator; no invented/altered line ending');
|
||||
assert.equal(afterBoth.model, null);
|
||||
assert.equal(afterBoth.reasoningEffort, null);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── A17: enum lock ────────────────────────────────────────────────────────
|
||||
|
||||
describe('PARSE_REASON enum', () => {
|
||||
test('A17: Object.keys(PARSE_REASON).sort() is locked and frozen', () => {
|
||||
assert.deepEqual(Object.keys(PARSE_REASON).sort(), ['UNTERMINATED_BLOCK']);
|
||||
assert.equal(PARSE_REASON.UNTERMINATED_BLOCK, 'unterminated_block');
|
||||
assert.ok(Object.isFrozen(PARSE_REASON));
|
||||
});
|
||||
});
|
||||
@@ -3704,6 +3704,459 @@ describe('feat-488: effort sync command', () => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
});
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
// #3243 (ADR-2313 D7) — the Codex `.toml` branch of `cmdEffortSync`.
|
||||
// Spec: .gsd/phase/feat-3243-codex-toml-sync/{40-design,50-test-matrix}.md
|
||||
// Row numbers below (# B<N>) map 1:1 to 50-test-matrix.md's "B — the sync"
|
||||
// table. B1/B2 (the claude/opencode rows) are the EXISTING tests directly
|
||||
// above this block ('feat-488: effort sync command' + the non-claude-runtime
|
||||
// test) and are asserted UNCHANGED — this describe adds only new coverage.
|
||||
//
|
||||
// Every `.toml` fixture below is hand-authored against the real
|
||||
// `generateCodexAgentToml` shape (CONTRIBUTING's fixture-provenance rule,
|
||||
// #2371), not produced by calling that writer.
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
describe('#3243 (ADR-2313 D7): Codex .toml effort sync', () => {
|
||||
const { PARSE_REASON } = require('../gsd-core/bin/lib/codex-agent-toml.cjs');
|
||||
const FIXTURES_DIR = path.join(__dirname, 'fixtures', 'adversarial', 'toml');
|
||||
|
||||
function writeCodexAgentToml(agentsDir, agentName, content) {
|
||||
fs.mkdirSync(agentsDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(agentsDir, `${agentName}.toml`), content);
|
||||
}
|
||||
|
||||
function syncCodex(tmpDir, dryRun) {
|
||||
const { cmdEffortSync } = require('../gsd-core/bin/lib/commands.cjs');
|
||||
return captureOutput(() =>
|
||||
cmdEffortSync(tmpDir, false, { dryRun, configDir: tmpDir, runtime: 'codex' })
|
||||
);
|
||||
}
|
||||
|
||||
test('B3: model = "sonnet" — the model line stripped; one structured change', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b3-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
writeCodexAgentToml(
|
||||
agentsDir,
|
||||
'gsd-planner',
|
||||
'name = "gsd-planner"\nmodel = "sonnet"\ndeveloper_instructions = \'\'\'\nPlan.\n\'\'\'\n',
|
||||
);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 1);
|
||||
assert.equal(result.skipped, 0);
|
||||
assert.equal(result.changes.length, 1);
|
||||
assert.equal(result.changes[0].agent, 'gsd-planner');
|
||||
assert.equal(result.changes[0].field, 'model');
|
||||
assert.equal(result.changes[0].from, 'sonnet');
|
||||
assert.equal(result.changes[0].to, null);
|
||||
const updated = fs.readFileSync(path.join(agentsDir, 'gsd-planner.toml'), 'utf8');
|
||||
assert.ok(!updated.includes('model = "sonnet"'), 'the stale model line must be gone');
|
||||
assert.ok(updated.includes('name = "gsd-planner"'), 'other lines must survive');
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B4 (negative proof): model = "gpt-5.6-sol" (legal pin) — untouched, reported skipped not synced', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b4-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const content = 'name = "gsd-gpt-agent"\nmodel = "gpt-5.6-sol"\ndeveloper_instructions = \'\'\'\nWork.\n\'\'\'\n';
|
||||
writeCodexAgentToml(agentsDir, 'gsd-gpt-agent', content);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 0, 'a legal pin must never be reported synced');
|
||||
assert.equal(result.skipped, 1);
|
||||
assert.equal(result.changes.length, 0);
|
||||
assert.equal(fs.readFileSync(path.join(agentsDir, 'gsd-gpt-agent.toml'), 'utf8'), content);
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B5 (negative proof): legal pin plus its coupled effort — both untouched', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b5-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const content = 'name = "gsd-pinned-agent"\nmodel = "gpt-5-codex"\nmodel_reasoning_effort = "high"\n' +
|
||||
"developer_instructions = '''\nWork.\n'''\n";
|
||||
writeCodexAgentToml(agentsDir, 'gsd-pinned-agent', content);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 0);
|
||||
assert.equal(result.skipped, 1);
|
||||
assert.equal(fs.readFileSync(path.join(agentsDir, 'gsd-pinned-agent.toml'), 'utf8'), content);
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B6: orphaned model_reasoning_effort, no model — the effort stripped', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b6-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
writeCodexAgentToml(
|
||||
agentsDir,
|
||||
'gsd-orphan-agent',
|
||||
'name = "gsd-orphan-agent"\nmodel_reasoning_effort = "high"\ndeveloper_instructions = \'\'\'\nWork.\n\'\'\'\n',
|
||||
);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 1);
|
||||
assert.equal(result.changes.length, 1);
|
||||
assert.equal(result.changes[0].field, 'model_reasoning_effort');
|
||||
assert.equal(result.changes[0].from, 'high');
|
||||
const updated = fs.readFileSync(path.join(agentsDir, 'gsd-orphan-agent.toml'), 'utf8');
|
||||
assert.ok(!updated.includes('model_reasoning_effort'), 'the orphaned effort must be gone');
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B7: stale model plus its effort — both stripped', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b7-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
writeCodexAgentToml(
|
||||
agentsDir,
|
||||
'gsd-stale-agent',
|
||||
'name = "gsd-stale-agent"\nmodel = "opus"\nmodel_reasoning_effort = "medium"\n' +
|
||||
"developer_instructions = '''\nWork.\n'''\n",
|
||||
);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 1);
|
||||
assert.equal(result.changes.length, 2, 'both the model and the coupled effort must be reported');
|
||||
const fields = result.changes.map(c => c.field).sort();
|
||||
assert.deepEqual(fields, ['model', 'model_reasoning_effort']);
|
||||
const updated = fs.readFileSync(path.join(agentsDir, 'gsd-stale-agent.toml'), 'utf8');
|
||||
assert.ok(!updated.includes('model = "opus"'));
|
||||
assert.ok(!updated.includes('model_reasoning_effort'));
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B8: posture-clean .toml — synced:0, no write (mtime unchanged)', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b8-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
writeCodexAgentToml(
|
||||
agentsDir,
|
||||
'gsd-clean',
|
||||
'name = "gsd-clean"\ndeveloper_instructions = \'\'\'\nWork.\n\'\'\'\n',
|
||||
);
|
||||
const filePath = path.join(agentsDir, 'gsd-clean.toml');
|
||||
const mtimeBefore = fs.statSync(filePath).mtimeMs;
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 0);
|
||||
assert.equal(fs.statSync(filePath).mtimeMs, mtimeBefore, 'a posture-clean file must never be written');
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B9 (boundary): dry-run is the default — changes reported, file byte-identical after', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b9-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const content = 'name = "gsd-planner"\nmodel = "sonnet"\ndeveloper_instructions = \'\'\'\nPlan.\n\'\'\'\n';
|
||||
writeCodexAgentToml(agentsDir, 'gsd-planner', content);
|
||||
|
||||
const { cmdEffortSync } = require('../gsd-core/bin/lib/commands.cjs');
|
||||
const result = captureOutput(() =>
|
||||
cmdEffortSync(tmpDir, false, { configDir: tmpDir, runtime: 'codex' }) // no dryRun key — must default true
|
||||
);
|
||||
|
||||
assert.equal(result.dry_run, true);
|
||||
assert.equal(result.synced, 1, 'the pending strip must still be reported');
|
||||
assert.equal(fs.readFileSync(path.join(agentsDir, 'gsd-planner.toml'), 'utf8'), content, 'dry-run must not write');
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B10: --no-dry-run writes; reports identically to the dry run', () => {
|
||||
const content = 'name = "gsd-planner"\nmodel = "sonnet"\ndeveloper_instructions = \'\'\'\nPlan.\n\'\'\'\n';
|
||||
|
||||
const dryTmpDir = makeTmpDir('codex-sync-b10-dry-');
|
||||
writeCodexAgentToml(makeAgentsDir(dryTmpDir), 'gsd-planner', content);
|
||||
const dryResult = syncCodex(dryTmpDir, true);
|
||||
|
||||
const applyTmpDir = makeTmpDir('codex-sync-b10-apply-');
|
||||
const applyAgentsDir = makeAgentsDir(applyTmpDir);
|
||||
writeCodexAgentToml(applyAgentsDir, 'gsd-planner', content);
|
||||
const applyResult = syncCodex(applyTmpDir, false);
|
||||
|
||||
assert.equal(dryResult.synced, applyResult.synced);
|
||||
assert.deepEqual(dryResult.changes, applyResult.changes, 'the report must match the dry run exactly');
|
||||
assert.equal(dryResult.dry_run, true);
|
||||
assert.equal(applyResult.dry_run, false);
|
||||
assert.ok(!fs.readFileSync(path.join(applyAgentsDir, 'gsd-planner.toml'), 'utf8').includes('model = "sonnet"'));
|
||||
|
||||
cleanup(dryTmpDir);
|
||||
cleanup(applyTmpDir);
|
||||
});
|
||||
|
||||
test('B11 (negative proof): unterminated block — skipped and reported, file byte-identical after', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b11-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const content = 'name = "gsd-broken"\nmodel = "sonnet"\ndeveloper_instructions = \'\'\'\nThis block never closes.\n';
|
||||
writeCodexAgentToml(agentsDir, 'gsd-broken', content);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 0, 'an unparseable document must never be synced');
|
||||
assert.equal(result.skipped, 1);
|
||||
assert.equal(result.refused.length, 1);
|
||||
assert.equal(result.refused[0].agent, 'gsd-broken');
|
||||
assert.equal(result.refused[0].reason, PARSE_REASON.UNTERMINATED_BLOCK);
|
||||
assert.equal(
|
||||
fs.readFileSync(path.join(agentsDir, 'gsd-broken.toml'), 'utf8'),
|
||||
content,
|
||||
'a refused file must never be partially rewritten',
|
||||
);
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B12 (negative proof): symlinked .toml — skipped, target byte-identical after', (t) => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b12-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const targetPath = path.join(tmpDir, 'outside-target.toml');
|
||||
const targetContent = 'model = "sonnet"\n';
|
||||
fs.writeFileSync(targetPath, targetContent);
|
||||
const symlinkPath = path.join(agentsDir, 'gsd-linked.toml');
|
||||
try {
|
||||
fs.symlinkSync(targetPath, symlinkPath, 'file');
|
||||
} catch (error) {
|
||||
if (error && ['EPERM', 'EACCES', 'ENOTSUP'].includes(error.code)) {
|
||||
t.skip('symlink creation is not available on this platform');
|
||||
cleanup(tmpDir);
|
||||
return;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 0);
|
||||
assert.ok(
|
||||
!result.changes.some(c => c.agent === 'gsd-linked'),
|
||||
'a symlinked agent must never be reported as synced',
|
||||
);
|
||||
assert.equal(fs.readFileSync(targetPath, 'utf8'), targetContent, 'the symlink target must never be written through');
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B13: agents dir absent — reports not-found, as the claude path does', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b13-');
|
||||
// agentsDir intentionally not created
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 0);
|
||||
assert.equal(result.reason, 'agents directory not found');
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B14 (hostile, headline data-loss case): model = inside developer_instructions — file byte-identical after a non-dry-run sync', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b14-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const fixtureContent = fs.readFileSync(path.join(FIXTURES_DIR, 'model-in-developer-instructions.toml'), 'utf8');
|
||||
writeCodexAgentToml(agentsDir, 'gsd-planner', fixtureContent);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 0, 'the prose model= inside the block must never be treated as a pin');
|
||||
assert.equal(
|
||||
fs.readFileSync(path.join(agentsDir, 'gsd-planner.toml'), 'utf8'),
|
||||
fixtureContent,
|
||||
'the agent prompt must survive a non-dry-run sync untouched',
|
||||
);
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B15 (cross-platform): CRLF file, stale pin — pin stripped, remaining line endings still CRLF', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b15-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const lfContent = 'name = "gsd-scribe"\nmodel = "sonnet"\ndeveloper_instructions = \'\'\'\nWrite a changelog entry.\n\'\'\'\n';
|
||||
const crlfContent = lfContent.replace(/\n/g, '\r\n');
|
||||
writeCodexAgentToml(agentsDir, 'gsd-scribe', crlfContent);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 1);
|
||||
const updated = fs.readFileSync(path.join(agentsDir, 'gsd-scribe.toml'), 'utf8');
|
||||
assert.ok(!updated.includes('model = "sonnet"'));
|
||||
assert.equal(
|
||||
updated,
|
||||
'name = "gsd-scribe"\r\ndeveloper_instructions = \'\'\'\r\nWrite a changelog entry.\r\n\'\'\'\r\n',
|
||||
'every remaining line ending must still be CRLF',
|
||||
);
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B16 (cross-platform): BOM file, stale pin — pin stripped, BOM preserved', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b16-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const content = String.fromCharCode(0xfeff) +
|
||||
'name = "gsd-archivist"\nmodel = "sonnet"\ndeveloper_instructions = \'\'\'\nArchive.\n\'\'\'\n';
|
||||
writeCodexAgentToml(agentsDir, 'gsd-archivist', content);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 1);
|
||||
const updatedRaw = fs.readFileSync(path.join(agentsDir, 'gsd-archivist.toml'));
|
||||
assert.equal(updatedRaw[0], 0xef, 'BOM byte 1 (EF) must survive');
|
||||
assert.equal(updatedRaw[1], 0xbb, 'BOM byte 2 (BB) must survive');
|
||||
assert.equal(updatedRaw[2], 0xbf, 'BOM byte 3 (BF) must survive');
|
||||
assert.ok(!updatedRaw.toString('utf8').includes('model = "sonnet"'));
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B17 (boundary): file with no trailing newline — preserved', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b17-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const content = 'name = "gsd-bare"\nmodel = "sonnet"'; // deliberately no trailing \n
|
||||
writeCodexAgentToml(agentsDir, 'gsd-bare', content);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 1);
|
||||
const updated = fs.readFileSync(path.join(agentsDir, 'gsd-bare.toml'), 'utf8');
|
||||
assert.equal(updated, 'name = "gsd-bare"', 'the file must still have no trailing newline');
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B18 (negative proof): hand-added approval_policy survives a strip of the stale pin untouched', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b18-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
writeCodexAgentToml(
|
||||
agentsDir,
|
||||
'gsd-custom-agent',
|
||||
'name = "gsd-custom-agent"\nmodel = "sonnet"\napproval_policy = "on-request"\n' +
|
||||
"developer_instructions = '''\nFollow policy.\n'''\n",
|
||||
);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 1);
|
||||
const updated = fs.readFileSync(path.join(agentsDir, 'gsd-custom-agent.toml'), 'utf8');
|
||||
assert.ok(updated.includes('approval_policy = "on-request"'), 'the hand-added key must survive verbatim');
|
||||
assert.ok(!updated.includes('model = "sonnet"'));
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B19 (negative proof): interleaved comments preserved verbatim', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b19-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
writeCodexAgentToml(
|
||||
agentsDir,
|
||||
'gsd-commented',
|
||||
'# top comment\nname = "gsd-commented"\n# a note about the model below\nmodel = "sonnet"\n# trailing comment\n' +
|
||||
"developer_instructions = '''\nWork.\n'''\n",
|
||||
);
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 1);
|
||||
const updated = fs.readFileSync(path.join(agentsDir, 'gsd-commented.toml'), 'utf8');
|
||||
assert.ok(updated.includes('# top comment'));
|
||||
assert.ok(updated.includes('# a note about the model below'));
|
||||
assert.ok(updated.includes('# trailing comment'));
|
||||
assert.ok(!updated.includes('model = "sonnet"'));
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B20 (filesystem failure, atomic-write proof): a mid-write failure is reported; the remaining agents still get processed; the target is left byte-identical, never partially rewritten', (t) => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b20-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
const alphaOriginal = 'name = "gsd-alpha"\nmodel = "sonnet"\ndeveloper_instructions = \'\'\'\nWork.\n\'\'\'\n';
|
||||
writeCodexAgentToml(agentsDir, 'gsd-alpha', alphaOriginal);
|
||||
writeCodexAgentToml(agentsDir, 'gsd-bravo', 'name = "gsd-bravo"\nmodel = "opus"\ndeveloper_instructions = \'\'\'\nWork.\n\'\'\'\n');
|
||||
const failingPath = path.join(agentsDir, 'gsd-alpha.toml');
|
||||
|
||||
// Unlike the old version of this test — which mocked fs.writeFileSync to
|
||||
// throw BEFORE any bytes ever reached disk, proving nothing about a
|
||||
// mid-write failure — this injects the failure at the point a NON-ATOMIC
|
||||
// implementation (`fs.writeFileSync(filePath, ...)` straight into the
|
||||
// target, in place) would already have truncated the real file: 'w'-mode
|
||||
// open+truncate happens before any content is written, so a crash between
|
||||
// open and completion leaves a partial file. The mock actually performs a
|
||||
// REAL (truncated) write to whatever path it's called with — including a
|
||||
// hypothetical direct write to `failingPath` itself — before throwing, so
|
||||
// an in-place implementation's target would end up holding these 4 bytes,
|
||||
// not the original content. An atomic tmp+rename implementation instead
|
||||
// sends this call to a SIBLING tmp path (never `failingPath` itself), so
|
||||
// `failingPath` is never opened for write in the first place and survives
|
||||
// untouched.
|
||||
const realWriteFileSync = fs.writeFileSync;
|
||||
t.mock.method(fs, 'writeFileSync', (target, data, ...args) => {
|
||||
if (typeof target === 'string' && target.startsWith(failingPath)) {
|
||||
realWriteFileSync.call(fs, target, String(data).slice(0, 4));
|
||||
throw Object.assign(new Error('injected ENOSPC (mid-write)'), { code: 'ENOSPC' });
|
||||
}
|
||||
return realWriteFileSync.call(fs, target, data, ...args);
|
||||
});
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 1, 'only the non-failing agent must be reported synced');
|
||||
assert.equal(result.write_failures.length, 1);
|
||||
assert.equal(result.write_failures[0].agent, 'gsd-alpha');
|
||||
assert.ok(
|
||||
!result.changes.some(c => c.agent === 'gsd-alpha'),
|
||||
'a failed write must never be reported as a completed change',
|
||||
);
|
||||
assert.ok(
|
||||
fs.readFileSync(path.join(agentsDir, 'gsd-bravo.toml'), 'utf8').indexOf('model = "opus"') === -1,
|
||||
'the sibling agent must still be synced despite the other write failing',
|
||||
);
|
||||
// The load-bearing assertion (ADR-2313 "never partially rewritten"): the
|
||||
// target must be BYTE-IDENTICAL to its pre-sync content, not merely
|
||||
// "contains model = sonnet somewhere" — a truncated-to-4-bytes file would
|
||||
// pass a substring check but fail this equality. Against the pre-fix
|
||||
// in-place `fs.writeFileSync(filePath, renderCodexAgentToml(doc))`, this
|
||||
// assertion FAILS: that call's target IS `failingPath`, so the mock's
|
||||
// real truncated write lands directly on the file, leaving it as the
|
||||
// 4-byte slice `'name'` instead of `alphaOriginal`.
|
||||
assert.equal(
|
||||
fs.readFileSync(failingPath, 'utf8'),
|
||||
alphaOriginal,
|
||||
'a mid-write failure must leave the original file byte-identical, never partially rewritten',
|
||||
);
|
||||
// The atomic write path cleans up its sibling tmp file on failure — no
|
||||
// stray `.tmp.<pid>` left behind in the agents directory.
|
||||
const leftovers = fs.readdirSync(agentsDir).filter(f => f !== 'gsd-alpha.toml' && f !== 'gsd-bravo.toml');
|
||||
assert.deepEqual(leftovers, [], 'a failed write must not leave a stray tmp file behind');
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('B21 (independence): several agents, mixed states — per-agent results, deterministic order', () => {
|
||||
const tmpDir = makeTmpDir('codex-sync-b21-');
|
||||
const agentsDir = makeAgentsDir(tmpDir);
|
||||
writeCodexAgentToml(agentsDir, 'gsd-alpha', 'name = "gsd-alpha"\ndeveloper_instructions = \'\'\'\nClean.\n\'\'\'\n');
|
||||
writeCodexAgentToml(agentsDir, 'gsd-bravo', 'name = "gsd-bravo"\nmodel = "opus"\ndeveloper_instructions = \'\'\'\nWork.\n\'\'\'\n');
|
||||
writeCodexAgentToml(agentsDir, 'gsd-charlie', 'name = "gsd-charlie"\nmodel_reasoning_effort = "medium"\ndeveloper_instructions = \'\'\'\nWork.\n\'\'\'\n');
|
||||
|
||||
const result = syncCodex(tmpDir, false);
|
||||
|
||||
assert.equal(result.synced, 2, 'gsd-bravo and gsd-charlie must both sync; gsd-alpha is clean');
|
||||
assert.equal(result.skipped, 1);
|
||||
assert.deepEqual(
|
||||
result.changes.map(c => c.agent),
|
||||
['gsd-bravo', 'gsd-charlie'],
|
||||
'agents must be processed in deterministic (sorted) order',
|
||||
);
|
||||
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user