refactor: consolidate markdown code fences, error coercion, and byte-identical helper pairs by steipete · Pull Request #99932 · openclaw/openclaw

steipete · 2026-07-04T10:36:18Z

Related: #99841

What Problem This Solves

Third consolidation pass over the verbatim-duplication backlog catalogued in #99841 (rounds one and two landed as #99850 and #99901). This round removes the remaining high-value exact clones inside core src/ and packages/:

The "grow a Markdown fence/backtick delimiter past embedded backticks" logic existed seven times across four modules (exec approval prompts, approval forwarder, bash exec prompts, OAuth login hints, tool display, auto-reply tool meta, plugin-sdk approval renderers).
toErrorObject (normalize unknown thrown values into an Error with cause + copied enumerable fields) existed six times: src/infra/errors.ts plus verbatim private copies in media-core, memory-host-sdk (twice), and two plugin-sdk runtime files.
Byte-identical helper pairs: usage credential-type bucketing (status text vs status runtime), operator-scope list normalization (gateway vs plugin CLI runtime, where the CLI copy had silently lost deduplication), visible control-character escaping (tool policy audit vs sandbox runtime status), installed-plugin directory listing (two security audits with drifted stat-error handling), proxy-defensive tool-map building (agents inventory vs doctor warnings), toPosixPath (two session tools vs the exported canonical), tilde path expansion (prompt templates vs skills loader), CLI OAuth token-field validation (portal vs Gemini readers), compaction summarization tails (main vs turn-prefix paths), extractAgentIdFromSessionsDir and the dreaming-narrative session-key classifier (two memory-host-sdk files each), and the media chunk-idle-timeout reader (core media fetch vs the media-core superset with proper cancel-with-reason).

Why This Change Was Made

Same policy as the previous rounds: one canonical owner per behavior, chosen by existing ownership rather than new surface.

New src/shared/markdown-code.ts (formatInlineCodeSpan, formatFencedCodeBlock) with a colocated test; all seven sites migrate. The two inline-span variants were verbatim; the OAuth hint variant gains the newline-padding rule (no-op for single-line commands).
toErrorObject moves to @openclaw/normalization-core/error-coercion (new subpath registered in the package exports, build entry list, root tsconfig paths, and the shared Vitest alias table). src/infra/errors.ts re-exports it, so its many existing importers are untouched. The zero-dependency memory-host-sdk package intentionally keeps one internal copy, now shared between its own two files instead of duplicated.
Every pair consolidates into the file that already owns the domain: status/codex-synthetic-usage.ts, gateway/operator-scopes.ts, agents/tool-policy-audit.ts, security/installed-plugin-dirs.ts, agents/tools-effective-inventory-build.ts, shared/ignore-rules.ts (existing exported toPosixPath), and memory-host-sdk/host/openclaw-runtime-session.ts. Only two new modules were added (markdown-code, tilde-path), each with multiple consumers on day one.
Two unifications adopt the strictly-better variant and are called out as intentional: plugin CLI operator-scope normalization now dedupes like the gateway (set semantics), and the extra security audit now surfaces extensions-dir stat failures through its existing scan_failed finding instead of silently reporting "no plugins".

Audited and intentionally left, with reasons recorded: the code-mode worker's inlined JSON serializer (standalone worker bundle cannot import), the plugin-sdk video-generation type mirror (deliberate public-API stability snapshot with assignability assertions and a jscpd-ignore marker), the skills-cli error-handling idiom (13 occurrences deserve a file-wide action wrapper, not a two-site nibble), sync/async file readers in pairing, and several near-clones whose divergence is semantic (platform-specific env setup, deprecated/current API bridges, schema declarations).

User Impact

No user-visible behavior change, with two internal hardening notes: duplicate operator scopes from plugin CLI callers are now deduplicated before gateway requests (previously only the gateway path deduped), and security audits report extensions-directory stat failures instead of silently skipping the scan. Net negative production LOC.

Evidence

Colocated tests for all 25 touched production files pass, plus the new src/shared/markdown-code.test.ts (12 assertions covering delimiter growth, padding, and language fences).
All four tsgo lanes, oxlint/oxfmt on changed files, check-import-cycles (0 cycles), and the plugin-sdk surface check (no surface change this round) are green.
Full build validates the new package subpath end to end.
Codex autoreview caught one real regression in an earlier draft of this branch (the unified plugin-dir lister would have warned on clean installs with no extensions directory); fixed so only non-ENOENT stat failures report, then re-reviewed clean twice (0.81/0.82 on the final diff).

