fix: reject wrong-typed reasoner envelope fields as malformed output (#4166)

.changelog/next/fixed-issue-4166.md

@@ -0,0 +1 @@
+- Layered Intelligence now records a wrong-typed reasoner envelope field (e.g. a non-string `analysis`, an unresolvable `pause`) as `unparseable-response` instead of counting it as a successful `no-proposal` run

client/src/components/apps/LayeredIntelligenceTab.test.jsx

@@ -29,7 +29,7 @@ describe('describeLastRun', () => {
   it('surfaces an unparseable reasoning response as a warning with actionable prose', () => {
     const r = describeLastRun({ lastRunAt: at, lastRunAction: 'no-op', lastRunReason: 'unparseable-response' });
     expect(r.tone).toBe('warn');
-    expect(r.text).toMatch(/no usable JSON/i);
+    expect(r.text).toMatch(/no usable answer/i);
   });
 
   it('treats a genuine no-proposal run as neutral (not an error)', () => {
@@ -65,7 +65,7 @@ describe('LayeredIntelligenceTab last-run status', () => {
   it('renders the durable last-run line when the config carries a run outcome', () => {
     render(<LayeredIntelligenceTab li={li({ lastRunAt: '2026-07-09T20:29:05.000Z', lastRunAction: 'no-op', lastRunReason: 'unparseable-response' })} onChange={noop} providers={[]} isPortos loaded />);
     expect(screen.getByText(/Last run/i)).toBeInTheDocument();
-    expect(screen.getByText(/no usable JSON/i)).toBeInTheDocument();
+    expect(screen.getByText(/no usable answer/i)).toBeInTheDocument();
   });
 
   it('omits the last-run line before the loop has ever run', () => {

client/src/utils/layeredIntelligenceReasons.js

@@ -20,7 +20,7 @@ export const LI_NEUTRAL_REASONS = new Set([
 
 const LI_REASON_LABELS = {
   'no-provider': 'no AI provider is configured for it',
-  'unparseable-response': 'the reasoning model returned no usable JSON — try a non-reasoning model or an API provider',
+  'unparseable-response': 'the reasoning model returned no usable answer — no JSON at all, or an envelope field it got wrong (see the server log for which) — try a non-reasoning model or an API provider',
   'no-proposal': 'the loop had nothing to propose',
   'scope-suppressed': "the proposal's scope isn't allowed for this app",
   'hard-gate-excluded': "excluded before filing — the loop's own execution is degraded and this proposal maps to self-improve scope or a chronically-failing domain",

client/src/utils/layeredIntelligenceReasons.test.js

@@ -3,7 +3,7 @@ import { formatLiReason, liReasonTone, LI_NEUTRAL_REASONS } from './layeredIntel
 
 describe('formatLiReason', () => {
   it('glosses a plain reason token', () => {
-    expect(formatLiReason({ action: 'no-op', reason: 'unparseable-response' })).toMatch(/no usable JSON/i);
+    expect(formatLiReason({ action: 'no-op', reason: 'unparseable-response' })).toMatch(/no usable answer/i);
     expect(formatLiReason({ action: 'no-op', reason: 'no-provider' })).toMatch(/no AI provider/i);
   });
 

server/services/autonomousJobs/layeredIntelligenceHooks.js

@@ -589,20 +589,34 @@ export async function processTaskOutput({ appId, success, payload, agentId } = {
   // envelope only requires `payload` to be an object, and salvageSentinelPayload's
   // lenient extractor can surface a non-envelope object out of prose.
   const envelope = isTaskOutputPayload(payload) ? payload : null
-  const { proposal, pause } = validateReasonerResponse(envelope)
-
-  // A reasoner that SUPPLIED a non-null proposal which then failed validation
-  // (missing/unknown scope, no title, unnormalizable slug) did not "look and find
-  // nothing" — it tried to propose and emitted the wrong shape. Both used to land
-  // on `no-proposal` → success. `proposal: null` stays the legitimate empty answer.
-  // Narrow on purpose: validateReasonerResponse is documented to drop invalid
-  // pieces leniently, and this only reclassifies the field that IS the deliverable.
-  const proposalAttemptedButInvalid = envelope != null && !proposal && envelope.proposal != null
+  const { proposal, invalidFields = [], pause } = validateReasonerResponse(envelope)
+
+  // A reasoner that SUPPLIED an envelope field which then failed validation did not
+  // "look and find nothing" — it tried to answer and emitted the wrong shape. That
+  // used to land on `no-proposal` → success. Originally (#2727) only the `proposal`
+  // field was reclassified; #4166 extends it to every supplied-but-unusable field
+  // (`{"analysis": 7}`, a `pause` with no resolvable target), since
+  // validateReasonerResponse now reports them all in `invalidFields`. A `null` field
+  // is ABSENT, not malformed, so `proposal: null` stays the legitimate empty answer.
+  // A null envelope never reaches here with a non-empty list (the validator returns
+  // the empty triple for it), and the ternary below short-circuits on it anyway; the
+  // `= []` destructuring default keeps a stubbed validator from throwing.
+  //
+  // Asymmetry, unchanged from #2727 and deliberate: a SURVIVING proposal overwrites
+  // `reason` on every branch of the filing path below, so a junk `analysis` alongside
+  // a filed proposal records the filing outcome — but a surviving `pause` does NOT
+  // re-derive `reason`, so it still applies its blocking label while the run records
+  // `unparseable-response`. The proposal is the run's deliverable; a pause is a side
+  // effect, and a reasoner that got another field wrong is worth surfacing.
+  const malformedEnvelope = invalidFields.length > 0
+  // Name the offending fields — `unparseable-response` alone can't tell an operator
+  // whether the model returned no JSON at all or one wrong-typed key.
+  if (malformedEnvelope) console.warn(`⚠️ Layered Intelligence: ${app.name} reasoner envelope has unusable fields: ${invalidFields.join(', ')}`)
 
   let filedNumber = null
   let filedKey = null
   let filedAction = 'no-op'
-  let reason = envelope == null || proposalAttemptedButInvalid ? 'unparseable-response' : 'no-proposal'
+  let reason = envelope == null || malformedEnvelope ? 'unparseable-response' : 'no-proposal'
   let handedOff = false
   // §4 (#2764): when the deterministic routing gate files-for-human instead of
   // auto-handing-off a trivial+safe proposal, surface WHY on the returned result.

server/services/autonomousJobs/layeredIntelligenceHooks.test.js

@@ -18,7 +18,11 @@ vi.mock('../../lib/workTracker.js', async (importActual) => {
   };
 });
 
-vi.mock('../../lib/fileUtils.js', () => ({
+// Only `tryReadFile` is stubbed — the rest passes through so the real validator this
+// suite imports for the #4166 end-to-end case can load its own module graph (its
+// constants reach `PATHS`/`DAY` through here). fileUtils has no load-time side effects.
+vi.mock('../../lib/fileUtils.js', async (importOriginal) => ({
+  ...(await importOriginal()),
   tryReadFile: vi.fn().mockResolvedValue(null)
 }));
 
@@ -51,7 +55,7 @@ vi.mock('../layeredIntelligence.js', () => ({
   listForgeIssues: vi.fn().mockResolvedValue({ ok: true, issues: [] }),
   listBlockingIssues: vi.fn().mockResolvedValue({ ok: true, issues: [] }),
   isAppParked: vi.fn(() => false),
-  validateReasonerResponse: vi.fn(() => ({ proposal: null, pause: null })),
+  validateReasonerResponse: vi.fn(() => ({ proposal: null, pause: null, invalidFields: [] })),
   isScopeAllowed: vi.fn(() => true),
   isProposalDuplicate: vi.fn(() => false),
   checkSemanticDuplicate: vi.fn().mockResolvedValue({ available: false, duplicate: false }),
@@ -133,6 +137,10 @@ import { resolveAppWorkTracker } from '../../lib/workTracker.js';
 import { tryReadFile } from '../../lib/fileUtils.js';
 import { getProviderById } from '../providers.js';
 import { getTaskInterval } from '../taskSchedule.js';
+// The REAL validator (its own module, so untouched by the `../layeredIntelligence.js`
+// mock above) — used by the #4166 end-to-end case so the envelope→reason verdict is
+// exercised through production validation, not a hand-written stub of it.
+import { validateReasonerResponse as realValidateReasonerResponse } from '../layeredIntelligence/proposal.js';
 
 const APP = { id: 'app-1', name: 'App One', repoPath: '/repo', taskTypeOverrides: {} };
 
@@ -158,6 +166,9 @@ beforeEach(() => {
   // so without this a test that arms the gate would leak into later ones.
   li.computeHardExclusionGate.mockReturnValue({ excluded: false, reason: null });
   li.computeHardExclusionNotice.mockReturnValue('');
+  // Same leak guard for the validator (#4166): a test that hands back a non-empty
+  // `invalidFields` would otherwise make every later run read as unparseable-response.
+  li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null, invalidFields: [] });
 });
 
 describe('buildTaskInput', () => {
@@ -557,7 +568,7 @@ describe('processTaskOutput', () => {
   });
 
   it('records unparseable-response when the payload is null', async () => {
-    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null });
+    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null, invalidFields: [] });
     const out = await processTaskOutput({ appId: 'app-1', success: true, payload: null });
     expect(out).toMatchObject({ action: 'no-op', reason: 'unparseable-response' });
   });
@@ -569,7 +580,7 @@ describe('processTaskOutput', () => {
     // indistinguishable from a correct empty answer and got recorded as a
     // successful run. `{}` is reachable: the sentinel envelope only requires
     // `payload` to be an object.
-    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null });
+    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null, invalidFields: [] });
     for (const payload of ['just some prose', 42, ['a', 'b'], true, {}, { foo: 1 }]) {
       const out = await processTaskOutput({ appId: 'app-1', success: true, payload });
       expect(out).toMatchObject({ action: 'no-op', reason: 'unparseable-response' });
@@ -580,7 +591,7 @@ describe('processTaskOutput', () => {
     // The other side of the sentinel: the reasoner answered correctly and simply
     // had nothing to file. That is a successful run, not malformed output. Any ONE
     // documented key makes it a real answer.
-    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null });
+    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null, invalidFields: [] });
     for (const payload of [{ analysis: 'nothing worth proposing', proposal: null }, { proposal: null }, { analysis: '' }]) {
       const out = await processTaskOutput({ appId: 'app-1', success: true, payload });
       expect(out).toMatchObject({ action: 'no-op', reason: 'no-proposal' });
@@ -591,13 +602,53 @@ describe('processTaskOutput', () => {
     // The reasoner ATTEMPTED a proposal and emitted the wrong shape (no scope/title,
     // bad slug). That is malformed output, not "I looked and found nothing" — both
     // used to land on `no-proposal` and count as a successful run.
-    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null });
+    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null, invalidFields: ['proposal'] });
     const out = await processTaskOutput({ appId: 'app-1', success: true, payload: { analysis: 'x', proposal: { title: '' } } });
     expect(out).toMatchObject({ action: 'no-op', reason: 'unparseable-response' });
   });
 
+  it('records unparseable-response for a wrong-typed NON-deliverable field (#4166)', async () => {
+    // `{"analysis": 7}` is an envelope (it carries a documented key) that supplies a
+    // field it then gets wrong. Pre-#4166 the deliverable-only check let it through as
+    // `no-proposal`, i.e. a successful run indistinguishable from a correct empty
+    // answer. Any field validateReasonerResponse reports in `invalidFields` now counts.
+    for (const [payload, invalidFields] of [
+      [{ analysis: 7 }, ['analysis']],
+      [{ analysis: 'x', pause: { reason: 'no target' } }, ['pause']],
+      [{ analysis: [], pause: 3 }, ['analysis', 'pause']]
+    ]) {
+      li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null, invalidFields });
+      const out = await processTaskOutput({ appId: 'app-1', success: true, payload });
+      expect(out).toMatchObject({ action: 'no-op', reason: 'unparseable-response' });
+    }
+  });
+
+  it('reaches the same verdict through the REAL validator (#4166)', async () => {
+    // End-to-end over the two halves this suite otherwise stubs apart: the real
+    // envelope validation feeding the real reason classification.
+    li.validateReasonerResponse.mockImplementation(realValidateReasonerResponse);
+    for (const payload of [{ analysis: 7 }, { analysis: 'x', pause: { reason: 'no target' } }, { proposal: { title: '' } }]) {
+      const out = await processTaskOutput({ appId: 'app-1', success: true, payload });
+      expect(out).toMatchObject({ action: 'no-op', reason: 'unparseable-response' });
+    }
+    for (const payload of [{ analysis: 'nothing worth proposing', proposal: null }, { analysis: '' }]) {
+      const out = await processTaskOutput({ appId: 'app-1', success: true, payload });
+      expect(out).toMatchObject({ action: 'no-op', reason: 'no-proposal' });
+    }
+  });
+
+  it('logs which envelope fields were unusable (#4166)', async () => {
+    // `unparseable-response` alone can't tell an operator whether the model returned
+    // no JSON at all or one wrong-typed key — the offending names must reach the log.
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null, invalidFields: ['analysis', 'pause'] });
+    await processTaskOutput({ appId: 'app-1', success: true, payload: { analysis: 7, pause: 3 } });
+    expect(warn).toHaveBeenCalledWith(expect.stringContaining('analysis, pause'));
+    warn.mockRestore();
+  });
+
   it('treats an explicit proposal:null as a legitimate empty answer, not malformed (#2727)', async () => {
-    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null });
+    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null, invalidFields: [] });
     const out = await processTaskOutput({ appId: 'app-1', success: true, payload: { analysis: 'nothing to propose', proposal: null } });
     expect(out).toMatchObject({ action: 'no-op', reason: 'no-proposal' });
   });
@@ -606,7 +657,7 @@ describe('processTaskOutput', () => {
     // readIssues is an unbounded forge call and only the has-a-proposal branch
     // consumes it. Since the #2727 hoist the hook runs while the agent still holds
     // a CoS concurrency slot, so the common no-op path must not shell out to `gh`.
-    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null });
+    li.validateReasonerResponse.mockReturnValue({ proposal: null, pause: null, invalidFields: [] });
     li.listForgeIssues.mockClear();
     await processTaskOutput({ appId: 'app-1', success: true, payload: { analysis: 'nothing to do', proposal: null } });
     expect(li.listForgeIssues).not.toHaveBeenCalled();

server/services/layeredIntelligence.test.js

@@ -375,9 +375,10 @@ describe('validateReasonerResponse', () => {
   });
 
   it('handles garbage input without throwing', () => {
-    expect(validateReasonerResponse(null)).toEqual({ analysis: '', proposal: null, pause: null });
-    expect(validateReasonerResponse('nope')).toEqual({ analysis: '', proposal: null, pause: null });
-    expect(validateReasonerResponse({ proposal: [], pause: [] })).toEqual({ analysis: '', proposal: null, pause: null });
+    expect(validateReasonerResponse(null)).toEqual({ analysis: '', proposal: null, pause: null, invalidFields: [] });
+    expect(validateReasonerResponse('nope')).toEqual({ analysis: '', proposal: null, pause: null, invalidFields: [] });
+    expect(validateReasonerResponse({ proposal: [], pause: [] }))
+      .toEqual({ analysis: '', proposal: null, pause: null, invalidFields: ['proposal', 'pause'] });
   });
 
   it('normalizes proposal complexity + safe (defaults: null / false)', () => {
@@ -394,6 +395,67 @@ describe('validateReasonerResponse', () => {
     expect(junk.proposal.complexity).toBe(null);
     expect(junk.proposal.safe).toBe(false);
   });
+
+  describe('invalidFields (#4166)', () => {
+    it('reports a wrong-typed analysis and drops it', () => {
+      const r = validateReasonerResponse({ analysis: 7 });
+      expect(r.analysis).toBe('');
+      expect(r.invalidFields).toEqual(['analysis']);
+    });
+
+    it('reports a supplied-but-unusable proposal', () => {
+      expect(validateReasonerResponse({ proposal: { scope: 'nuke', slug: 'x', title: 'X' } }).invalidFields).toEqual(['proposal']);
+      expect(validateReasonerResponse({ proposal: 'do the thing' }).invalidFields).toEqual(['proposal']);
+    });
+
+    it('reports a supplied-but-unusable pause', () => {
+      expect(validateReasonerResponse({ pause: { blockOnIssue: 42, reason: '' } }).invalidFields).toEqual(['pause']);
+      expect(validateReasonerResponse({ pause: { reason: 'no target' } }).invalidFields).toEqual(['pause']);
+      // "this" with nothing to block on is malformed, not an empty answer.
+      expect(validateReasonerResponse({ proposal: null, pause: { blockOnIssue: 'this', reason: 'x' } }).invalidFields).toEqual(['pause']);
+    });
+
+    it('does not double-report a "this" pause dropped as collateral of an invalid proposal', () => {
+      // The pause is well-formed — it was dropped only because the proposal it pointed
+      // at failed validation. One root cause, one report.
+      const r = validateReasonerResponse({
+        proposal: { scope: 'nuke', slug: 'x', title: 'X' },
+        pause: { blockOnIssue: 'this', reason: 'block on the new one' }
+      });
+      expect(r.pause).toBe(null);
+      expect(r.invalidFields).toEqual(['proposal']);
+    });
+
+    it('still reports a "this" pause that is itself malformed alongside a bad proposal', () => {
+      // No reason → the pause is broken on its own terms, not collateral.
+      const r = validateReasonerResponse({
+        proposal: { scope: 'nuke', slug: 'x', title: 'X' },
+        pause: { blockOnIssue: 'this', reason: '' }
+      });
+      expect(r.invalidFields).toEqual(['proposal', 'pause']);
+    });
+
+    it('treats an absent or explicitly-null field as absent, never malformed', () => {
+      expect(validateReasonerResponse({ analysis: 'nothing to propose', proposal: null, pause: null }).invalidFields).toEqual([]);
+      expect(validateReasonerResponse({ proposal: null }).invalidFields).toEqual([]);
+      expect(validateReasonerResponse({ analysis: '' }).invalidFields).toEqual([]);
+      expect(validateReasonerResponse({}).invalidFields).toEqual([]);
+    });
+
+    it('stays empty for a fully valid envelope', () => {
+      const r = validateReasonerResponse({
+        analysis: 'a',
+        proposal: { scope: 'app-improvement', slug: 'x', title: 'X' },
+        pause: { blockOnIssue: 'this', reason: 'block on the new one' }
+      });
+      expect(r.invalidFields).toEqual([]);
+    });
+
+    it('collects every unusable field, in envelope order', () => {
+      const r = validateReasonerResponse({ analysis: [], proposal: {}, pause: 3 });
+      expect(r.invalidFields).toEqual(['analysis', 'proposal', 'pause']);
+    });
+  });
 });
 
 describe('isHandoffEligible', () => {