feat: expose quality_score on news_editor_review_signal (#604)

[biwasxyz]

Fixes #604.

agent-news #864 added an optional quality_score override to PATCH /api/signals/:id/review. The news_editor_review_signal wrapper was never updated, so the field was unreachable for editors working through MCP — agent-news#810 read as fixed while the lever stayed inert.

Change

• Add optional quality_score to the wrapper's input schema (z.number().int().min(0).max(100)), mirroring the constraint the API enforces.
• Pass it through unmodified in the PATCH payload.
• Note the auto-scorer's per-signal blind spot in the tool description.

The guard is quality_score !== undefined rather than a truthiness test, since 0 is a valid score and the neighbouring if (feedback) pattern would drop it.

Omitting the field leaves the payload byte-identical to before, so callers that don't pass it are unaffected.

Tests

tests/tools/news-editor-review-signal.test.ts asserts the payload on the wire for three cases:

Input	quality_score sent
quality_score: 15	15
quality_score: 0	0 (not dropped)
omitted	absent

The 0 row is the one carrying weight. if (quality_score) reads as consistent with the three truthiness guards directly above it, so it's a plausible future cleanup — and it would silently drop a score of 0, the #810 fabricated-sources case, turning an editor's most severe correction into a no-op.

I checked the test fails for that reason rather than assuming it would. Applying the refactor:

• the 0 case fails (expected { …(3) } to have property "quality_score")
• the 15 and omitted cases still pass — which is exactly why the trap is easy to walk into
• the pre-existing 525 tests all still pass, so nothing in the suite before this PR would have caught it

Restored the guard, and the comment on it now names the test.

Full suite: 528 passing (525 + 3), build clean. tools/list on the built server confirms the parameter as type: integer, minimum: 0, maximum: 100, and the payload matches what src/routes/signal-review.ts destructures and validates.

Note on placement: the sibling convention tests/scripts/test-news-file-signal.ts is a manual script — it needs TEST_WALLET_PASSWORD and a live endpoint, and vitest.config.ts only collects tests/**/*.test.ts, so it never runs in CI. This went in as a collected test instead, since a script wouldn't fail on the refactor above.

Not addressed

The issue also proposes a CI contract check diffing wrapper params against the agent-news route handlers' accepted fields — worth doing given this is the fourth instance of this drift, but it spans both repos and is out of scope here.

[arc0btc]

Closes the #604 gap cleanly — agent-news#864 added quality_score override to the PATCH route, but the MCP wrapper never exposed it, so the lever was inert for editors going through MCP.

What works well:

• quality_score !== undefined instead of a truthiness check is the right call, and you caught the actual failure mode instead of assuming it — the PR description shows the 0-drops-silently trap reproduced and fixed, not just asserted.
• Zod bounds (.int().min(0).max(100)) mirror the API's own validation, so a bad value fails at the tool boundary instead of round-tripping to the server.
• Test suite hits the case that matters: quality_score: 0 for the #810 fabricated-sources scenario is exactly the input a naive refactor would silently swallow. Good regression coverage against the exact bug this PR fixes.
• Tool description addition (auto-scorer's per-signal blind spot) gives editors the why, not just the what — useful for anyone calling this tool without repo context.
• Omission-preserves-payload guarantee (byte-identical when quality_score isn't passed) means zero blast radius for existing callers.

Code quality notes:
Nothing to flag — the explicit-undefined-check comment names the test that would catch a regression, which is the right amount of context for a subtle guard like this.

[question] The PR body mentions this is "the fourth instance of this drift" between agent-news route params and MCP wrapper schemas, and proposes a cross-repo CI contract check as out-of-scope follow-up. Worth filing that as a tracked issue now (even if unscheduled) so it doesn't require a fifth drift incident to resurface? Not blocking either way.

Approving — functionality, security (bounded input, no injection surface), performance (no hot-path impact), clean code, and fit all check out.