diff --git a/scripts/dev/gateguard-eval.js b/scripts/dev/gateguard-eval.js index ccc23442d..927272469 100644 --- a/scripts/dev/gateguard-eval.js +++ b/scripts/dev/gateguard-eval.js @@ -24,6 +24,7 @@ const QUESTION_IDS = Object.freeze([ 'callers', 'no-duplicate', 'data-schema', + 'external-contract', 'loader', 'behaviour-change', 'no-duplicate-instruction', diff --git a/scripts/lib/gateguard-target-class.js b/scripts/lib/gateguard-target-class.js index 0cc330eba..0f2e3a4db 100644 --- a/scripts/lib/gateguard-target-class.js +++ b/scripts/lib/gateguard-target-class.js @@ -156,6 +156,8 @@ const QUESTION_TEXT = Object.freeze({ 'no-duplicate': `Confirm no existing file serves the same purpose ${SEARCH_THE_TREE}`, 'data-schema': 'If this file reads/writes data files, show field names, structure, and date format (use redacted or synthetic values, not raw production data)', + 'external-contract': + 'Name what outside this repository decides the format, units, timezone or protocol semantics here (the consumer, the producer, or a stated convention), or state that the choice is unconstrained', loader: 'Name the harness/loader that reads this file (Claude Code, Codex, Cursor, OpenCode, …) and when it loads it', 'behaviour-change': 'Describe what agent behaviour changes as a result', 'no-duplicate-instruction': `Confirm no existing instruction, skill, or agent file already covers this ${SEARCH_THE_TREE}`, @@ -189,7 +191,11 @@ function codeQuestionIds(isWrite, profile) { const surface = !known || profile.touchesPublicSurface !== false; const data = !known || profile.touchesData !== false; const opening = isWrite ? ['callers', 'no-duplicate'] : surface ? ['importers', 'public-api'] : ['local-callers']; - return data ? [...opening, 'data-schema'] : opening; + // A change that handles data is usually constrained by something outside the + // tree - a consumer's dialect, a reporting timezone, a protocol's semantics - + // and the repository cannot settle it. data-schema asks what the change + // touches; external-contract asks who decides what it has to be. + return data ? [...opening, 'data-schema', 'external-contract'] : opening; } /** True when a class's questions depend on the change profile. */ @@ -209,7 +215,7 @@ function questionText(id) { } const CODE_CONDENSED_HINT = - "briefly state importers/callers, affected API, data schemas if any, and the user's verbatim instruction, then retry."; + "briefly state importers/callers, affected API, data schemas if any, what outside the repository fixes the format or semantics, and the user's verbatim instruction, then retry."; const CONDENSED_PHRASES = Object.freeze({ importers: 'the files that import this file', @@ -218,6 +224,7 @@ const CONDENSED_PHRASES = Object.freeze({ callers: 'the file(s) and line(s) that will call it', 'no-duplicate': 'that no existing file serves the same purpose', 'data-schema': 'the data schemas it reads or writes', + 'external-contract': 'what outside the repository fixes the format or semantics', 'quote-instruction': "the user's verbatim instruction" }); diff --git a/tests/fixtures/gateguard-scenarios/01-docs-heavy-session.json b/tests/fixtures/gateguard-scenarios/01-docs-heavy-session.json index ef916402c..2b24a4c90 100644 --- a/tests/fixtures/gateguard-scenarios/01-docs-heavy-session.json +++ b/tests/fixtures/gateguard-scenarios/01-docs-heavy-session.json @@ -951,6 +951,7 @@ "callers", "no-duplicate", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -1019,6 +1020,7 @@ "callers", "no-duplicate", "data-schema", + "external-contract", "quote-instruction" ] }, diff --git a/tests/fixtures/gateguard-scenarios/04-cold-writes.json b/tests/fixtures/gateguard-scenarios/04-cold-writes.json index 15d2030de..26efdee7c 100644 --- a/tests/fixtures/gateguard-scenarios/04-cold-writes.json +++ b/tests/fixtures/gateguard-scenarios/04-cold-writes.json @@ -29,6 +29,7 @@ "callers", "no-duplicate", "data-schema", + "external-contract", "quote-instruction" ] }, diff --git a/tests/fixtures/gateguard-scenarios/07-data-handling-edits.json b/tests/fixtures/gateguard-scenarios/07-data-handling-edits.json index 649c84928..8cdf4183b 100644 --- a/tests/fixtures/gateguard-scenarios/07-data-handling-edits.json +++ b/tests/fixtures/gateguard-scenarios/07-data-handling-edits.json @@ -67,6 +67,7 @@ "relevantQuestions": [ "local-callers", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -135,6 +136,7 @@ "relevantQuestions": [ "local-callers", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -204,6 +206,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -271,6 +274,7 @@ "callers", "no-duplicate", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -339,6 +343,7 @@ "relevantQuestions": [ "local-callers", "data-schema", + "external-contract", "quote-instruction" ] } diff --git a/tests/fixtures/gateguard-scenarios/08-comment-only-edits.json b/tests/fixtures/gateguard-scenarios/08-comment-only-edits.json index e670a0ee8..ec5914b20 100644 --- a/tests/fixtures/gateguard-scenarios/08-comment-only-edits.json +++ b/tests/fixtures/gateguard-scenarios/08-comment-only-edits.json @@ -679,6 +679,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] } diff --git a/tests/fixtures/gateguard-scenarios/09-sensitive-targets.json b/tests/fixtures/gateguard-scenarios/09-sensitive-targets.json index 9090c17b7..b10276418 100644 --- a/tests/fixtures/gateguard-scenarios/09-sensitive-targets.json +++ b/tests/fixtures/gateguard-scenarios/09-sensitive-targets.json @@ -152,6 +152,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -385,6 +386,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -641,6 +643,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] } diff --git a/tests/fixtures/gateguard-scenarios/10-subagent-edits.json b/tests/fixtures/gateguard-scenarios/10-subagent-edits.json index 27bb0cca4..510489599 100644 --- a/tests/fixtures/gateguard-scenarios/10-subagent-edits.json +++ b/tests/fixtures/gateguard-scenarios/10-subagent-edits.json @@ -135,6 +135,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -271,6 +272,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, diff --git a/tests/fixtures/gateguard-scenarios/12-windows-paths.json b/tests/fixtures/gateguard-scenarios/12-windows-paths.json index af4fe20eb..a2009a99d 100644 --- a/tests/fixtures/gateguard-scenarios/12-windows-paths.json +++ b/tests/fixtures/gateguard-scenarios/12-windows-paths.json @@ -368,6 +368,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, diff --git a/tests/fixtures/gateguard-scenarios/16-cap-with-sensitive.json b/tests/fixtures/gateguard-scenarios/16-cap-with-sensitive.json index c76fe8e0a..139c490a5 100644 --- a/tests/fixtures/gateguard-scenarios/16-cap-with-sensitive.json +++ b/tests/fixtures/gateguard-scenarios/16-cap-with-sensitive.json @@ -274,6 +274,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, diff --git a/tests/fixtures/gateguard-scenarios/19-notebook-edits.json b/tests/fixtures/gateguard-scenarios/19-notebook-edits.json index a304ba8b5..9cd22a4c4 100644 --- a/tests/fixtures/gateguard-scenarios/19-notebook-edits.json +++ b/tests/fixtures/gateguard-scenarios/19-notebook-edits.json @@ -71,6 +71,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -321,6 +322,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -442,6 +444,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -490,6 +493,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] } diff --git a/tests/fixtures/gateguard-scenarios/20-hard-linked-targets.json b/tests/fixtures/gateguard-scenarios/20-hard-linked-targets.json index b62fce549..23421e027 100644 --- a/tests/fixtures/gateguard-scenarios/20-hard-linked-targets.json +++ b/tests/fixtures/gateguard-scenarios/20-hard-linked-targets.json @@ -112,6 +112,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -246,6 +247,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] }, @@ -294,6 +296,7 @@ "importers", "public-api", "data-schema", + "external-contract", "quote-instruction" ] } diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index c29542ba3..bd2310127 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -4345,7 +4345,8 @@ function runTests() { '1. Name the file(s) and line(s) that will call this new file', '2. Confirm no existing file serves the same purpose (search the tree — Glob/Grep, or find/grep via Bash)', '3. If this file reads/writes data files, show field names, structure, and date format (use redacted or synthetic values, not raw production data)', - "4. Quote the user's current instruction verbatim", + '4. Name what outside this repository decides the format, units, timezone or protocol semantics here (the consumer, the producer, or a stated convention), or state that the choice is unconstrained', + "5. Quote the user's current instruction verbatim", '', 'If this call was sent in a parallel batch, other edits to src/a.py from that batch may already have been applied. Re-read the file before building on them.', '', @@ -4362,7 +4363,8 @@ function runTests() { '1. List ALL files that import/require this file (search the tree — Glob/Grep, or find/grep via Bash)', '2. List the public functions/classes affected by this change', '3. If this file reads/writes data files, show field names, structure, and date format (use redacted or synthetic values, not raw production data)', - "4. Quote the user's current instruction verbatim", + '4. Name what outside this repository decides the format, units, timezone or protocol semantics here (the consumer, the producer, or a stated convention), or state that the choice is unconstrained', + "5. Quote the user's current instruction verbatim", '', 'If this call was sent in a parallel batch, other edits to src/b.py from that batch may already have been applied. Re-read the file before building on them.', '', @@ -4424,10 +4426,12 @@ function runTests() { callers: 'Name the file(s) and line(s) that will call this new file', noDuplicate: 'Confirm no existing file serves the same purpose (search the tree — Glob/Grep, or find/grep via Bash)', dataSchema: 'If this file reads/writes data files, show field names, structure, and date format (use redacted or synthetic values, not raw production data)', + externalContract: + 'Name what outside this repository decides the format, units, timezone or protocol semantics here (the consumer, the producer, or a stated convention), or state that the choice is unconstrained', quote: QUOTE_LINE }; - const B2_FULL_EDIT = [QUESTION_LINES.importers, QUESTION_LINES.publicApi, QUESTION_LINES.dataSchema, QUESTION_LINES.quote]; - const B2_FULL_WRITE = [QUESTION_LINES.callers, QUESTION_LINES.noDuplicate, QUESTION_LINES.dataSchema, QUESTION_LINES.quote]; + const B2_FULL_EDIT = [QUESTION_LINES.importers, QUESTION_LINES.publicApi, QUESTION_LINES.dataSchema, QUESTION_LINES.externalContract, QUESTION_LINES.quote]; + const B2_FULL_WRITE = [QUESTION_LINES.callers, QUESTION_LINES.noDuplicate, QUESTION_LINES.dataSchema, QUESTION_LINES.externalContract, QUESTION_LINES.quote]; clearState(); if ( @@ -4455,7 +4459,7 @@ function runTests() { if ( test('an Edit that handles data keeps the data-schema question', () => { const reason = profileDenialReason('Edit', { file_path: 'src/calc.py', old_string: ' x = 1', new_string: ' rows = json.load(fh)' }); - assert.deepStrictEqual(listedQuestions(reason), [QUESTION_LINES.localCallers, QUESTION_LINES.dataSchema, QUESTION_LINES.quote]); + assert.deepStrictEqual(listedQuestions(reason), [QUESTION_LINES.localCallers, QUESTION_LINES.dataSchema, QUESTION_LINES.externalContract, QUESTION_LINES.quote]); }) ) passed++; @@ -4562,7 +4566,7 @@ function runTests() { assert.deepStrictEqual(listedQuestions(exported), [QUESTION_LINES.importers, QUESTION_LINES.publicApi, QUESTION_LINES.quote]); clearState(); const fetch = profileDenialReason('Edit', { file_path: 'scripts/q4.sh', old_string: ' run', new_string: ' curl -s "$URL" > out.json' }); - assert.deepStrictEqual(listedQuestions(fetch), [QUESTION_LINES.localCallers, QUESTION_LINES.dataSchema, QUESTION_LINES.quote]); + assert.deepStrictEqual(listedQuestions(fetch), [QUESTION_LINES.localCallers, QUESTION_LINES.dataSchema, QUESTION_LINES.externalContract, QUESTION_LINES.quote]); clearState(); const ps = profileDenialReason('Edit', { file_path: 'scripts/q5.ps1', old_string: 'function Get-A {', new_string: 'function Get-B {' }); assert.deepStrictEqual(listedQuestions(ps), [QUESTION_LINES.importers, QUESTION_LINES.publicApi, QUESTION_LINES.quote]); diff --git a/tests/lib/gateguard-target-class.test.js b/tests/lib/gateguard-target-class.test.js index 14da04545..47a2b2602 100644 --- a/tests/lib/gateguard-target-class.test.js +++ b/tests/lib/gateguard-target-class.test.js @@ -324,21 +324,21 @@ test('every non-code class has questions and a condensed hint; code has neither' test('questionIdsFor gives stable ids per class, action and change profile', () => { const known = (touchesPublicSurface, touchesData) => ({ known: true, language: 'js', touchesPublicSurface, touchesData, trivial: false }); const table = [ - ['code', false, null, ['importers', 'public-api', 'data-schema', 'quote-instruction']], - ['code', true, null, ['callers', 'no-duplicate', 'data-schema', 'quote-instruction']], - ['code', false, { known: false, touchesPublicSurface: false, touchesData: false }, ['importers', 'public-api', 'data-schema', 'quote-instruction']], - ['code', false, known(true, true), ['importers', 'public-api', 'data-schema', 'quote-instruction']], + ['code', false, null, ['importers', 'public-api', 'data-schema', 'external-contract', 'quote-instruction']], + ['code', true, null, ['callers', 'no-duplicate', 'data-schema', 'external-contract', 'quote-instruction']], + ['code', false, { known: false, touchesPublicSurface: false, touchesData: false }, ['importers', 'public-api', 'data-schema', 'external-contract', 'quote-instruction']], + ['code', false, known(true, true), ['importers', 'public-api', 'data-schema', 'external-contract', 'quote-instruction']], ['code', false, known(true, false), ['importers', 'public-api', 'quote-instruction']], - ['code', false, known(false, true), ['local-callers', 'data-schema', 'quote-instruction']], + ['code', false, known(false, true), ['local-callers', 'data-schema', 'external-contract', 'quote-instruction']], ['code', false, known(false, false), ['local-callers', 'quote-instruction']], ['code', true, known(false, false), ['callers', 'no-duplicate', 'quote-instruction']], - ['code', true, known(true, true), ['callers', 'no-duplicate', 'data-schema', 'quote-instruction']], + ['code', true, known(true, true), ['callers', 'no-duplicate', 'data-schema', 'external-contract', 'quote-instruction']], ['instruction', false, known(false, false), ['loader', 'behaviour-change', 'no-duplicate-instruction', 'quote-instruction']], ['test', true, known(false, false), ['under-test', 'existing-tests', 'quote-instruction']], ['prose', true, null, ['supersedes', 'linked-from', 'why-new-file', 'quote-instruction']], ['prose', false, null, ['references', 'corrects-or-adds', 'quote-instruction']], ['config', false, known(false, false), ['config-reader', 'config-effect', 'no-plaintext-secrets', 'quote-instruction']], - ['__proto__', false, null, ['importers', 'public-api', 'data-schema', 'quote-instruction']] + ['__proto__', false, null, ['importers', 'public-api', 'data-schema', 'external-contract', 'quote-instruction']] ]; for (const [cls, isWrite, profile, expected] of table) { const ids = questionIdsFor(cls, isWrite, profile); @@ -363,7 +363,7 @@ test('condensedHintFor matches the question set', () => { test('code condensed hints name exactly the questions of the change profile', () => { const known = (touchesPublicSurface, touchesData) => ({ known: true, language: 'js', touchesPublicSurface, touchesData, trivial: false }); - const ids = ['importers', 'public-api', 'local-callers', 'callers', 'no-duplicate', 'data-schema', 'quote-instruction']; + const ids = ['importers', 'public-api', 'local-callers', 'callers', 'no-duplicate', 'data-schema', 'external-contract', 'quote-instruction']; for (const id of ids) assert.ok(condensedQuestionPhrase(id), `${id} has a phrase`); for (const isWrite of [false, true]) { for (const [s, d] of [[true, true], [true, false], [false, true], [false, false]]) { @@ -404,7 +404,7 @@ test('code condensed hints read as one sentence', () => { ); assert.strictEqual( condensedHintFor('code', false, known(true, true)), - "briefly state the files that import this file, the public functions/classes affected, the data schemas it reads or writes, and the user's verbatim instruction, then retry." + "briefly state the files that import this file, the public functions/classes affected, the data schemas it reads or writes, what outside the repository fixes the format or semantics, and the user's verbatim instruction, then retry." ); assert.strictEqual( condensedHintFor('code', true, known(true, false)), @@ -422,11 +422,11 @@ test('code condensed hints keep the data and duplicate checks when the profile a } }); -test('code condensed Edit hint for an unknown profile is unchanged', () => { - const legacy = "briefly state importers/callers, affected API, data schemas if any, and the user's verbatim instruction, then retry."; +test('code condensed Edit hint for an unknown profile names every question asked', () => { + const expected = "briefly state importers/callers, affected API, data schemas if any, what outside the repository fixes the format or semantics, and the user's verbatim instruction, then retry."; for (const profile of [null, undefined, { known: false, touchesPublicSurface: false, touchesData: false }, 'x']) { - assert.strictEqual(condensedHintFor('code', false, profile), legacy); - assert.strictEqual(condensedHintFor('__proto__', false, profile), legacy); + assert.strictEqual(condensedHintFor('code', false, profile), expected); + assert.strictEqual(condensedHintFor('__proto__', false, profile), expected); } }); @@ -434,7 +434,7 @@ test('code condensed Write hint for an unknown profile asks every creation quest for (const profile of [null, undefined, { known: false, touchesPublicSurface: false, touchesData: false }, 'x']) { assert.strictEqual( condensedHintFor('code', true, profile), - "briefly state the file(s) and line(s) that will call it, that no existing file serves the same purpose, the data schemas it reads or writes, and the user's verbatim instruction, then retry." + "briefly state the file(s) and line(s) that will call it, that no existing file serves the same purpose, the data schemas it reads or writes, what outside the repository fixes the format or semantics, and the user's verbatim instruction, then retry." ); } assert.strictEqual(condensedQuestionPhrase('constructor'), '');