Skip to content

fix(mcp): validate_expression refuses a blank expression with the save gate's verdict - #27534

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-27411-mcp-validate-blank
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-27411-mcp-validate-blank

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #27411

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:MCP 工具 validate_expression 收到空串或纯空白表达式时,不再回 ok: true,改回 ok: false,错误文案与保存闸门(422 INVALID_METADATA)同一句。
  • 为什么改:agent 先用这个工具校验、再保存;以前工具说"没问题",保存却被拒,两边口径不一致。
  • 风险与回滚:只收窄了工具对"空白表达式"的判定,非空表达式的结果不变;回滚即 revert 本 PR(一个文件的工具逻辑加一个测试文件)。
  • 先看:packages/mcp/src/mcp-http-tools.ts 中 validate_expression 处理函数里的 slot.safeParse(expression) 分支;其次 VALIDATE_SITE_MAP 新增的 slot 列。
  • 两分钟验证:pnpm --filter @objectstack/mcp exec vitest run src/mcp-validate-expression.test.ts,预期 22 个用例全部通过。
  • 需要你做的:无,席位落地。

What changes

validate_expression now parses the expression with the same spec input schema the save/build gate parses that slot with, before it runs the formula checks:

site schema (from @objectstack/spec/shared) refusal sentence
formula, validation, flow_condition EvaluatedExpressionInputSchema EVALUATED_EXPRESSION_SOURCE_REQUIRED
template TemplateExpressionInputSchema TYPED_EXPRESSION_SOURCE_REQUIRED.template

When the schema refuses, the tool answers ok: false and each errors[] entry is that schema's own issue: code is the issue code (invalid_union today, the shape the spec docblock documents for a blank bare string at an evaluated slot) and message is the published sentence. No blank rule and no sentence is written in packages/mcp. warnings is empty, inferredType is omitted, and inScope still comes back so the agent can correct itself. When the schema accepts, the tool runs validateExpression exactly as before.

What was narrowed, and why it is the published rule. Before this change an empty or whitespace-only expression ('', ' ', a tab or newline mix) read ok: true at every site, because validateExpression treats a blank source as "not authored" and returns ok. Saving the same expression is refused: the formula expression, validation and flow-edge conditions are declared with EvaluatedExpressionInputSchema, which refuses a source that is blank after trimming (packages/spec/src/shared/expression.zod.ts), and template slots with TemplateExpressionInputSchema, which refuses a blank template. The tool now gives that verdict. Nothing else is narrowed: every non-blank string passes both schemas, so its answer is byte-for-byte the old one.

Tests

packages/mcp/src/mcp-validate-expression.test.ts adds 14 cases:

  • '', ' ' and a tab and newline mix, at formula, validation and flow_condition: ok: false, one error, code invalid_union, message equal to the imported EVALUATED_EXPRESSION_SOURCE_REQUIRED, no warnings, no inferredType, inScope still present.
  • The same three inputs at template: message equal to TYPED_EXPRESSION_SOURCE_REQUIRED.template.
  • No site given defaults to formula and refuses.
  • Controls: ' record.amount / 100 ' (formula) and 'Hi {{ record.title }}' (template) still answer ok: true with no errors.