clawsweeper · 2026-07-04T11:37:15Z

Codex review: needs real behavior proof before merge. Reviewed July 4, 2026, 8:35 AM ET / 12:35 UTC.

Summary
The PR consolidates duplicated Markdown fencing, error coercion, path/session/status/security helpers, package aliases, and related tests/configuration across core and packages.

PR surface: Source -224, Tests +50, Config +8, Generated 0, Other 0. Total -166 across 47 files.

Reproducibility: yes. for the review finding: source inspection at the current PR head shows an exported subpath and built consumers but no matching shared tsdown entry. I did not run a package build because this review must keep the checkout read-only.

Review metrics: 1 noteworthy metric.

Package subpath build wiring: 1 added export, 1 missing shared build entry. The new @openclaw/normalization-core/error-coercion export must be emitted by the repo-wide package build before merge.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor/shared/active-tool-schema-warnings.ts, serialized state: packages/memory-host-sdk/src/host/session-files.ts, serialized state: packages/memory-host-sdk/src/host/session-transcript-corpus.ts, serialized state: src/agents/sessions/prompt-templates.ts, serialized state: src/agents/sessions/tools/find.ts, serialized state: src/agents/sessions/tools/read.ts, and 5 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add the missing shared normalization-core tsdown entry.
[P1] Add redacted real behavior proof, such as terminal output showing a built import of @openclaw/normalization-core/error-coercion and representative refactored approval/status rendering.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body lists tests, lint/type/build claims, and review output, but no redacted real setup output, logs, terminal transcript, recording, or artifact showing the changed behavior after the refactor. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] The new @openclaw/normalization-core/error-coercion export points at dist files that the shared repo build does not currently emit.
[P1] Built packages now import the new subpath, so merging as-is can produce module-resolution failures in packaged/runtime execution.
[P1] Real behavior proof is still missing; the PR body lists tests and build claims, but no redacted live output, logs, recording, or artifact showing the changed behavior after the refactor.

Maintainer options:

Fix the package build wiring (recommended)
Add error-coercion to buildNormalizationCoreDistEntries() and rerun focused package/build proof so the exported dist files exist before merge.
Require real setup proof after the fix
Keep the PR open until the contributor adds redacted terminal output, logs, or an artifact showing the built subpath import and representative refactored behavior after the change.
Pause if proof or wiring stays missing
If the build wiring or real setup proof cannot be supplied, maintainers can pause this broad cleanup and ask for a narrower replacement PR.

Next step before merge

[P1] The code repair is narrow, but the PR has a protected maintainer label and still needs contributor real-behavior proof that automation cannot supply.

Security
Cleared: No concrete security or supply-chain concern was found; the blocker is package-build correctness for a new exported subpath.

Review findings

[P1] Add error-coercion to the shared tsdown entries — packages/normalization-core/package.json:22-25

Review details

Best possible solution:

Add the missing normalization-core tsdown entry, keep the helper consolidation scoped to existing owner boundaries, and require redacted real behavior proof before merge.

Do we have a high-confidence way to reproduce the issue?

Yes for the review finding: source inspection at the current PR head shows an exported subpath and built consumers but no matching shared tsdown entry. I did not run a package build because this review must keep the checkout read-only.

Is this the best way to solve the issue?

No: the consolidation is mostly in the right ownership direction, but the new package subpath is not safely integrated until the central package build emits it. The narrow fix is to update buildNormalizationCoreDistEntries() rather than relying on source/test aliases or the package-local script.

Full review comments:

[P1] Add error-coercion to the shared tsdown entries — packages/normalization-core/package.json:22-25
This is still unresolved from the previous review. The PR exports @openclaw/normalization-core/error-coercion to dist/error-coercion.mjs, and built packages now import that subpath, but buildNormalizationCoreDistEntries() still omits the source. The repo-wide package build uses that entry map for packages/normalization-core/dist, so built consumers can resolve the new export to a missing file.
Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 706443c79b1c.

Label changes

Label justifications:

