Merge pull request #3164 from affaan-m/fix/3136-gateguard-batch-consistency

fix(gateguard): warn that parallel-batch siblings may already be applied

scripts/hooks/gateguard-fact-force.js

@@ -1101,6 +1101,21 @@ function isReadOnlyGitIntrospection(command) {
 
 // --- Gate messages ---
 
+/**
+ * Batch-consistency warning (#3136). A first-touch denial marks the file
+ * checked so the retry passes; a parallel batch of edits to one
+ * not-yet-touched file therefore partially applies (first call denied,
+ * siblings allowed). Hooks see calls one at a time and cannot lock a
+ * batch, so the denial must say this out loud: name the file and tell
+ * the agent that siblings may already have been applied.
+ */
+function batchSiblingWarning(safePath) {
+  return (
+    `If this call was sent in a parallel batch, other edits to ${safePath} from that batch ` +
+    'may already have been applied. Re-read the file before building on them.'
+  );
+}
+
 function editGateMsg(filePath) {
   const safe = sanitizePath(filePath);
   return [
@@ -1113,6 +1128,8 @@ function editGateMsg(filePath) {
     '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",
     '',
+    batchSiblingWarning(safe),
+    '',
     'Present the facts, then retry the same operation.'
   ].join('\n');
 }
@@ -1129,6 +1146,8 @@ function writeGateMsg(filePath) {
     '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",
     '',
+    batchSiblingWarning(safe),
+    '',
     'Present the facts, then retry the same operation.'
   ].join('\n');
 }
@@ -1143,6 +1162,7 @@ function condensedGateMsg(action, filePath, ordinal) {
   return (
     `[Fact-Forcing Gate] (denial #${ordinal} this session) First ${action} of ${safe}: ` +
     "briefly state importers/callers, affected API, data schemas if any, and the user's verbatim instruction, then retry. " +
+    `${batchSiblingWarning(safe)} ` +
     '(Use GATEGUARD_EXEMPT_GLOBS for path-scoped exemptions; ECC_GATEGUARD=off disables this gate.)'
   );
 }

skills/gateguard/SKILL.md

@@ -89,6 +89,26 @@ Triggers on: `rm -rf`, `git reset --hard`, `git push --force`, `drop table`, etc
 2. What this specific command verifies or produces
 ```
 
+## Parallel Batches and Partial Application
+
+The first-touch gate evaluates each tool call independently. When several
+edits to a file that has not been touched yet are sent in one parallel
+batch, the first call is denied and the denial marks the file as checked,
+so the sibling edits in that batch are applied. Nothing is rolled back:
+the file can end up holding the sibling edits without the denied one.
+
+The denial message names the file and warns that batch siblings may
+already have been applied. Treat it literally:
+
+- Send dependent edits to a not-yet-touched file sequentially, not in a
+  parallel batch. A definition and its first use, or an import and its
+  call site, must not ride in the same batch.
+- After a first-touch denial, present the facts, retry the denied edit,
+  and re-read the file before building on anything else from the batch.
+
+A batch-wide lock is not possible: hooks see tool calls one at a time, so
+the gate cannot know which calls arrived together.
+
 ## Quick Start
 
 ### Option A: Use the ECC hook (zero install)

tests/hooks/gateguard-fact-force.test.js

@@ -3091,6 +3091,93 @@ function runTests() {
     passed++;
   else failed++;
 
+  // --- Batch consistency (#3136): a parallel batch of edits to one ---
+  // not-yet-touched file partially applies: the first denial marks the
+  // file checked, so sibling edits in the same batch are allowed. Hooks
+  // see calls one at a time and cannot lock a batch, so the contract is
+  // that the denial itself names the file and warns that batch siblings
+  // may already have been applied.
+  clearState();
+  if (
+    test('first-touch Edit denial warns about applied batch siblings (#3136)', () => {
+      // Two edits to the same unchecked file, sent as a parallel batch.
+      // Each hook invocation is its own process, exactly as in a batch.
+      const editA = {
+        tool_name: 'Edit',
+        tool_input: { file_path: '/src/batch-target.js', old_string: 'a', new_string: 'b' }
+      };
+      const editB = {
+        tool_name: 'Edit',
+        tool_input: { file_path: '/src/batch-target.js', old_string: 'c', new_string: 'd' }
+      };
+
+      const first = parseOutput(runHook(editA).stdout);
+      assert.strictEqual(first.hookSpecificOutput.permissionDecision, 'deny', 'first edit of the batch is gated');
+      const firstReason = first.hookSpecificOutput.permissionDecisionReason;
+      assert.ok(firstReason.includes('/src/batch-target.js'), 'denial names the exact file');
+      assert.ok(
+        firstReason.includes('parallel batch'),
+        'denial warns that batch siblings may already have been applied'
+      );
+      assert.ok(
+        firstReason.includes('Re-read'),
+        'denial tells the agent to re-read the file before building on siblings'
+      );
+
+      // Sibling edit in the same batch: judged against post-denial state,
+      // so it applies. The warning above is what makes this visible.
+      const second = parseOutput(runHook(editB).stdout);
+      if (second && second.hookSpecificOutput) {
+        assert.notStrictEqual(second.hookSpecificOutput.permissionDecision, 'deny', 'batch sibling is not re-gated');
+      }
+    })
+  )
+    passed++;
+  else failed++;
+
+  clearState();
+  if (
+    test('condensed Edit denial also warns about applied batch siblings (#3136)', () => {
+      writeState({ checked: [], last_active: Date.now(), fact_force_denials: 3 });
+      const result = runHook({ tool_name: 'Edit', tool_input: { file_path: '/src/batch-condensed.js' } });
+      const output = parseOutput(result.stdout);
+      assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny');
+      const reason = output.hookSpecificOutput.permissionDecisionReason;
+      assert.ok(reason.includes('parallel batch'), 'condensed denial keeps the batch-sibling warning');
+      assert.ok(!reason.includes('\n'), 'condensed denial stays a single line');
+    })
+  )
+    passed++;
+  else failed++;
+
+  clearState();
+  if (
+    test('first-touch Write and MultiEdit denials warn about applied batch siblings (#3136)', () => {
+      const writeOut = parseOutput(
+        runHook({ tool_name: 'Write', tool_input: { file_path: '/src/batch-new.js', content: 'x' } }).stdout
+      );
+      assert.strictEqual(writeOut.hookSpecificOutput.permissionDecision, 'deny');
+      assert.ok(
+        writeOut.hookSpecificOutput.permissionDecisionReason.includes('parallel batch'),
+        'Write denial carries the batch-sibling warning'
+      );
+
+      const multiOut = parseOutput(
+        runHook({
+          tool_name: 'MultiEdit',
+          tool_input: { edits: [{ file_path: '/src/batch-multi.js', old_string: 'a', new_string: 'b' }] }
+        }).stdout
+      );
+      assert.strictEqual(multiOut.hookSpecificOutput.permissionDecision, 'deny');
+      assert.ok(
+        multiOut.hookSpecificOutput.permissionDecisionReason.includes('parallel batch'),
+        'MultiEdit denial carries the batch-sibling warning'
+      );
+    })
+  )
+    passed++;
+  else failed++;
+
   // Cleanup only the temp directory created by this test file.
   try {
     if (fs.existsSync(stateDir)) {