Acceptance notes

  • The error code is the spec schema's zod issue code (invalid_union), not a kebab code from @objectstack/formula's refusal vocabulary. That vocabulary is closed and owned by packages/formula, and its contract is one code per message template, so reusing empty-expression (whose message is different) would break it. Adding a new formula code is outside this card's file surface. If a stable kebab code is wanted for this refusal, that is a formula-package change.
  • The template site uses the general TemplateExpressionInputSchema sentence. A few template slots (the notify node's title and message) refuse with their own sentence that names the key. The tool has no key, so it cannot pick between them; the general sentence is the one the template site's default slots use.
  • PR assignee: the dispatch asked for assignee sunhaolin on pr_create, but the relay's own row comment (scripts/pm/fleet-write/ops.mjs, pr_create) says a dev's create carries no assignees. This PR was opened without one. The seat decides.

Verification

All readings below were taken at head bb5eb6dd (the final commit), except the premise row, which was taken on origin/main before the edit.

Premise, measured before the edit on the built packages (origin/main ded67bf)

validateExpression('value', '', hint) and validateExpression('value', ' ', hint) both answered {"ok":true,"errors":[],"warnings":[]}. EvaluatedExpressionInputSchema.safeParse('') answered one issue, code invalid_union, message EVALUATED_EXPRESSION_SOURCE_REQUIRED. TemplateExpressionInputSchema.safeParse('') answered one issue, code invalid_union, message TYPED_EXPRESSION_SOURCE_REQUIRED.template. 'record.amount' passed both.

Ablation (one run, on the committed fix)

node scripts/ablation-replace.mjs replaced const slotParse = slot.safeParse(expression); with a constant { success: true }: anchor 1 to 0, blob 0015f6b5 to 1609783f. src/mcp-validate-expression.test.ts: 13 failed, 9 passed (22), meaning the 12 blank cases plus the default-site case failed and both controls passed. Restored: blob equals HEAD 0015f6b5, git diff HEAD empty. The test imports the tool from src through a relative path, so no dist rebuild was part of the path.

Package test and typecheck

pnpm --filter @objectstack/mcp test: Test Files 45 passed (45), Tests 581 passed (581). pnpm --filter @objectstack/mcp typecheck: tsc --noEmit clean, and check:test-typecheck OK (6 files, 53 errors, 8 pinned signatures held in the ledger, unchanged). Lock VERDICT command-exit 0.

Derived gate families

node scripts/pm/dispatch-gates.mjs --commands (change set of 3 paths against merge base ded67bf) named 59 families; all 59 ran and exited 0, and --ran reconciled 59 derived, 59 run, 0 NOT-MEASURED. Three needed a prerequisite first and were re-run after it: check:lean-entry-closure (exit 0 after building @objectstack/objectql) check-plugin-teardown-shape --self-test (exit 0 after fetching its pinned fixture commit on this shallow clone), and check:dual-build-cjs-loads (exit 0 once check:type-check-debt had built every package: 112 require entry points across 69 packages load).


Generated by Claude Code

…e gate's verdict

The tool parses the expression with the spec input schema the save/build
gate uses for the slot before running the formula checks, so a blank or
whitespace-only source answers ok:false with the gate's own issue code and
published sentence instead of ok:true.

Claude-Session: https://claude.ai/code/session_017xRBcScm27Q3R5mkYLPWdB
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Oct 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 58 of 210 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 57c51aab4d2f63293676e9412daed8a5fc54c1bc → packageMentionDocs.

Which tree this was computed on

This run read content/docs from d7bc3f0c4d5575c7dfb6e099d414596c37b665e4 — the merge of head bb5eb6dd02918a08862500c787d71f6f3f797256 into base 57c51aab4d2f63293676e9412daed8a5fc54c1bc, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d7bc3f0c4d5575c7dfb6e099d414596c37b665e4 && git checkout d7bc3f0c4d5575c7dfb6e099d414596c37b665e4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 57c51aab4d2f63293676e9412daed8a5fc54c1bc bb5eb6dd02918a08862500c787d71f6f3f797256 && git checkout -B drift-repro 57c51aab4d2f63293676e9412daed8a5fc54c1bc && git merge --no-ff bb5eb6dd02918a08862500c787d71f6f3f797256

node scripts/docs-audit/affected-docs.mjs --json 57c51aab4d2f63293676e9412daed8a5fc54c1bc

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcScm27Q3R5mkYLPWdB

席位核对

Verdict: ACCEPT at head bb5eb6dd02918a08862500c787d71f6f3f797256, reviewed 2026-10-29T11:28Z. Landing waits for CI on this head to finish green.

check reading
Shape Draft, base main. The first line is Fixes #27411, which is correct because the card is fully delivered. No other closing keyword appears in the body.
Path surface 3 files: packages/mcp/src/mcp-http-tools.ts, its test, one changeset. +100/-8 lines. No governed path is touched.
Contract faces None hit. No packages/spec/src/**, no governed text, and no accept set narrowed: the tool's input schema still takes any string for expression, and only its verdict for a blank source changes, to the published save-gate rule. So no contract-tier review is owed. This corrects the note on the claim (6494155245), which said one was owed.
Clause-② no, correct.
Changeset @objectstack/mcp patch; each sentence matches the diff.
Diff read VALIDATE_SITE_MAP gains the spec input schema per site, and the handler parses with it before validateExpression. A refusal returns the schema's own issue (code and published sentence); no second blank rule is written in packages/mcp. inScope is still returned, and inferredType is omitted on refusal.
Assertions 12 blank cases across all four sites assert ok: false, one error, code, and message equal to the imported spec constant. The default-site case is covered, and two non-blank controls stay ok: true. The tool returns a verdict rather than refusing a request, so there is no HTTP status to assert.
Dev evidence At bb5eb6dd: mcp package 581 passed; typecheck clean. The ablation turns 13 cases red with both controls green. dispatch-gates --ran: 59 derived, 59 run. mcp_calls 0.
CI on head At review time, 13 success, 14 skipped and 18 in progress, none red. Read again before landing.

Open question from the dev (PR assignee): option A. The PR stays without an assignee, as the relay pr_create row records for a dev's draft; the card carries the assignee.

Accepted deviations: the error code is the schema's zod code (invalid_union), not a formula kebab code. That is right, because the formula vocabulary is closed and outside this surface.


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcScm27Q3R5mkYLPWdB

Ready to land at 2026-10-29T11:41Z, held. CI on head bb5eb6dd is complete: 48 check-runs, 34 success, 14 skipped. Lint & Repo Gates and TypeScript Type Check are both success. check-expected-skips --pr 27534 is OK (all 14 skips are in main's roster). check-governed-merges --pr 27534: 0 of 3 paths governed, 108 lines.

The landing step (relay pr_ready + automerge_enable) is the same command this session's permission classifier refused on #27424 (comment 6494048067), so it is not retried here. Needed from the maintainer: allow that command for this seat, or mark this PR ready and enable auto-merge by hand.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 29, 2026 16:39
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 29, 2026 16:39
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit e5c5035 Oct 29, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-27411-mcp-validate-blank branch October 29, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mcp: validate_expression answers ok:true for an empty or blank expression that the metadata save gate refuses as needing a non-blank source

1 participant