Skip to content

fix: only treat structural issues as fatal in applyRuntimeStateDelta#135

Open
pzysvip99999 wants to merge 2 commits intoNarcooo:masterfrom
pzysvip99999:fix/state-reducer-warning-filter
Open

fix: only treat structural issues as fatal in applyRuntimeStateDelta#135
pzysvip99999 wants to merge 2 commits intoNarcooo:masterfrom
pzysvip99999:fix/state-reducer-warning-filter

Conversation

@pzysvip99999
Copy link
Copy Markdown

Fix: only treat structural issues as fatal in applyRuntimeStateDelta

Problem

applyRuntimeStateDelta() in state-reducer.ts calls validateRuntimeState() and throws whenever issues.length > 0, treating both fatal structural errors (duplicate hook IDs, malformed JSON) and warning-level content contradictions (missing state changes, unsupported changes) as fatal. This causes the pipeline to crash when the LLM reports a missing_state_change or unsupported_change warning — even though these are legitimate content-level observations, not structural failures.

Fix

Filter issues into fatal vs warning before throwing:

const fatalIssues = issues.filter(
  (issue) =>
    !["missing_state_change", "unsupported_change", "temporal_impossibility", "validator_crash"].includes(
      issue.code,
    ),
);
if (fatalIssues.length > 0) {
  throw new Error(fatalIssues.map(...).join("; "));
}
return next; // pipeline continues with warnings logged

Together with PRs #132, #133, #134

PR Layer Fix
#132 state/state-validator.ts Outer try-catch prevents crashes
#133 settler-delta-parser.ts sanitizeJSON() fixes root cause of parse failures
#134 agents/state-validator.ts parseResult() / validate() return graceful result on errors
This state/state-reducer.ts Warning-level issues no longer treated as fatal

After all four PRs: the write pipeline is resilient to both crashes (JSON parse/validation errors) and content-level contradictions (warnings), while still stopping on genuine structural errors.

OpenClaw Bot added 2 commits April 1, 2026 00:23
…ors from crashing the pipeline

The state validator was throwing an uncaught exception when the LLM output
contains invalid JSON characters (e.g. control chars, unescaped sequences),
causing the entire write pipeline to crash with 'State validator returned
invalid JSON'. This wraps the entire function in a try-catch so that any
such errors are returned as a validation issue rather than crashing.
Filter warnings (missing_state_change, unsupported_change,
temporal_impossibility, validator_crash) before throwing, allowing
the pipeline to continue when the LLM reports content-level
contradictions that are not structural errors.
Copy link
Copy Markdown

@JiwaniZakir JiwaniZakir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The decision to include validator_crash in the non-fatal allowlist in state-reducer.ts is worth scrutinizing. A validator_crash means validateRuntimeState threw an unexpected exception mid-execution — validation did not complete, so the returned issues array is incomplete and we have no actual guarantee about the validity of next. Treating this as non-fatal and returning next anyway could silently propagate a structurally broken state, which seems worse than the validator crashing loudly.

The hardcoded string array in the filter (["missing_state_change", "unsupported_change", "temporal_impossibility", "validator_crash"]) is also fragile — if a new non-structural issue code is added to the validator, it will be treated as fatal by default, requiring a second PR to update this list. Consider deriving this from a typed constant or enum shared with the validator, so exhaustiveness can be checked at compile time.

Finally, there's no test coverage shown for the validator_crash path introduced in state-validator.ts. A test that forces parseOrIssue to throw (e.g., by passing a getter that throws on a schema property) would confirm the catch block returns the expected issue shape and that applyRuntimeStateDelta doesn't throw in that scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants