Auto-CR (P1) correctly flagged that the carve-out's "nothing else changed"
guard was keyed on len(otherFailures) == 0, which only observes changes the
gate already judges incompatible. Several parameter contract changes are
individually compatible and so produce no failure at all: relaxing required or
cli_required, clearing required_when, widening enum, clearing interface_type,
and clearing property through a reviewed mapping exclusion. Any of those could
have ridden along with a reviewed type migration, leaving the exemption wider
than both its documentation and what the entry actually reviewed.
Replace the failure-list heuristic with a real equality check over every
published field except Type. Comparing the struct also means a field added to
parameterSchema later is covered automatically, instead of silently widening
every existing entry. The type check moves back ahead of the field loop because
it no longer needs to observe the other findings.
Add a rejection case for each individually-compatible direction. Each case first
asserts that the drift alone really is compatible, so it keeps exercising the
equality guard instead of quietly duplicating one of the incompatible-bundle
cases.
Verified against the previous implementation: with the old guard all six new
cases fail while the nine incompatible-bundle cases still pass, which is exactly
the gap that was reported.
schema-compatibility is the third check in the same Interface Integrity job,
after the two CLI interface gates. It also rejected every published parameter
type change outright. Because the earlier gates failed first and `set -e`
stopped the step from ever running, this one never surfaced in CI, so the
previous exemption only covered two thirds of the problem.
checkParameterCompatibility now consults a precise allowlist: the tool path,
parameter name and both type values must match exactly, making it
direction-sensitive by construction, and it applies only when nothing else the
gate checks about the parameter moved (default, interface_default, format,
property, interface_type, required, cli_required, required_when, enum). The type
check moved to the end of the function so the carve-out can see those findings;
ordering is unobservable because the result is sorted.
The only entry is "minutes/minutes.apply_minutes_permission" parameter "policy"
migrating from "string" to "integer" (for #912). That type is projected from the
Cobra flag type (provenance cobra_flag_type), so it describes how the CLI accepts
a value. Consumers build a command line from it, and "--policy 4" is the same
argv under either declaration — a quoted "--policy \"4\"" still reaches pflag as
4 — while RunE keeps enforcing the same [2,4] domain. The parameter maps to
property "policyId", which the command has always sent as a number, so "integer"
is closer to the actual request than "string" was.
Table values must be the canonical form schemaType emits: the JSON encoding of
the type keyword, so `"string"` with its quotes rather than a bare string. The
guard test recomputes both through schemaType and checks the decoded name
against the closed JSON Schema type set — reviewedInterfaceRefRedirect was
silently disabled twice by exactly this class of spelling mistake.
mergedFlagContractOtherwiseChanged was only ever exercised on the path where
every condition holds still, because `||` short-circuits: with no reviewed
entry the first operand already decides the outcome and the function is never
called at all. That left its five regression branches uncovered and put
changed-code coverage at 88.0952% against a 100% target.
Add the merge-path counterpart of the checkCompatibility bundled-regression
table, pairing each of shorthand / required / hidden / no-opt / scope with the
reviewed type change and requiring the type failure to reappear. Changed-code
coverage is now 100%.
The authoritative interface baseline and command-compatibility gates
rejected every flag type change on a historical command, with no review
channel — even when the new type only moves the same validation from RunE
to flag parsing. Both now consult a precise allowlist.
An entry must match command path, flag name and both type names exactly,
so it is direction-sensitive by construction, and it applies only when
nothing else about the flag moved (shorthand, required, hidden, no-opt,
scope). A bundled regression re-reports the type change.
The first and only entry is "dws minutes permission apply --policy" moving
from string to int (for #912): the old RunE parsed with
strconv.ParseInt(v, 10, 64) and enforced [2,4], the new one lets pflag
parse with base 0 and still enforces [2,4], so the historical set of
successful invocations is a subset of the new one. Base 0 additionally
accepts spellings like "0x3", which widens rather than narrows. Defaults
are excluded from the guard because the migration necessarily changes one.
In the snapshot gate the exemption resolves against the canonical
Command.Path, never the alias-expanded accepted path: an aliased command is
compared once per accepted spelling, so keying on that would let every
alias bypass the table.
The table is duplicated because check-authoritative-interface-baselines.sh
copies the whole scripts/policy/interface-baseline directory into a
worktree checked out at a historical revision and builds it there, so that
copy cannot import a package this branch adds. A guard test fails if the
two copies drift.
The allowlist added in the previous commit keyed the reviewed
sheet.range_set_style migration by bare RPC name ("update_range"), but
interface_ref holds the canonicalized JSON that parseTool produces via
canonicalRawJSON. The lookup therefore never matched, the carve-out was
effectively disabled, and the real gate failed with
`schema tool "sheet/sheet.range_set_style" changed interface_ref`.
The existing redirect test did not catch this: it registered the fixture entry
using its own value and then asserted with the same value, so any format would
have passed. That is the same mistake as keying a probe on the author's field
spelling instead of the wire contract.
- The allowlist entry now uses the compact canonical JSON, taken from the gate's
own output rather than from pretty-printed `dws schema`.
- TestCrossPlatformCoverageReviewedRedirectKeysAreCanonicalJSON recomputes every
registered key through canonicalRawJSON, so a bare name or a pretty-printed
variant fails locally instead of only in CI.
sheet export keeps its reviewed mcp + submit_export_job declaration. The comment
now records the known trade-off explicitly: --export-format csv is a mutually
exclusive branch that reads get_range_as_csv and never invokes the declared
submit_export_job, so this declaration does not cover the csv branch's backing
interface. Attributing that branch is left out of scope for this change.
Verified against the real gate, not just unit tests: all four Interface
Integrity checks pass (authoritative-interface-integrity,
check-command-compatibility, schema-compatibility, skill-command-integrity).
Two review findings, plus a same-class defect found by sweeping for it.
1. compatibleInterfaceRefRedirect accepted any mcp tool repointing from any
non-empty interface_ref to any other, as long as no other check for that tool
failed. Schema shape cannot prove two RPCs share business semantics,
permissions, error behaviour, or side effects, so that would have let every
future backend swap bypass the gate it exists to enforce. It is now keyed on
an explicit reviewedInterfaceRefRedirect allowlist of exact tool + old→new
pairs, currently holding only the reviewed
sheet.range_set_style: update_range → set_cell_range migration. Every other
ref change is reported again.
2. sheet export --export-format csv only printed a stderr warning when
get_range_as_csv returned hasMore=true, then wrote --output and reported
success with exit code 0. Automated callers, and anyone not watching stderr,
would treat an incomplete file as a complete export, and an existing target
file was overwritten with truncated data. Truncation now fails before the
write (leaving any existing file untouched) unless --allow-truncated is
passed; with the opt-in the success line states the data is incomplete.
--allow-truncated is registered in the reviewed mapping ledger as a local
policy input.
3. Swept for the same classes and found sheet_dimension.go repeating the
fmt.Sscanf("%d") prefix-parse hole in three places: insert_dimension,
delete_dimension, and update_dimension all accepted --length "3x" as 3, so a
malformed value silently operated on the wrong row/column count — the delete
direction is not rollbackable. All three now use strconv.Atoi. (Checked and
cleared: sheet csv-get also surfaces hasMore, but it has no --output and only
returns the flag in its JSON payload, so it is not the same fail-open shape.)
Tests: allowlist rejection cases (unreviewed target ref, and the same pair on a
tool absent from the allowlist, asserted outside the table so the registration
survives until checkCompatibility runs); truncation fail-closed with a
pre-existing output file asserted byte-for-byte unchanged; --allow-truncated
write-through; and an untruncated read needing no opt-in. Changed-code coverage
stays at 100% (939 statements).
Two compatibility carve-outs, written alongside the existing interface_type
retirement allowance. Both cover declarative provenance metadata that nothing
reads at runtime: the tool a leaf invokes is decided in the CLI source, so a
stale interface_ref or property misinforms a reader rather than misrouting a
call. Neither carve-out can mask a change to the surface callers depend on.
Cleared property through a reviewed mapping exclusion. A leaf whose backing RPC
moves to a nested payload has no honest flat property to publish. The two
alternatives are worse: keep naming a field the request no longer contains, or
let assembly fall back to flag_name_inference and publish a name that appears in
no request at all. Accepted only when the old value was non-empty, the new value
is empty, and the new value resolved through reviewed_mapping_exclusion. A
redirect to a different non-empty value, a clearing by inference or native
annotation, a clearing with no recorded source, and populating a previously
empty property all stay incompatible. The exclusion table cannot be abused to
wave arbitrary clearing through: internal/cli/schema_parameter_bindings.go
verifies every parameter claiming an exclusion really does deliver an empty
property, and every entry carries a non-empty reviewed reason.
Redirected interface_ref with an unchanged CLI contract. Accepted only when
interface_mode is unchanged and stays mcp, both refs are non-empty, and no other
compatibility failure was recorded for that tool. That last condition is the
operative definition of "the contract is unchanged" — it is measured, not
asserted, so it automatically covers a lost parameter, a newly required one, a
moved type / default / format / enum, a tightened constraint, a positional or
dry_run change, and any effect / risk / confirmation / idempotency move. Any one
of them re-reports the redirect, so a surface change cannot ride along behind a
backend move. Moving to or from composite is a change in kind rather than a
redirect and stays reported; so does removing a ref outright.
Deliberately still incompatible: mcp -> composite with the ref dropped. That is
a leaf declaring it now orchestrates several RPCs, which is a semantic upgrade
rather than a like-for-like substitution, and it belongs in review.
For this branch the two carve-outs take schema-compatibility from 17 changed
fields to 4 — the twelve sheet.range_set_style property clearings and its
update_range -> set_cell_range redirect are now accepted. The remaining four are
the interface_mode plus interface_ref pairs on sheet.create_workspace_sheet and
sheet.submit_export_job.
Tests: TestCrossPlatformCoverageSchemaCompatPropertyClearingExclusion and
TestCrossPlatformCoverageSchemaCompatInterfaceRefRedirect assert the accepted
shapes plus twelve neighbouring shapes that must stay incompatible; the drift
table gains cases for clearing without an exclusion and redirecting despite one.
Keep schema-compat tests aligned with main after removing the ding
property-correction allowlist; the hard-fail case is already covered elsewhere.
Co-authored-by: Cursor <cursoragent@cursor.com>
Hit the nil-allowlist early return so the platform changed-statement gate
reaches 100% after shared-file overall baseline filtering.
Co-authored-by: Cursor <cursoragent@cursor.com>
Deleting 100%-covered packages such as recovery falsely regressed overall
percent against merge-base. Baseline overall now uses only files still
present in the candidate profile, with a 0.1pp CI tolerance. Also exercise
stripSchemaValueCompact nested map/slice branches.
Co-authored-by: Cursor <cursoragent@cursor.com>
Cover each equals-form flag in its own switch case so the platform
changed-statement coverage gate reliably hits both branches.
Co-authored-by: Cursor <cursoragent@cursor.com>
Complete ding leaf Property/Required from Execute CallMCP keys and live
help semantics, and allowlist the four inference→declare property remaps
so schema-compat does not freeze wrong camelCase flag names.
Co-authored-by: Cursor <cursoragent@cursor.com>
Host hrbrain, markdown, pat, and profile under dingtalk-misc, retire
their standalone packages, and rename dws-shared to dingtalk-shared
across live paths, coverage, installers, and policy.
Co-authored-by: Cursor <cursoragent@cursor.com>
Point coverage, installers, IM skill-chain, and shared routing at
dingtalk-misc references after retiring the standalone event package.
Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the recovery package/commands and related Schema/skill teaching so agents
stop being steered at a dead surface, while keeping mono↔multi content QA work.
Co-authored-by: Cursor <cursoragent@cursor.com>
embeddedMCPMetadata only feeds interface validation now; its summary was
projected onto every schema payload as a constant-empty blob. Remove the
SchemaRegistry field, all three payload projections, the snapshot wire
field, the overview copy, the compact strip entry, and the jq policy gate.
Also delete the redundant io.Discard dead-code suppressor in
event_command.go (io has five genuine uses there).
- Add testseam.Protect for save-and-restore seams with no up-front stub
value (e.g. os.Args mutated by the code under test).
- Migrate all 35 remaining manual prev/assign/t.Cleanup-restore trios to
testseam.Swap/Protect across app, auth, cli, event, helpers, output,
pipeline, and shortcut tests; restores can no longer be forgotten.
- check-schema-catalog.sh: fail closed when a manual seam restore
reappears in any *_test.go (internal/testseam exempt).
Seam injection is now a mechanism, not a convention.
- schema_cobra_binding: the Identity-vs-spec check is now a defensive
self-consistency assertion (the spec is collected from the same
ContractFinal.Identity), not a cross-source pin; name the real drift
anchors (native annotation cross-check, collector uniqueness
self-validation, homology tool-count tripwire, surface/catalog hash
baselines) and relabel the mismatch diagnostic as collected vs declared.
- schema-compat: state explicitly that accepting interface_type clearing
is a deliberate wire-visible policy decision taken with the MCP pin
retirement (missing = unknown; re-population requires ParamDecl).
- schema_command_registry: drop retired bindings audit / MCP pin from the
peer reviewed-inputs comment; note they must not reappear.
- canonical: schema help no longer claims commands must enter a reviewed
registry; identity is collected from ContractFinal.Identity.
- homology: the tool-count tripwire error now says where to bump it after
review.
- Sweep stale 'reviewed registry' wording in corecmd and the help-flag
completeness gate comment; AGENTS.md interface-facts section matches.
Close each owned os.Stdin file when replacing it in the stdin coverage
matrix so Windows TempDir cleanup does not fail on leaked handles. Update
the command registry coverage test for contract_identity source and drop
the retired loadPinnedMCPMetadata loader reference gate from policy.
Co-authored-by: Cursor <cursoragent@cursor.com>
Allow clearing interface_type and expanding constraint group members so
MCP-pin retirement and declare≡execute alias groups stay backward-compatible.
Close platform coverage gaps with TestCrossPlatformCoverage* and bump the
ContractFinal consistency count to 847.
Co-authored-by: Cursor <cursoragent@cursor.com>
Schema Catalog now assembles from Contract/ParamDecl/Interface and Cobra only.
Keep fetch-mcp-metadata as an optional diagnostic dump and ban the retired pin path.
Co-authored-by: Cursor <cursoragent@cursor.com>
- Relocate the registry-agnostic side-guards from the deleted
check-schema-command-registry.sh into check-schema-catalog.sh:
legacy hint/visibility source ban, go:generate single-track checks,
agent-metadata embed/loader bans, lazy-loader reference-count checks,
package-scope eager-initializer ban, internal/app loader ban, and the
two fresh-process laziness tests (TestRuntimeSchemaMetadataLoadsOnlyOnDemand,
TestOrdinaryRootCommandsDoNotLoadSchemaMetadata). Drop the guards that only
protected the retired reviewed registry (JSON Schema/product shard presence,
registry-overwrite go:generate ban, registry-count test runs); the native
materialization ban already lived in check-schema-catalog.sh.
- Rename the wire-visible CommandSpec.Source label from
"reviewed_command_registry" to "contract_identity" (new exported
constant CommandSourceContractIdentity): identity is collected from
ContractFinal.Identity declarations, the registry is gone. Source is a
provenance label excluded from the identity SourceHash (surface hash is
unchanged); the catalog content hash shifts with the delivered bytes as
expected. Updated every assignment and every test pin consistently.
- Update docs/schema-dynamic-endpoint-design.md,
docs/rfc-command-framework-convergence.md and
docs/flag-help-schema-homology.md: collector is the single identity
source, reviewed registry retired; keep genuine historical context.
炳昱