* 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>
530 lines
24 KiB
JavaScript
530 lines
24 KiB
JavaScript
'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));
|
|
});
|
|
});
|