Skip to content

fix(agent-core): preserve empty prompt arguments#96405

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
lin-hongkuan:codex/prompt-template-empty-args
Jun 25, 2026
Merged

fix(agent-core): preserve empty prompt arguments#96405
vincentkoc merged 1 commit into
openclaw:mainfrom
lin-hongkuan:codex/prompt-template-empty-args

Conversation

@lin-hongkuan

@lin-hongkuan lin-hongkuan commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Prompt template command arguments silently dropped quoted empty values ("" / ''). That shifted positional placeholders after the empty argument, so a template using $1, $2, $3 could substitute the third shell token into $2 instead of preserving the intended blank value.

Why This Change Was Made

The parser previously used current as both the accumulated value and the token-presence flag. Empty quoted strings are valid arguments, but their accumulated value is intentionally empty, so they were indistinguishable from whitespace-only gaps.

This patch tracks whether the parser is currently inside a real token separately from the token text, preserving empty quoted arguments while leaving whitespace collapsing unchanged.

User Impact

Prompt templates can now intentionally pass blank positional arguments without corrupting later argument positions.

Evidence

Real behavior proof added in packages/agent-core/src/harness/prompt-template-arguments.test.ts:

expect(parseCommandArgs('first "" third')).toEqual(["first", "", "third"]);
expect(parseCommandArgs("first '' third")).toEqual(["first", "", "third"]);
expect(substituteArgs("$1|$2|$3", parseCommandArgs('first "" third'))).toBe("first||third");

Validation run from this branch:

RUN  v4.1.8 C:/Users/lin20/Documents/New project 3/openclaw

Test Files  1 passed (1)
     Tests  1 passed (1)
  Start at  20:29:05
  Duration  876ms (transform 591ms, setup 481ms, import 11ms, tests 121ms, environment 0ms)

Checking formatting...

All matched files use the correct format.
Finished in 1155ms on 2 files using 32 threads.

git diff --check produced no output.

AI-assisted: OpenAI Codex helped identify the parser edge case and prepare the focused regression test.

Follow-up Evidence After Review

The review asked for stronger real behavior proof beyond the regression assertions. I ran the parser/substitution helpers directly from this branch:

{"input":"first \"\" third","args":["first","","third"],"rendered":"first||third"}
{"input":"first '' third","args":["first","","third"],"rendered":"first||third"}

This shows both quoted-empty forms preserve the blank second positional argument, so $3 remains aligned with third instead of shifting into $2.

@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 4:23 AM ET / 08:23 UTC.

Summary
The branch tracks token presence separately in prompt-template argument parsing and adds regression coverage for quoted empty arguments.

PR surface: Source +5, Tests +11. Total +16 across 2 files.

Reproducibility: yes. at source level: current main and v2026.6.10 only push non-empty current, so first "" third drops the blank second argument. I did not run tests in this read-only review, but the PR body includes after-fix live output for the changed helper.

Review metrics: 1 noteworthy metric.

  • Exported Parser Surface: 1 exported parser changed. parseCommandArgs is exported by agent-core and consumed by session prompt-template expansion, so tokenization compatibility needs maintainer acceptance.

Root-cause cluster
Relationship: canonical
Canonical: #96405
Summary: This PR is the canonical open item for preserving quoted empty prompt-template arguments; the apostrophe parser PR overlaps the same parser but does not currently carry the empty-token invariant.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

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.

Risk before merge

  • [P1] The parser is exported from agent-core, so preserving "" and '' intentionally changes shipped tokenization for callers that previously observed blank quoted arguments being dropped.
  • [P1] The overlapping apostrophe parser PR at fix(agents): keep apostrophes in slash-command prompt-template arguments #90468 rewrites the same helper and must preserve this empty-token invariant if both changes land.

Maintainer options:

  1. Land With Compatibility Acceptance (recommended)
    Accept the deliberate tokenization change for quoted empty arguments and require any later parser rewrite to keep the new empty-token regression test green.
  2. Fold Into The Apostrophe Parser Work
    Pause this PR only if fix(agents): keep apostrophes in slash-command prompt-template arguments #90468 is updated to include the same empty-token behavior and test before landing.

Next step before merge

  • No automated repair remains; maintainers should decide whether to land this parser compatibility change directly or fold the invariant into related parser work.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes only a local TypeScript parser and a colocated test.

Review details

Best possible solution:

Land one shared parser path that preserves quoted empty tokens; if the apostrophe parser PR also lands, it should carry this invariant and regression coverage forward.

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

Yes, at source level: current main and v2026.6.10 only push non-empty current, so first "" third drops the blank second argument. I did not run tests in this read-only review, but the PR body includes after-fix live output for the changed helper.

Is this the best way to solve the issue?

Yes. Separating token presence from accumulated text is the narrow shared-parser fix; the main alternative is to fold the same invariant into #90468 if maintainers prefer one parser grammar change.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This fixes a user-visible prompt-template argument corruption bug with limited blast radius.
  • merge-risk: 🚨 compatibility: The diff intentionally changes exported parser tokenization by preserving blank quoted arguments that current releases drop.
  • 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 (live_output): The PR body contains after-fix copied live output for both quoted-empty forms showing parsed args and rendered substitution preserve the blank second slot.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains after-fix copied live output for both quoted-empty forms showing parsed args and rendered substitution preserve the blank second slot.
Evidence reviewed

PR surface:

Source +5, Tests +11. Total +16 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 7 2 +5
Tests 1 11 0 +11
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 18 2 +16

What I checked:

Likely related people:

  • vincentkoc: GitHub commit history shows this handle authored the shared prompt-template argument extraction and a later agent-core harness cleanup touching the same parser and tests. (role: introduced shared parser and recent area contributor; confidence: high; commits: deb48a96fb8d, 464adfe5e544; files: packages/agent-core/src/harness/prompt-template-arguments.ts, packages/agent-core/src/harness/prompt-templates.test.ts, src/agents/sessions/prompt-templates.ts)
  • steipete: GitHub file history shows prior prompt-template safety work around unsafe template indexes and nearby session prompt-template maintenance. (role: adjacent prompt-template safety contributor; confidence: medium; commits: 8ac0c3546291, f84460e62534; files: src/agents/sessions/prompt-templates.ts, packages/agent-core/src/harness/prompt-templates.test.ts)
  • yetval: This handle owns the open overlapping parser PR that rewrites the same helper, so they are relevant for coordinating parser invariants even though that work is not merged. (role: adjacent parser PR contributor; confidence: medium; commits: 9b0ff9836ee4; files: packages/agent-core/src/harness/prompt-template-arguments.ts, packages/agent-core/src/harness/prompt-templates.test.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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 24, 2026
@vincentkoc vincentkoc self-assigned this Jun 25, 2026
@vincentkoc
vincentkoc force-pushed the codex/prompt-template-empty-args branch from 0202948 to 37840d4 Compare June 25, 2026 08:14
@vincentkoc
vincentkoc merged commit c030b30 into openclaw:main Jun 25, 2026
77 checks passed
@vincentkoc

vincentkoc commented Jun 25, 2026

Copy link
Copy Markdown
Member

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jun 25, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Closed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 26, 2026
@lin-hongkuan
fix(agent-core): preserve empty prompt arguments (openclaw#96405)
ab394d5
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@lin-hongkuan @QiuYuang
fix(agent-core): preserve empty prompt arguments (openclaw#96405)
01cb000
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lin-hongkuan @vincentkoc