* fix(usage-bar): clear dead watcher reference after transient error
When a usage bar template file watcher hits a transient error, the error
handler closes the FSWatcher but leaves entry.watcher pointing to the
closed instance. loadUsageBarTemplate treats a truthy cached.watcher
as "still watching" and never re-reads the file, so a template that
became invalid stays stuck at the default forever.
Clear entry.watcher after closing so the next access that needs a
re-read can create a fresh watcher.
This matches the pattern from #109682 (config hot-reload watcher
recovery).
* fix(usage-bar): invalidate cached template on watcher error
Clear both entry.watcher and entry.template when the FSWatcher errors
so the next loadUsageBarTemplate call re-reads from disk and creates a
fresh watcher. Previously only the watcher reference was cleared,
leaving a valid cached template that never observed future file edits.
This addresses the remaining valid-cache recovery gap from #109682.
* fix(usage-bar): fix no-promise-executor-return lint in test
* fix(config): bound state-directory .env file reads with size limit
Replace unbounded fs.readFileSync with readRegularFileSync capped at
1 MiB (MAX_STATE_DIR_DOTENV_BYTES) so an oversized .env file is
rejected before loading the entire file into memory.
Resolve symlinks via fs.realpathSync before the bounded read so
symlinked .env files keep working — matching the marketplace.ts
pattern for bounded manifest reads.
* fix(config): add diagnostic when oversized state .env is skipped
Log a warning when the state-directory .env file exceeds the 1 MiB
limit so operators know a configured file was skipped — matching the
pattern from #108200 (plugin catalog bounded read diagnostic).
* fix(config): bound global runtime dotenv file reads with size limit
Replace unbounded fs.readFileSync in readDotEnvFile with readRegularFileSync
capped at 1 MiB so CLI/Gateway startup dotenv loading also gets the bounded
read protection — not just the state-dir service-env path.
This addresses the P1 review finding that the original fix only capped the
helper-level reader while the shared readDotEnvFile used by
loadGlobalRuntimeDotEnvFiles was still unbounded.
* fix(config): bound external catalog file reads with size limit
Replace unbounded fs.readFileSync with readRegularFileSync capped at
16 MiB (MAX_EXTERNAL_CATALOG_BYTES) to prevent memory exhaustion from
oversized or malicious plugin catalog files.
Resolve symlinks via fs.realpathSync before the bounded read so
symlinked catalog files keep working — matching the marketplace.ts
pattern for bounded manifest reads.
* fix(config): add diagnostic when oversized catalog is skipped
Log a warning when an external catalog file exceeds the 16 MiB limit
so operators know a configured file was skipped. Add regression test
verifying the oversized catalog is skipped and selection continues.
* fix(config): fix false-positive oversized catalog test
Replace the broken spy-on-object-literal mock with a real sparse file
that genuinely triggers readRegularFileSync rejection via stat.size.
The previous vi.spyOn({ readRegularFileSync }, ...) intercepted a
throwaway object, never the actual module import, so the test passed
regardless of whether the fix was applied or not.
Also fix the no-unused-expressions lint error on the env-primary
assertion that used a discarded ternary (? undefined : undefined)
instead of a proper expect assertion.
* fix(agents): use fatal UTF-8 decoding in provider response readers
* fix(agents): use fatal UTF-8 decoding only for JSON responses, preserve compatibility for text
* fix(config): use Object.hasOwn in restoreEnvVarRefs to avoid prototype pollution
When key comes from Object.entries(incoming) on user config data,
'key in parsed' traverses the prototype chain. If a config key
collides with an Object.prototype property (e.g. 'toString'),
the check incorrectly returns true and calls restoreEnvVarRefs
with a prototype function instead of a config value.
Replace with Object.hasOwn(parsed, key) to check only own properties.
* test(config): prove inherited env refs are ignored
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
Co-authored-by: Peter Steinberger <peter@steipete.me>
* test(utils): add unit tests for parseJsonWithJson5Fallback
* test(utils): verify JSON.parse fast path with spy, per ClawSweeper review
- Add vi.spyOn to prove JSON.parse is called (not JSON5) for strict JSON
- Add spy to prove JSON5.parse is called on fallback path
- Run full src/utils test suite (263 tests passed)
When decodeMcpAppSandboxCsp receives a base64-encoded value that
decodes to non-JSON text, JSON.parse throws an unhandled exception.
Add a try-catch so the function returns undefined instead of crashing.
* test(utils): add unit tests for formatTokenCount
* fix(test): use Number.NaN/Number.POSITIVE_INFINITY per oxlint unicorn/prefer-number-properties
* test(utils): fold formatTokenCount boundary tests into existing usage-format test
Move edge case coverage (invalid inputs, zero/negative, exact boundaries,
thousands overflow) into src/utils/usage-format.test.ts instead of a
separate colocated test file per reviewer feedback.
* fix(envelope): accept timestamp 0 in formatEnvelopeTimestamp
formatEnvelopeTimestamp used if (!ts) to check for missing timestamps,
which incorrectly rejected the valid numeric timestamp 0 (Unix epoch).
Replace the falsy check with an explicit undefined/null check so 0 is
treated as a valid timestamp value.
* refactor(auto-reply): tighten epoch timestamp fix
Exercise the public envelope path with an exact epoch result and keep the missing-value guard aligned with its declared input type.
Co-authored-by: zenglingbiao <zeng.lingbiao@xydigit.com>
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
* fix(restart-sentinel): log DB errors instead of silently swallowing them
Replace empty catch blocks in clearRestartSentinel, readRestartSentinel,
and hasRestartSentinel with sentinelLog.warn(...) calls so SQLite errors
are visible in diagnostics while preserving best-effort fallback semantics.
Tests use deterministic fault injection via vi.mock on openclaw-state-db.js
with hoisted throw flags, replacing the previous non-deterministic state-dir
removal approach. Each error path asserts both the warning message and the
correct fallback return value.
* fix(restart-sentinel): report persistence failures
Co-authored-by: wendy-chsy <wan.wenyan@xydigit.com>
* ci: retrigger pull request checks
* chore(ci): prune stale max-lines baseline
* chore(ci): retest against repaired protocol baseline
* chore: stabilize restart sentinel changelog entry
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
Co-authored-by: wendy-chsy <wan.wenyan@xydigit.com>
* fix(http-error-body): log response body snippet read errors instead of silently returning empty string
readResponseBodySnippet used a bare catch {} that silently swallowed all
errors (network failure, body already consumed, decode errors, stream
aborts) and returned an empty string with no diagnostic. This made it
impossible to distinguish between a genuinely empty response body and
a read failure.
Replace catch {} with catch (err) { errorBodyLog.warn(...); return ""; }
so failures are visible in diagnostics while preserving the best-effort
snippet contract.
* fix(http-error-body): assert subsystem warning in error visibility tests
ClawSweeper review identified that the error-path tests only checked
the empty-string fallback, which the old silent catch would also pass.
Add mockWarn assertions to verify the warning is emitted with the
formatted error text, and add beforeEach to clear mocks between tests.
* test(http-error-body): assert logged read errors
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
* fix(transcript-stream): log stat/open errors instead of silently returning empty iterator
streamSessionTranscriptLines and streamSessionTranscriptLinesReverse
used bare catch { return; } blocks that silently swallowed all fs errors
(permission denied, I/O error, etc.) and returned empty iterators. This
made it impossible to diagnose why transcript streaming failed.
Replace catch { return; } with catch (err) { transcriptLog.warn(...); return; }
so failures are visible in diagnostics.
* fix(transcript-stream): exclude ENOENT from warning logs in stat/open catch blocks
Both streamSessionTranscriptLines and streamSessionTranscriptLinesReverse
caught all fs errors and logged warnings, but ENOENT is an expected empty-
iterator case for these best-effort readers. Filter ENOENT before logging
so missing transcripts stay silent while permission/I/O errors surface.
Added deterministic regression tests with mocked fs.promises.stat/open:
- EACCES triggers a warning in both forward and reverse directions
- ENOENT stays silent in both directions
* fix(transcript): propagate session read failures
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
* fix(cron): reject non-string values in delivery target validation
assertNonBlankStringField had inverted logic — non-string values
(number, boolean, object) were silently accepted because typeof !== 'string'
was grouped with the undefined/null early return. The function name says
'assert non-blank string field' but it was bypassing validation for any
non-string type, defeating the purpose of input validation.
* test(cron): cover invalid delivery fields at entrypoints
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
* fix(web-shared): bound response.text() fallback to honor maxBytes
readResponseText's .text() fallback path ignored the maxBytes option,
reading the full body unbounded. When a foreign Response lacks a body
stream (no getReader) and no arrayBuffer(), the .text() call was the
last resort — but maxBytes was never applied.
- Honor maxBytes in the .text() fallback: truncate + set truncated=true
- Under-cap responses pass through unchanged
- No maxBytes set → original behavior preserved (regression safe)
* fix(web-shared): use byte-level truncation in .text() fallback
Replace text.length/text.slice (character-level) with TextEncoder/
TextDecoder (byte-level) so the maxBytes cap is respected at byte
granularity, consistent with the bounded-stream path at lines 240-298.
- bytes.byteLength > maxBytes instead of text.length > maxBytes
- TextDecoder().decode(bytes.slice(0, maxBytes)) for byte-safe truncation
- bytesRead reports actual bytes, not character count
* fix(web-shared): refuse unbounded .text() when maxBytes is set
When maxBytes is set the caller expects bounded memory. The .text()
fallback path buffers the full body before any post-read check,
defeating the cap. Fail-closed by returning empty with truncated=true
instead of calling .text() unbounded.
Without maxBytes .text() is safe — the caller accepts full allocation.
* fix(web-shared): byte-level truncation in .text() fallback with maxBytes
The .text() fallback used text.length (characters) and text.slice
(char-level) against a byte-level maxBytes cap. Use TextEncoder/
TextDecoder for byte-accurate comparison and truncation.
- bytes.byteLength > maxBytes instead of text.length > maxBytes
- TextDecoder().decode(bytes.slice(0, maxBytes)) for byte-safe truncation
- bytesRead reports actual byte count, not character count
* fix(web): fail closed on unbounded response fallbacks
* test(web): use streaming response fixtures
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
* fix(markdown-core): use Object.hasOwn instead of in operator in parseFrontmatterBlock
* test(markdown-core): add regression test for prototype-named null frontmatter keys
* style(markdown-core): fix unbound-method lint in regression test
---------
Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
* fix(config): use Object.hasOwn instead of in operator in restoreOriginalValueOrThrow
* test(config): add regression test for prototype-named keys in restoreRedactedValues
* test(config): focus prototype restore regression
---------
Co-authored-by: Peter Steinberger <steipete@gmail.com>
* fix(cron): truncate failure alert error text on UTF-16 boundary
* test(cron): add regression test for UTF-16 safe failure alert truncation
Verify that emitFailureAlert does not produce dangling surrogates when
truncating a cron failure error message with a non-BMP character (emoji)
straddling the 200-code-unit truncation boundary.
* fix(test): cover last code unit in surrogate scan
The dangling surrogate check loop stopped at length-1, missing a
high surrogate at the final position. Extend to i < alertText.length
so charCodeAt(i+1) returns NaN for the last char, correctly failing
the high-surrogate pair assertion.
* fix(test): address lint issues in regression test
- Use template literal instead of string concatenation
- Add braces to type guard if-statement
* fix(shared): use UTF-16 safe truncation in subagent line display
truncateLine sliced by code units instead of preserving surrogate pairs,
causing emoji / CJK Extension B characters at the truncation boundary
to display as broken replacement characters.
* fix(shared): move import above export to satisfy import/first lint rule
truncateLine could cut a surrogate pair when the maxChars boundary
fell between a high surrogate and its paired low surrogate, producing
a broken unpaired surrogate in grep tool output.
* fix(shared): truncate assistant error text on UTF-16 boundary
Use truncateUtf16Safe from the existing utf16-slice utility instead of
String.prototype.slice(0, 600), which can split a surrogate pair and
produce malformed Unicode when the error message contains characters
outside the Basic Multilingual Plane (emoji, rare CJK, etc.).
* test(shared): add regression test for UTF-16 safe error truncation
Verify formatRawAssistantErrorForUi does not produce dangling surrogates
when truncating a fallback raw error message with a non-BMP character
(emoji) straddling the 600-code-unit boundary.
* fix(agents): truncate assistant error text fallback on UTF-16 boundary
Use truncateUtf16Safe instead of String.prototype.slice(0,600) in
formatAssistantErrorText and its companion comparison in
isRawAssistantErrorPassthrough, preventing dangling surrogates when
non-BMP characters straddle the 600-code-unit truncation boundary.
Summary:
- The PR imports `truncateUtf16Safe` and uses it for the Codex usage-limit preview and verbose working-label truncation paths in auto-reply.
- PR surface: Source +2. Total +2 across 2 files.
- Reproducibility: yes. Current main uses raw `slice(0, N)` at both reported user-facing truncation sites, and ... ludes terminal before/after output showing dangling surrogates before the fix and safe truncation after it.
Automerge notes:
- No ClawSweeper repair was needed after automerge opt-in.
Validation:
- ClawSweeper review passed for head 74a0a32ed9.
- Required merge gates passed before the squash merge.
Prepared head SHA: 74a0a32ed9
Review: https://github.com/openclaw/openclaw/pull/97299#issuecomment-4820038635
Co-authored-by: zenglingbiao <zeng.lingbiao@xydigit.com>
Approved-by: takhoffman
Prevent duplicate `before_tool_call` execution when an already wrapped tool passes through schema normalization and coding-tool assembly. Preserve the normalized schema while replacing stale wrapper context with the current agent/session/run context.
Fixes#92973.
Co-authored-by: zengLingbiao <zeng.lingbiao@xydigit.com>
Align the export-session template asset copy step and build-all cache output with the runtime lookup path so published packages include the HTML export assets at `dist/export-html`.
Adds a focused build-all regression assertion for the output path contract.
Fixes#90843