P3: This is a broad cleanup/refactor PR with low direct user-facing urgency, despite a concrete pre-merge package-build blocker.
merge-risk: 🚨 compatibility: The PR adds a package export whose dist target is not emitted by the shared package build.
merge-risk: 🚨 availability: Built modules import the missing subpath, so package/runtime startup can fail after merge.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists tests, lint/type/build claims, and review output, but no redacted real setup output, logs, terminal transcript, recording, or artifact showing the changed behavior after the refactor. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source -224, Tests +50, Config +8, Generated 0, Other 0. Total -166 across 47 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	41	376	600	-224
Tests	2	50	0	+50
Docs	0	0	0	0
Config	2	9	1	+8
Generated	1	2	2	0
Other	1	1	1	0
Total	47	438	604	-166

Acceptance criteria:

[P1] Confirm error-coercion is present in buildNormalizationCoreDistEntries() in tsdown.config.ts.
[P1] After a write-capable fix, run focused normalization-core/package build validation that proves dist/error-coercion.mjs and dist/error-coercion.d.mts are emitted.

What I checked:

Live PR state: Live GitHub data shows this PR is open at head 1328d53 with maintainer, status: 📣 needs proof, compatibility and availability merge-risk labels, and mergeStateStatus UNSTABLE. (1328d53ca201)
New package export: The PR adds ./error-coercion to @openclaw/normalization-core and points it at dist/error-coercion.mjs and dist/error-coercion.d.mts. (packages/normalization-core/package.json:22, 1328d53ca201)
Missing shared build entry: At the current PR head, buildNormalizationCoreDistEntries() still lists index, boolean, number, record, string, and string-normalization entries, but not error-coercion. (tsdown.config.ts:435, 1328d53ca201)
Repo build uses the omitted entry map: The tsdown config uses buildNormalizationCoreDistEntries() for the normalization-core workspace package build, and the root build runs scripts/tsdown-build.mjs. (tsdown.config.ts:734, 1328d53ca201)
Built consumer import: @openclaw/media-core/read-response-with-limit now imports @openclaw/normalization-core/error-coercion, so missing package output would affect built consumers. (packages/media-core/src/read-response-with-limit.ts:2, 1328d53ca201)
Local/test aliases do not prove dist output: The PR adds tsconfig, Vitest, and SDK alias coverage for the new subpath, which explains why local source resolution can pass while the shared package build still omits the dist entry. (src/plugins/sdk-alias.ts:774, 1328d53ca201)

Likely related people:

steipete: Authored this PR, authored the two merged consolidation passes this PR builds on, and merged the build-entry baseline PR that introduced the normalization-core tsdown seam. (role: recent consolidation contributor and merger; confidence: high; commits: 1328d53ca201, eafe2a8d0bb6, be95bb72d459; files: packages/normalization-core/package.json, tsdown.config.ts, src/plugin-sdk/approval-reaction-runtime.ts)
fuller-stack-dev: Authored the merged PR whose current-main commit introduced the explicit normalization-core tsdown package entry map that this PR must extend. (role: adjacent build-surface contributor; confidence: medium; commits: cccc856b82f7; files: tsdown.config.ts, packages/normalization-core/package.json)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (4 earlier review cycles)

reviewed 2026-07-04T11:39:56.277Z sha 3744d97 :: needs real behavior proof before merge. :: none
reviewed 2026-07-04T11:56:02.740Z sha a9339df :: needs real behavior proof before merge. :: none
reviewed 2026-07-04T12:10:59.265Z sha 034a4b7 :: needs real behavior proof before merge. :: none
reviewed 2026-07-04T12:17:10.198Z sha 034a4b7 :: needs real behavior proof before merge. :: [P1] Add error-coercion to the shared tsdown entries

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 034a4b719f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-04T12:09:48Z

+    "./error-coercion": {
+      "types": "./dist/error-coercion.d.mts",
+      "import": "./dist/error-coercion.mjs",
+      "default": "./dist/error-coercion.mjs"


Add error-coercion to the shared tsdown entries

When adding this exported subpath, also add error-coercion to buildNormalizationCoreDistEntries in tsdown.config.ts; the repo build (pnpm build via scripts/tsdown-build.mjs) does not run this package-local script, so it will emit dist/index.mjs but not dist/error-coercion.mjs. Any built package that now imports @openclaw/normalization-core/error-coercion (for example @openclaw/media-core/read-response-with-limit) will then resolve this export to a missing file at runtime.

Useful? React with 👍 / 👎.

…ven call sites

… six copies

… credentials

…dir parsing

…r lister

…ey helper

steipete · 2026-07-04T12:40:45Z

Merged via squash.

Prepared head SHA: 1328d53ca2011e5eedbfeca70f497d102e649ac4
Landed commit: 49cc59b1e88e2b51f0ddc245db5c1a36e2acf400

…identical helper pairs (openclaw#99932) * refactor(shared): add markdown code span/fence helpers and migrate seven call sites * refactor(normalization-core): add canonical toErrorObject and migrate six copies * refactor: consolidate byte-identical helper pairs onto owner modules * refactor: reuse canonical path and token helpers in session tools and credentials * refactor(packages): dedupe compaction summarization tail and session dir parsing * fix(security): keep missing extensions dir silent in shared plugin dir lister * refactor: reuse media-core chunk reader and shared dreaming session key helper * fix(plugins): register error-coercion subpath in sdk alias table * chore(plugin-sdk): re-pin callable export count after rebase * chore(plugin-sdk): re-pin callable export count after rebase

steipete requested a review from a team as a code owner July 4, 2026 10:36

openclaw-barnacle Bot added gateway Gateway runtime commands Command implementations agents Agent runtime and tooling size: XL maintainer Maintainer-authored PR scripts Repository scripts labels Jul 4, 2026

openclaw-barnacle Bot added docs Improvements or additions to documentation channel: slack Channel integration: slack app: web-ui App: web-ui docker Docker and sandbox tooling labels Jul 4, 2026

chatgpt-codex-connector Bot reviewed Jul 4, 2026

View reviewed changes

steipete added 6 commits July 4, 2026 13:26

refactor(shared): add markdown code span/fence helpers and migrate se…

c519369

…ven call sites

refactor(normalization-core): add canonical toErrorObject and migrate…

2fb37c9

… six copies

refactor: consolidate byte-identical helper pairs onto owner modules

2fcab2c

refactor: reuse canonical path and token helpers in session tools and…

1479355

… credentials

refactor(packages): dedupe compaction summarization tail and session …

21207f8

…dir parsing

fix(security): keep missing extensions dir silent in shared plugin di…

e1ef00d

…r lister

steipete added 4 commits July 4, 2026 13:26

refactor: reuse media-core chunk reader and shared dreaming session k…

920e12e

…ey helper

fix(plugins): register error-coercion subpath in sdk alias table

b092ca5

chore(plugin-sdk): re-pin callable export count after rebase

2918fc9

chore(plugin-sdk): re-pin callable export count after rebase

1328d53

steipete force-pushed the claude/dedup-round-3 branch from 034a4b7 to 1328d53 Compare July 4, 2026 12:29

openclaw-barnacle Bot removed channel: slack Channel integration: slack app: web-ui App: web-ui docker Docker and sandbox tooling labels Jul 4, 2026

steipete merged commit 49cc59b into main Jul 4, 2026
101 of 103 checks passed

steipete deleted the claude/dedup-round-3 branch July 4, 2026 12:40

github-actions Bot mentioned this pull request Jul 4, 2026

📡 Upstream Digest — 2026-07-04 13:04 UTC curtismercier/openclaw-mods#1204

Open

clawsweeper Bot mentioned this pull request Jul 4, 2026

fix(auth): classify claude-cli 401 output as oauth refresh failure #98036

Closed

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix(agents): keep chunkString and buildResumeMessage truncation UTF-16 safe #102085

Merged

omarshahine mentioned this pull request Jul 21, 2026

docs: correct RFC 0002 stale references and migration claims openclaw/rfcs#47

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refactor: consolidate markdown code fences, error coercion, and byte-identical helper pairs#99932

refactor: consolidate markdown code fences, error coercion, and byte-identical helper pairs#99932
steipete merged 10 commits into
mainfrom
claude/dedup-round-3

steipete commented Jul 4, 2026

Uh oh!

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 4, 2026

Uh oh!

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

steipete commented Jul 4, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

steipete commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jul 4, 2026 •

edited

Loading