fix(skill-workshop): preserve proposal terminal newline by anyech · Pull Request #100293 · openclaw/openclaw

anyech · 2026-07-05T10:50:11Z

Summary

Preserve proposal_content exactly at the Skill Workshop tool boundary by disabling string trimming for that parameter.
Keep rejecting whitespace-only proposal_content before the raw string reaches proposal persistence.
Make proposal markdown rendering defensively end with a terminal newline.
Add focused regressions for both the renderer and the tool write path.

What Problem This Solves

Skill Workshop proposal content is markdown supplied through an agent tool boundary. Trimming the raw string can remove a meaningful terminal newline before the proposal is rendered to PROPOSAL.md. The fix preserves raw valid markdown, while retaining the existing safety behavior that blank or whitespace-only proposal content is rejected instead of persisted as a frontmatter-only proposal.

Evidence

Current-head terminal proof from a disposable test state:

whitespace-only proposal_content: rejected (proposal_content required)
proposal_id: terminal-newline-proof-20260705-e5f8628f80
PROPOSAL.md last byte: 0x0a
PROPOSAL.md byte length: 230

Current-head validation after merging latest main:

Skill Workshop service Vitest: 26 tests passed.
Tooling project-list Vitest: 182 tests passed.
Skill Workshop tool Vitest: 7 tests passed.
pnpm check:test-types: passed.
pnpm tsgo:core: passed.
targeted oxlint: passed.
git diff --check: clean.

Boundary

