fix(plan-eng-review): register the structure question id so never-ask can fire

The complexity gate's Original/Smaller arrangement question had no
registered question_id, so each run invented a per-plan slug (one install
logged ten different ids for it). --check matches ids exactly, so a stored
never-ask could never fire and the question was asked every time.

- Register plan-eng-review-structure as a two-way routing question.
- The template names that exact id and says AUTO_DECIDE picks the
  recommended arrangement; the preamble's check-first rule does the rest.
  Wording is byte-neutral against the parity budget and the per-host pins.
- --check falls back to $GSTACK_HOME/global-question-preferences.json, the
  file the question-preference hook already reads, when the project file
  has no entry for the id, so one setting covers every repo and both
  layers agree. A project entry still wins, the one-way and split-chain
  guards run first, a malformed or non-object global file is ignored with
  a NOTE, and --write stays per-project.
- Tests for the fallback and the template contract; two wording pins
  updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

bin/gstack-question-preference

@@ -2,6 +2,9 @@
 # gstack-question-preference — read/write/check explicit per-question preferences.
 #
 # Preference file: ~/.gstack/projects/{SLUG}/question-preferences.json
+# Global fallback: ~/.gstack/global-question-preferences.json, the file the
+#   question-preference hook reads (--check only; a project entry for the same
+#   id wins). Hand-edited; --write stays per-project.
 # Schema: { "<question_id>": "always-ask" | "never-ask" | "ask-only-for-one-way" }
 #
 # Subcommands:
@@ -42,6 +45,7 @@ GSTACK_HOME="$_gstack_sr_root"
 eval "$("$SCRIPT_DIR/gstack-slug" 2>/dev/null || true)"
 SLUG="${SLUG:-unknown}"
 PREF_FILE="$GSTACK_HOME/projects/$SLUG/question-preferences.json"
+GLOBAL_PREF_FILE="$GSTACK_HOME/global-question-preferences.json"
 EVENT_FILE="$GSTACK_HOME/projects/$SLUG/question-events.jsonl"
 mkdir -p "$GSTACK_HOME/projects/$SLUG"
 
