fix(contract): close the last duplications and make the .shape trap impossible (#9762) - #9765
Conversation
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 09:23:28 UTC
Review summary Nits — 7 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | fdbdbf8 | Commit Preview URL Branch Preview URL |
Jul 29 2026, 09:08 AM |
9b75fb6 to
b29225d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9765 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01%
==========================================
Files 917 917
Lines 113904 113902 -2
Branches 26966 26966
==========================================
- Hits 102894 102888 -6
Misses 9681 9681
- Partials 1329 1333 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
The merge-base changed after approval.
b29225d to
cd43888
Compare
cd43888 to
72992d9
Compare
e4934c4 to
1dc0379
Compare
…mpossible (#9762) #9515's premise is that no tool, schema, or enum here is written down twice. Every sub-issue had landed and the premise was still not true in two ways. TWENTY-SEVEN registerTool call sites passed the SDK a raw `.shape` rather than the ZodObject. The SDK accepts either and re-wraps a raw shape in a plain z.object that DISCARDS the catchall, so a looseObject input would be advertised and enforced as additionalProperties: false and every extra field the payload carries becomes a -32602 the caller cannot act on -- #9518's defect class, found the hard way. None of those inputs is loose today, which is exactly the problem: the sites were indistinguishable from the correct ones by reading, and the day one becomes loose the failure is silent and remote. A guard test now walks every server source and fails on the pure form, while leaving a composed `{ ...X.shape, extra }` alone, since that genuinely builds a new shape. Six literal enum copies of values @loopover/contract already exports now import them: the settings schema's action classes and autonomy levels, the public-surface skip reasons in both the schema and the spec's query parameter, the feasibility verdicts, and the rotatable secret names. The two in the stdio bin came with a comment justifying them -- the bin resolves @loopover/engine through the PUBLISHED package, whose export map does not surface AUTONOMY_LEVELS (#6153). True of the engine, never true of the contract, which this file already imports. The comment and the literals are gone together; a load-bearing comment that is no longer true is worse than none. AGENT_ACTION_CLASSES joins the contract as its own pinned copy, since the settings response schema needs the engine's full list and cannot import it. Pinned against the engine's, alongside new pins proving MAINTAIN_ACTION_CLASSES stays a strict subset and PROPOSE_ACTION_CLASSES is that subset plus review_state_label -- "sync it to the engine" being the obvious wrong fix for both. The regenerated openapi.json is byte-identical: every substitution has the same members in the same order, which is what makes this a substitution rather than a schema change. Not changed, having checked: the miner's three `["global", "repo", "none"]` literals are kill-switch scopes, not CONFIG_ADMIN_WRITE_SCOPES -- a different concept that shares a prefix.
1dc0379 to
fdbdbf8
Compare
Closes #9762.
#9515's premise is that no tool, schema, or enum in this repo is written down twice. Every sub-issue had landed and the premise was still not true in two ways.
The
.shapetrap — 27 sites, not the 4 the issue estimatedregisterToolaccepts either aZodObjector a raw.shape, and re-wraps a raw shape in a plainz.objectthat discards the catchall. AlooseObjectinput therefore gets advertised and enforced asadditionalProperties: false, and every field the payload carries beyond the modelled set becomes a-32602the caller cannot act on. That is #9518's defect class, found the hard way.My issue text said four sites. Writing the guard found 27 — I'd grepped a pattern that matched only some of them. All 27 now pass the schema object.
None of those inputs is loose today, so nothing was broken. That is precisely what makes it worth closing: the sites were indistinguishable from the correct ones by reading, and the day one of those inputs becomes loose the failure is silent and remote. The guard walks every server source under
src/mcp,packages/loopover-mcp/{bin,lib}, andpackages/loopover-miner/lib, so it cannot be dodged by adding a file — and it leaves a composed{ ...X.shape, extra }alone, since that genuinely builds a new shape rather than defeating one.Six literal enum copies
AGENT_ACTION_CLASSESsrc/openapi/schemas.ts(autonomy record keys)AUTONOMY_LEVELSsrc/openapi/schemas.ts, the stdio binMAINTAIN_ACTION_CLASSESPUBLIC_SURFACE_SKIP_REASONSsrc/openapi/schemas.ts,src/openapi/spec.tsFEASIBILITY_VERDICTSsrc/openapi/schemas.tsROTATABLE_SECRET_NAMESsrc/mcp/server.tsThe two in the stdio bin carried a comment justifying them: the bin resolves
@loopover/enginethrough the published package, whose export map does not surfaceAUTONOMY_LEVELS(#6153). True of the engine — never true of the contract, which that file already imports. The comment went with the literals; a load-bearing comment that is no longer true is worse than no comment.AGENT_ACTION_CLASSESjoins the contract as its own pinned copy, because the settings response schema needs the engine's full list and cannot import it. It is pinned against the engine's, alongside two new pins that state the things "just sync it to the engine" would break:MAINTAIN_ACTION_CLASSESis deliberately a strict subset (the operator-settable classes; widening it would offer switches the surface cannot honor), andPROPOSE_ACTION_CLASSESis that subset plusreview_state_label.What I checked and did not change
My scan flagged three
["global", "repo", "none"]literals inpackages/loopover-miner/lib/asCONFIG_ADMIN_WRITE_SCOPES. They are kill-switch scopes — a different concept that happens to share a two-value prefix. Left alone.Validation
expected [ …(27) ] to deeply equal []for the shape guard.openapi.jsonis byte-identical — every substitution has the same members in the same order, which is what makes this a substitution and not a schema change.npx vitest run --changed: 2336 passed, 166 files.tsc --noEmitclean;validate:mcppasses.