No runtime or packaged dist/*.js hotpatch.
No live Gateway restart, reload, apply, or update.
Proof used disposable local test state only.

Preserve proposal_content exactly at the agent tool boundary and make renderProposalMarkdown defensively emit a terminal newline. Add focused regressions for the tool write path and markdown renderer.

clawsweeper · 2026-07-05T11:07:59Z

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 8:38 AM ET / 12:38 UTC.

Summary
This PR preserves raw valid Skill Workshop proposal_content, keeps whitespace-only proposal content rejected, ensures rendered PROPOSAL.md ends with a newline, and adds focused regressions.

PR surface: Source +5, Tests +64. Total +69 across 4 files.

Reproducibility: yes. by source inspection. Current main trims proposal_content before persistence through readStringParam, and the current renderer can return proposal markdown without a final newline.

Review metrics: 1 noteworthy metric.

Required tool string handling: 1 changed. proposal_content is a required tool parameter whose raw markdown preservation now depends on the paired explicit whitespace-only guard.

Stored data model
Persistent data-model change detected: serialized state: src/agents/tools/skill-workshop-tool.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

none.

Next step before merge

No repair lane is needed because the prior blank-content blocker is fixed and the remaining action is ordinary maintainer review and required checks.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes local TypeScript tool parsing/rendering behavior and tests only.

Review details

Best possible solution:

Merge the focused tool-boundary and renderer fix after normal required checks complete, keeping whitespace-only rejection and avoiding any new config or migration surface.

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

Yes by source inspection. Current main trims proposal_content before persistence through readStringParam, and the current renderer can return proposal markdown without a final newline.

Is this the best way to solve the issue?

Yes. The fix is at the narrow owner boundary: preserve raw valid markdown in the tool, reject whitespace-only input explicitly, and make the shared renderer terminal-newline-safe.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains current-head terminal output showing whitespace-only rejection and PROPOSAL.md ending with byte 0x0a.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body contains current-head terminal output showing whitespace-only rejection and PROPOSAL.md ending with byte 0x0a.
remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: This is a focused Skill Workshop bug fix with limited blast radius in an agent tool boundary and proposal artifact rendering.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body contains current-head terminal output showing whitespace-only rejection and PROPOSAL.md ending with byte 0x0a.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains current-head terminal output showing whitespace-only rejection and PROPOSAL.md ending with byte 0x0a.

Evidence reviewed

PR surface:

Source +5, Tests +64. Total +69 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	6	1	+5
Tests	2	64	0	+64
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	70	1	+69

What I checked:

Repository policy read: Root AGENTS.md plus scoped src/agents/AGENTS.md and src/agents/tools/AGENTS.md were read; their full-code-path and proof guidance affected this review. (AGENTS.md:1, deac98eb7204)
Current main string helper trims by default: readStringParam defaults trim to true, so a required string is trimmed before empty validation unless the caller opts out. (src/agents/tools/common.ts:130, deac98eb7204)
Current main trims proposal content at the tool boundary: The current Skill Workshop tool reads proposal_content as a required string without overriding the default trim behavior, then passes it into proposal creation/update/revision. (src/agents/tools/skill-workshop-tool.ts:205, deac98eb7204)
Current main renderer can omit the terminal newline: The current renderer returns the frontmatter/body string directly, so body content without a final newline produces a PROPOSAL.md without one. (src/skills/workshop/frontmatter.ts:50, deac98eb7204)
PR head preserves raw valid content while rejecting blank content: The PR opts proposal_content out of automatic trimming and then explicitly rejects whitespace-only content before it reaches the service. (src/agents/tools/skill-workshop-tool.ts:205, aa09b24b4ff7)
PR head makes proposal rendering newline-safe: The renderer now appends a final newline only when the constructed proposal markdown does not already end with one. (src/skills/workshop/frontmatter.ts:50, aa09b24b4ff7)

Likely related people:

steipete: Current-main blame and the merged source PR for the Skill Workshop tool, proposal renderer, and service tests point to steipete as the author and merger of the feature introduction. (role: feature introducer and likely follow-up owner; confidence: medium; commits: 4deb63c9791c; files: src/agents/tools/skill-workshop-tool.ts, src/skills/workshop/frontmatter.ts, src/skills/workshop/service.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.

Review history (2 earlier review cycles)

reviewed 2026-07-05T11:11:24.358Z sha 865212b :: needs real behavior proof before merge. :: [P2] Keep rejecting blank proposal content
reviewed 2026-07-05T12:30:45.127Z sha aa09b24 :: needs maintainer review before merge. :: none

…workshop

anyech · 2026-07-05T12:01:58Z

@clawsweeper re-review

Current-head update for #100293.

What changed:

Merged latest main to pick up the current tooling project list.
Preserved raw valid proposal_content while restoring the required nonblank guard for whitespace-only input.
Added a regression for whitespace-only rejection plus raw valid markdown terminal-newline preservation.
Updated the PR body with authored What Problem This Solves and Evidence sections.

Current-head validation on 4833a98635ea:

Disposable terminal proof showed whitespace-only proposal_content rejected and PROPOSAL.md last byte: 0x0a.
Skill Workshop service Vitest: 26 tests passed.
Tooling project-list Vitest: 182 tests passed.
Skill Workshop tool Vitest: 7 tests passed.
pnpm check:test-types: passed.
pnpm tsgo:core: passed.
targeted oxlint: passed.
git diff --check: clean.

No live Gateway/runtime state was used.

clawsweeper · 2026-07-05T12:02:00Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

…workshop

anyech · 2026-07-05T12:29:09Z

@clawsweeper re-review

Current-head update for #100293.

What changed:

Merged latest main again after the tooling test project list changed.
This picked up the new test/scripts/package-git-fixture.test.ts routing expectation.
PR body already contains authored What Problem This Solves and Evidence sections with the terminal newline proof.

Validation on current head aa09b24b4ff7:

Exact failed QA/tooling test: test/scripts/test-projects.test.ts, 182 tests passed.
Skill Workshop service Vitest: 26 tests passed.
Skill Workshop tool Vitest: 7 tests passed.
git diff --check: clean.

Earlier current-head proof showed whitespace-only proposal_content rejected and PROPOSAL.md last byte: 0x0a.

No live Gateway/runtime state was used.

clawsweeper · 2026-07-05T12:29:12Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

steipete · 2026-07-05T18:13:39Z

Thank you — this fix landed through maintainer batch PR #100399 in commit b22c36f, with contributor credit and changelog thanks preserved. Closing this source PR as superseded.

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 5, 2026

fix(skill-workshop): preserve proposal terminal newline

865212b

Preserve proposal_content exactly at the agent tool boundary and make renderProposalMarkdown defensively emit a terminal newline. Add focused regressions for the tool write path and markdown renderer.

anyech force-pushed the fix/skill-workshop-terminal-newline-source-20260705 branch from e94d56b to 865212b Compare July 5, 2026 10:58

anyech added 2 commits July 5, 2026 11:48

Merge remote-tracking branch 'origin/main' into work/pr-100293-skill-…

1123345

…workshop

fix(skill-workshop): reject blank raw proposal content

4833a98

openclaw-barnacle Bot added size: S and removed size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 5, 2026

Merge remote-tracking branch 'origin/main' into work/pr-100293-skill-…

aa09b24

…workshop

steipete self-assigned this Jul 5, 2026

This was referenced Jul 5, 2026

fix: land ten small reliability fixes #100383

Closed

fix: land ten small reliability fixes #100395

Closed

fix: land ten small reliability fixes #100399

Merged

steipete closed this Jul 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(skill-workshop): preserve proposal terminal newline#100293

fix(skill-workshop): preserve proposal terminal newline#100293
anyech wants to merge 4 commits into
openclaw:mainfrom
anyech:fix/skill-workshop-terminal-newline-source-20260705

anyech commented Jul 5, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

anyech commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

anyech commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

What Problem This Solves

Evidence

Boundary

Uh oh!

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

Uh oh!

Uh oh!

anyech commented Jul 5, 2026

Uh oh!

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

Uh oh!

Uh oh!

anyech commented Jul 5, 2026

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

anyech commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading