refactor: extract markdown core package by steipete · Pull Request #88265 · openclaw/openclaw

steipete · 2026-05-30T07:04:18Z

Summary

Extract src/markdown into private workspace package @openclaw/markdown-core with explicit build exports.
Rewire core consumers, plugin runtime surfaces, SDK declaration inputs, package-boundary configs, watch paths, and Vitest fast-path coverage.
Keep public plugin SDK declarations self-contained by re-exporting package sources through relative SDK dts paths, not the private workspace package name.

Verification

node scripts/run-vitest.mjs run packages/markdown-core/src/*.test.ts src/plugins/sdk-alias.test.ts src/infra/watch-node.test.ts src/plugins/contracts/extension-package-project-boundaries.test.ts test/vitest-unit-config.test.ts
pnpm check:changed
.agents/skills/autoreview/scripts/autoreview --mode local
pnpm build

Behavior addressed: internal markdown helpers now live under a package boundary without changing markdown parsing/rendering behavior.
Real environment tested: local macOS checkout, Node/pnpm repo toolchain.
Exact steps or command run after this patch: focused Vitest shard list above, pnpm check:changed, autoreview local, pnpm build.
Evidence after fix: focused tests passed; check:changed passed; autoreview clean; build passed.
Observed result after fix: package-boundary dts includes markdown-core outputs and public SDK dts no longer references @openclaw/markdown-core.
What was not tested: live channel delivery; this is an internal package-boundary refactor covered by existing markdown/runtime tests.

github-actions · 2026-05-30T07:04:34Z

Dependency Guard

This PR changes dependency-related files. Maintainers should confirm these changes are intentional.

Changed files:

npm-shrinkwrap.json
package.json
packages/markdown-core/package.json
pnpm-lock.yaml

Maintainer follow-up:

Review whether the dependency changes are intentional.
Inspect resolved package deltas when lockfile, shrinkwrap, or workspace dependency policy changes are present.
Treat package-lock.json and npm-shrinkwrap.json diffs as security-review surfaces.
Run pnpm deps:changes:report -- --base-ref origin/main --markdown /tmp/dependency-changes.md --json /tmp/dependency-changes.json locally for detailed release-style evidence.

github-actions · 2026-05-30T07:04:52Z

Dependency graph change authorized

This PR includes dependency graph changes. A member of @openclaw/openclaw-secops authorized this exact head SHA with /allow-dependencies-change.

Approved SHA: 6f69c727dc104f51e7ec136ddbfe21f8907b3e47
Approved by: @steipete
Reason: private markdown package importer, root markdown-it ownership cleanup, and regenerated root shrinkwrap; dependency report shows no resolved package version changes for 6f69c727dc104f51e7ec136ddbfe21f8907b3e47

A later push changes the PR head SHA and requires a fresh security approval.

clawsweeper · 2026-05-30T07:06:17Z

Codex review: found issues before merge. Reviewed May 30, 2026, 3:23 AM ET / 07:23 UTC.

Summary
The PR extracts src/markdown into a private @openclaw/markdown-core workspace package and rewires core consumers, plugin SDK declarations, build/watch scripts, package-boundary aliases, and focused tests to use it.

PR surface: Source +186, Tests +47, Config +105, Other +77. Total +415 across 58 files.

Reproducibility: yes. for the PR finding: current main trims chunks through chunkTextByBreakResolver, while the PR source pushes the sliced chunk without trimEnd(). I did not run tests because this review is read-only, but the source and existing test contract make the regression reproducible.

Review metrics: 2 noteworthy metrics.

Dependency graph surfaces: 1 new workspace manifest, 1 lockfile importer added. The dependency guard treats these as supply-chain review surfaces that need explicit approval before merge.
Plugin SDK boundary aliases: 1 private package added with 10 package-boundary alias entries. The refactor changes generated plugin SDK declaration resolution, which can affect bundled and third-party plugin package-boundary checks.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

[P2] Fix the soft-boundary trimming regression and add coverage for multi-space chunk boundaries.
Get the dependency graph/package-boundary change explicitly approved by the appropriate maintainer or security owner.

Risk before merge

[P1] The PR changes the dependency graph through a new workspace manifest and pnpm-lock.yaml importer; the dependency guard says the current head remains blocked until security or maintainers approve it.
[P1] The plugin SDK declaration and package-boundary outputs now include private markdown-core package paths, so maintainers should verify generated plugin SDK dts output and external plugin package-boundary compatibility before merge.
[P1] The new markdown-core chunker currently preserves trailing whitespace on soft chunk boundaries that current main trims, which can alter delivered/rendered markdown chunks.

Maintainer options:

Fix behavior before approval (recommended)
Restore the current trimming semantics in packages/markdown-core/src/chunk-text.ts, add the multi-space regression test, then re-run the focused markdown/package-boundary checks before maintainer review.
Approve the dependency graph explicitly
If maintainers want this package boundary, security or the authorized dependency owner should approve the current head after reviewing the new manifest and lockfile importer.
Pause the package extraction
If the private package and SDK dts churn are not worth the compatibility and dependency-review cost, close or pause this PR and keep markdown internals in core for now.

Next step before merge

[P2] Human maintainer/security review is still the right lane because the PR has a protected maintainer label and dependency graph approval blocker; the concrete chunking fix should be handled before that review completes.

Security
Needs attention: The diff introduces a dependency graph change that the repository dependency guard blocks until maintainer/security approval.

Review findings

[P2] Trim soft-break chunks before pushing them — packages/markdown-core/src/chunk-text.ts:60

Review details

Best possible solution:

Land the extraction only after restoring the existing chunk trimming contract with regression coverage and getting explicit maintainer/security approval for the dependency graph and plugin SDK package-boundary changes.

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

Yes for the PR finding: current main trims chunks through chunkTextByBreakResolver, while the PR source pushes the sliced chunk without trimEnd(). I did not run tests because this review is read-only, but the source and existing test contract make the regression reproducible.

Is this the best way to solve the issue?

No, not yet. Extracting markdown into a package is a plausible maintainable direction, but this implementation should preserve the old chunk boundary behavior and clear the dependency/package-boundary approval gates first.

Full review comments:

[P2] Trim soft-break chunks before pushing them — packages/markdown-core/src/chunk-text.ts:60
Current main routes markdown IR chunking through chunkTextByBreakResolver, whose tests lock abc def at limit 6 to ['abc', 'def']. This new loop pushes text.slice(cursor, end) before skipping separators, so the same whitespace run emits abc as the first chunk and can deliver trailing spaces.
Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 5f4fc7512e33.

Label changes

Label changes:

add P2: This is a meaningful internal refactor with a concrete P2 behavior regression and dependency/package-boundary review blockers, but it is not an urgent live-user outage.
add merge-risk: 🚨 compatibility: The PR changes plugin SDK declaration inputs and package-boundary aliases for a private workspace package, which can affect plugin packaging and upgrades.
add merge-risk: 🚨 other: The new package manifest and lockfile importer trigger the repository's dependency graph approval gate, a supply-chain merge risk outside the narrower runtime labels.

Label justifications:

P2: This is a meaningful internal refactor with a concrete P2 behavior regression and dependency/package-boundary review blockers, but it is not an urgent live-user outage.
merge-risk: 🚨 compatibility: The PR changes plugin SDK declaration inputs and package-boundary aliases for a private workspace package, which can affect plugin packaging and upgrades.
merge-risk: 🚨 other: The new package manifest and lockfile importer trigger the repository's dependency graph approval gate, a supply-chain merge risk outside the narrower runtime labels.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes structured after-change proof with local macOS Node/pnpm commands, focused Vitest coverage, pnpm check:changed, autoreview, build, and the observed package-boundary dts result.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes structured after-change proof with local macOS Node/pnpm commands, focused Vitest coverage, pnpm check:changed, autoreview, build, and the observed package-boundary dts result.

Evidence reviewed

PR surface:

Source +186, Tests +47, Config +105, Other +77. Total +415 across 58 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	28	216	30	+186
Tests	19	53	6	+47
Docs	0	0	0	0
Config	4	105	0	+105
Generated	0	0	0	0
Other	7	77	0	+77
Total	58	451	36	+415

Security concerns:

[medium] Dependency graph approval is still required — packages/markdown-core/package.json:58
The new private package manifest declares markdown-it and yaml, and the lockfile adds a packages/markdown-core importer. This is not a vulnerability by itself, but it is a supply-chain review surface and the PR discussion's dependency guard blocks the current head until authorized approval.
Confidence: 0.88

What I checked:

Current trim contract: Current chunkTextByBreakResolver trims the emitted raw chunk before pushing it, preserving whitespace-free chunk boundaries. (src/shared/text-chunking.ts:22, 5f4fc7512e33)
Current regression coverage: The existing test locks the multi-space boundary case abc def at limit 6 to ['abc', 'def'], which is the behavior the refactor needs to preserve. (src/shared/text-chunking.test.ts:34, 5f4fc7512e33)
PR untrimmed chunk: The PR's new markdown-core chunker pushes text.slice(cursor, end) directly, so a whitespace-run soft break can leave trailing whitespace in the emitted chunk. (packages/markdown-core/src/chunk-text.ts:60, 084d7617ca5f)
PR test gap: The added markdown IR chunking test covers a single-space split and final remainder, but not the existing multi-space trimming contract. (packages/markdown-core/src/ir.chunking.test.ts:12, 084d7617ca5f)
Dependency graph change: The new private workspace package declares markdown-it and yaml dependencies, and the lockfile adds a matching packages/markdown-core importer; the GitHub dependency guard in the PR discussion blocks this head pending approval. (packages/markdown-core/package.json:58, 084d7617ca5f)
Plugin SDK declaration boundary: The PR adds markdown-core sources and required dts outputs to the plugin SDK package-boundary artifact pipeline, making this more than a pure file move for plugin packaging. (scripts/prepare-extension-package-boundary-artifacts.mjs:19, 084d7617ca5f)

Likely related people:

steipete: Recent history shows Peter Steinberger extracting related core packages and touching markdown/package-boundary surfaces, so he is a strong routing candidate for this refactor shape. (role: recent package-boundary refactor owner; confidence: high; commits: 7c1484d637c8, aa0d6e1bca55, 04c976b43d50; files: scripts/prepare-extension-package-boundary-artifacts.mjs, src/plugin-sdk/text-chunking.ts, src/markdown/ir.ts)
vincentkoc: Recent commits in the plugin SDK/package-boundary alias and artifact freshness area make Vincent Koc a likely reviewer for the declaration and plugin packaging side. (role: plugin boundary contributor; confidence: medium; commits: 76296a9d145c, 2cab81d9a713, c727388f937f; files: src/plugins/sdk-alias.ts, scripts/prepare-extension-package-boundary-artifacts.mjs, extensions/tsconfig.package-boundary.paths.json)
ngutman: Current-main blame for chunkMarkdownIR and the shared text chunking helper points to Nimrod Gutman's recent broad commit, but the history is broad enough that ownership confidence is lower. (role: recent area contributor; confidence: low; commits: b352cb2d8e7f; files: src/markdown/ir.ts, src/shared/text-chunking.ts)

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.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 06ffb4297b

ℹ️ 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-05-30T07:07:39Z

+    const { lastNewline, lastWhitespace } = scanParenAwareBreakpoints(window);
+    const breakOffset = lastNewline > 0 ? lastNewline : lastWhitespace;
+    const end = breakOffset > 0 ? cursor + breakOffset : windowEnd;
+    chunks.push(text.slice(cursor, end));


Preserve trimming at soft chunk boundaries

When the best break is in a whitespace run, such as abc def with a limit of 5, lastWhitespace points at the second space and this slice keeps the first trailing space in the emitted chunk. The previous chunkTextByBreakResolver path trimmed the chunk end and trimmed/skipped the separator before continuing, so chunkMarkdownIR now produces/render-delivers chunks with stray trailing whitespace in these inputs instead of preserving the prior markdown chunking behavior.

Useful? React with 👍 / 👎.

steipete · 2026-05-30T07:20:04Z

/allow-dependencies-change private markdown package importer; dependency report shows 0 added/removed/changed resolved packages for 084d761

steipete · 2026-05-30T07:26:11Z

/allow-dependencies-change private markdown package importer and root markdown-it ownership cleanup; dependency report shows no resolved package version changes for dde5656

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dde5656981

ℹ️ 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-05-30T07:28:44Z

+  "dependencies": {
+    "markdown-it": "14.2.0",
+    "yaml": "2.9.0"


Regenerate the npm shrinkwrap for the moved dependency

Moving markdown-it into this new package leaves the published npm shrinkwrap stale: npm-shrinkwrap.json still lists markdown-it under the root package dependencies and has no packages/markdown-core package entry. Because the root package ships that shrinkwrap, npm-based installs and lockfile checks continue to consume the old root-owned dependency graph instead of the package-owned one, defeating the extraction and risking package/lock mismatches. Please regenerate/update the shrinkwrap alongside this dependency move.

Useful? React with 👍 / 👎.

steipete · 2026-05-30T07:30:16Z

/allow-dependencies-change private markdown package importer, root markdown-it ownership cleanup, and regenerated root shrinkwrap; dependency report shows no resolved package version changes for 6f69c72

* refactor: extract markdown core package * refactor: remove old markdown sources * fix: use source paths for markdown core imports * fix: clean markdown package dependency ownership * fix: refresh root shrinkwrap for markdown dependency move

steipete added 2 commits May 30, 2026 03:03

refactor: extract markdown core package

d44029e

refactor: remove old markdown sources

06ffb42

steipete requested a review from a team as a code owner May 30, 2026 07:04

openclaw-barnacle Bot added scripts Repository scripts agents Agent runtime and tooling extensions: xai size: M maintainer Maintainer-authored PR labels May 30, 2026

github-actions Bot added the dependencies-changed PR changes dependency-related files label May 30, 2026

chatgpt-codex-connector Bot reviewed May 30, 2026

View reviewed changes

fix: use source paths for markdown core imports

084d761

fix: clean markdown package dependency ownership

dde5656

chatgpt-codex-connector Bot reviewed May 30, 2026

View reviewed changes

fix: refresh root shrinkwrap for markdown dependency move

6f69c72

steipete merged commit 99ffd71 into main May 30, 2026
108 of 109 checks passed

steipete deleted the refactor/markdown-core branch May 30, 2026 07:33

github-actions Bot mentioned this pull request May 30, 2026

📡 Upstream Digest — 2026-05-30 09:51 UTC curtismercier/openclaw-mods#977

Open

vincentkoc mentioned this pull request Jun 13, 2026

fix(markdown-core): treat Infinity chunk limit as unbounded, not 1 #92735

Merged

clawsweeper Bot mentioned this pull request Jun 16, 2026

fix(markdown): align code tables by display width #55596

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refactor: extract markdown core package#88265

refactor: extract markdown core package#88265
steipete merged 5 commits into
mainfrom
refactor/markdown-core

steipete commented May 30, 2026

Uh oh!

github-actions Bot commented May 30, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 30, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 30, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 30, 2026

Uh oh!

steipete commented May 30, 2026

Uh oh!

steipete commented May 30, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 30, 2026

Uh oh!

steipete commented May 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

steipete commented May 30, 2026

Uh oh!

github-actions Bot commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Guard

Uh oh!

github-actions Bot commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency graph change authorized

Uh oh!

clawsweeper Bot commented May 30, 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 May 30, 2026

Choose a reason for hiding this comment

Uh oh!

steipete commented May 30, 2026

Uh oh!

steipete commented May 30, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

steipete commented May 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented May 30, 2026 •

edited

Loading

github-actions Bot commented May 30, 2026 •

edited

Loading

clawsweeper Bot commented May 30, 2026 •

edited

Loading