@@ -101,13 +105,23 @@ do_check() {
   fi
   ensure_file
   cd "$ROOT_DIR"
-  PREF_FILE_PATH="$PREF_FILE" QID="$QID" QSUMMARY="$QSUMMARY" bun -e "
+  PREF_FILE_PATH="$PREF_FILE" GLOBAL_PREF_FILE_PATH="$GLOBAL_PREF_FILE" QID="$QID" QSUMMARY="$QSUMMARY" bun -e "
     import('./scripts/one-way-doors.ts').then((oneway) => {
       const fs = require('fs');
       const qid = process.env.QID;
       const summary = process.env.QSUMMARY || undefined;
       const prefs = JSON.parse(fs.readFileSync(process.env.PREF_FILE_PATH, 'utf-8'));
-      const pref = prefs[qid];
+      let globalPrefs = {};
+      try {
+        if (fs.existsSync(process.env.GLOBAL_PREF_FILE_PATH)) {
+          const parsed = JSON.parse(fs.readFileSync(process.env.GLOBAL_PREF_FILE_PATH, 'utf-8'));
+          if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) throw new Error('not an object');
+          globalPrefs = parsed;
+        }
+      } catch (e) {
+        console.error('NOTE: global-question-preferences.json is unreadable; ignoring it.');
+      }
+      const pref = qid in prefs ? prefs[qid] : globalPrefs[qid];
 
       // Always check one-way status first — safety overrides preferences.
       // summary (when piped) lets the keyword net catch ad-hoc destructive

plan-eng-review/sections/review-sections.md

@@ -441,18 +441,19 @@ Initial scope selectors need no grid or **pre-answer** ledger write. Ask and
 wait before changes.
 
 1. Explain the complexity. Ask each proposed feature cut/deferral separately;
-   wait before changing scope. With no proposed cuts, keep the feature list and
-   go directly to the structure question.
-2. Always ask the structure question when this gate trips, even with no cuts.
-   Compare only the file/class arrangement. Use labels `Original arrangement`
-   and `Smaller arrangement`; put files/classes in each description. Both retain
+   wait before changing scope.
+2. Always resolve the structure question when this gate trips, even with no cuts,
+   as question_id `plan-eng-review-structure` (no per-plan slug); `AUTO_DECIDE`
+   picks the recommendation. Compare only the file/class arrangement, labeled
+   `Original arrangement` and `Smaller arrangement`, with files/classes per
+   option. Both retain
    the same approved feature list, contracts and approved
    security/error/test/performance fixes. Include `Pending remedies not decided here: <ids>` in the
    question; unapproved fixes stay pending. If no smaller arrangement preserves
    these commitments, explain that and offer confirmation of the original
    arrangement or a pause to investigate a smaller one. Wait for the answer.
    A pause leaves the arrangement undecided: investigate only the agreed question,
-   then return to this structure selector. Do not continue to C until it is settled.
+   then return to the selector. Do not continue to C until it is settled.
 3. Save the actual feature and structure answers as one scope record: `feature
    answers: <refs>; structure: <A/B + ref>; accepted scope: <exact scope>;
    pending remedies: <ids or none>`.

plan-eng-review/sections/review-sections.md.tmpl

@@ -339,18 +339,19 @@ Initial scope selectors need no grid or **pre-answer** ledger write. Ask and
 wait before changes.
 
 1. Explain the complexity. Ask each proposed feature cut/deferral separately;
-   wait before changing scope. With no proposed cuts, keep the feature list and
-   go directly to the structure question.
-2. Always ask the structure question when this gate trips, even with no cuts.
-   Compare only the file/class arrangement. Use labels `Original arrangement`
-   and `Smaller arrangement`; put files/classes in each description. Both retain
+   wait before changing scope.
+2. Always resolve the structure question when this gate trips, even with no cuts,
+   as question_id `plan-eng-review-structure` (no per-plan slug); `AUTO_DECIDE`
+   picks the recommendation. Compare only the file/class arrangement, labeled
+   `Original arrangement` and `Smaller arrangement`, with files/classes per
+   option. Both retain
    the same approved feature list, contracts and approved
    security/error/test/performance fixes. Include `Pending remedies not decided here: <ids>` in the
    question; unapproved fixes stay pending. If no smaller arrangement preserves
    these commitments, explain that and offer confirmation of the original
    arrangement or a pause to investigate a smaller one. Wait for the answer.
    A pause leaves the arrangement undecided: investigate only the agreed question,
-   then return to this structure selector. Do not continue to C until it is settled.
+   then return to the selector. Do not continue to C until it is settled.
 3. Save the actual feature and structure answers as one scope record: `feature
    answers: <refs>; structure: <A/B + ref>; accepted scope: <exact scope>;
    pending remedies: <ids or none>`.

scripts/question-registry.ts

@@ -336,6 +336,14 @@ export const QUESTIONS = {
     signal_key: 'scope-appetite',
     description: "Plan touches 8+ files — reduce scope or hold?",
   },
+  'plan-eng-review-structure': {
+    id: 'plan-eng-review-structure',
+    skill: 'plan-eng-review',
+    category: 'routing',
+    door_type: 'two-way',
+    options: ['original-arrangement', 'smaller-arrangement'],
+    description: "Complexity gate — Original arrangement or Smaller arrangement (file/class layout only)?",
+  },
   'plan-eng-review-test-gap': {
     id: 'plan-eng-review-test-gap',
     skill: 'plan-eng-review',

test/eng-review-routing.test.ts

@@ -76,10 +76,16 @@ describe('engineering review routing contracts', () => {
   test('high complexity stops before Section 1 and asks cuts and structure separately', () => {
     expectMentions(compact(complexity), [['stop', 'classes/services', 'section']], 'compact(complexity)');
     ordered(compact(complexity), ['1. Explain the complexity', 'Ask each proposed feature cut/deferral separately',
-      '2. Always ask the structure question', '3. Save the actual feature and structure answers']);
+      '2. Always resolve the structure question', '3. Save the actual feature and structure answers']);
     expect(compact(complexity)).toContain('Pending remedies not decided here: <ids>');
   });
 
+  test('the structure question uses its registered id and honors a stored preference', () => {
+    ordered(compact(complexity), ['2. Always resolve the structure question',
+      'as question_id `plan-eng-review-structure` (no per-plan slug)', '`AUTO_DECIDE` picks the recommendation',
+      'Compare only the file/class arrangement']);
+  });
+
   test('no safe smaller arrangement does not authorize scope cuts or bypass the pause', () => {
     ordered(compact(complexity), ['If no smaller arrangement preserves these commitments',
       'A pause leaves the arrangement undecided', 'Do not continue to C until it is settled']);

test/gstack-question-preference.test.ts

@@ -132,6 +132,57 @@ describe('--check with preferences set', () => {
   });
 });
 
+// The global file ($GSTACK_HOME/global-question-preferences.json, the same file
+// the question-preference hook reads) is a hand-edited fallback for --check
+// only: one setting covers every project, a project entry for the same id
+// wins, and the one-way / split-chain guards still run.
+describe('--check global preference fallback', () => {
+  function plantGlobal(contents: string) {
+    fs.writeFileSync(path.join(tmpHome, 'global-question-preferences.json'), contents);
+  }
+
+  test('global never-ask with no project entry → AUTO_DECIDE', () => {
+    plantGlobal(JSON.stringify({ 'plan-eng-review-structure': 'never-ask' }));
+    const r = run('--check', 'plan-eng-review-structure');
+    expect(r.status).toBe(0);
+    expect(r.stdout.trim()).toBe('AUTO_DECIDE');
+  });
+
+  test('project entry wins over the global entry', () => {
+    plantGlobal(JSON.stringify({ 'plan-eng-review-structure': 'never-ask' }));
+    run('--write', JSON.stringify({ question_id: 'plan-eng-review-structure', preference: 'always-ask', source: 'plan-tune' }));
+    const r = run('--check', 'plan-eng-review-structure');
+    expect(r.stdout.trim()).toBe('ASK_NORMALLY');
+  });
+
+  test('global never-ask on a one-way id → ASK_NORMALLY with safety note', () => {
+    plantGlobal(JSON.stringify({ 'ship-test-failure-triage': 'never-ask' }));
+    const r = run('--check', 'ship-test-failure-triage');
+    expect(r.stdout).toContain('ASK_NORMALLY');
+    expect(r.stdout).toContain('one-way door overrides');
+  });
+
+  test('global never-ask on a split-chain id → ASK_NORMALLY', () => {
+    plantGlobal(JSON.stringify({ 'plan-eng-review-split-option-a': 'never-ask' }));
+    const r = run('--check', 'plan-eng-review-split-option-a');
+    expect(r.stdout.split('\n')[0]).toBe('ASK_NORMALLY');
+  });
+
+  test.each(['{not json', 'null', '["never-ask"]'])('unusable global file %p is ignored with a note', (contents) => {
+    plantGlobal(contents);
+    const r = run('--check', 'plan-eng-review-structure');
+    expect(r.status).toBe(0);
+    expect(r.stdout.trim()).toBe('ASK_NORMALLY');
+    expect(r.stderr).toContain('global-question-preferences.json is unreadable');
+  });
+
+  test('--write stays per-project and never touches the global file', () => {
+    plantGlobal('{}');
+    run('--write', JSON.stringify({ question_id: 'plan-eng-review-structure', preference: 'never-ask', source: 'plan-tune' }));
+    expect(fs.readFileSync(path.join(tmpHome, 'global-question-preferences.json'), 'utf-8')).toBe('{}');
+  });
+});
+
 // #2024: the keyword net only fires when the question TEXT reaches the
 // classifier. --summary-stdin pipes it (stdin, not argv — summaries carry
 // quotes/newlines/shell metacharacters). Without the summary, an unregistered

test/plan-review-cases.test.ts

@@ -319,7 +319,7 @@ test('Eng loads its one remedy procedure before Scope Challenge findings and ret
     const scope = compactProse(sections.slice(sections.indexOf('## Scope Challenge'), sections.indexOf('## Review Sections')));
     expectAll(scope, [/decision-brief format for this complexity gate/i,
       /ask each proposed feature cut\/deferral separately; wait before changing scope/i,
-      /unapproved fixes stay pending/i, /always ask the structure question when this gate trips/i,
+      /unapproved fixes stay pending/i, /always resolve the structure question when this gate trips/i,
       /offer confirmation of the original arrangement or a pause to investigate a smaller one\. wait for the answer/i,
       /apply only accepted scope changes/i, /"No issues found" for an empty list/i, /findings and scope answers approve no remedies/i]);
     const complexityRule = at(scope, /initial scope selectors need no grid or \*\*pre-answer\*\* ledger write/i);