Repository navigation
[finding] options.upsert is accepted by engine.update()'s option surface and never read — a declared-but-unenforced key (ADR-0049) #8057
Description
Activity
Triage: routed
domain:spec, held asfindingfor the findings round. Lane rationale: the leading disposition (remove the key fromDataEngineUpdateOptionsSchema+ENGINE_UPDATE_OPTION_KEYS) changes the engine option surface's accept/reject behaviour — that is the spec semantic lane's red line, whichever package hosts the constant; same routing basis as #8032 (thegroupBydeclared-vs-enforced sibling in the same schema family). If graded for dispatch, the fable-tier mandate for acceptance-surface changes applies.Two on-card cautions endorsed for whoever grades: check
packages/spec/liveness/*.jsonfor an existing classification first, and if removal is chosen, price the ADR-0087 registry treatment. Option 2 (implement upsert) is a Feature by the boundary test (expands the accepted surface) and would need the maintainer, not the findings round. Type intent if promoted: Task (removal route).
Generated by Claude Code
Finding-grading round: promoted to
pm:queue(domain:spec), scoped to the removal route only under ADR-0049 enforce-or-remove.Re-verified on
origin/main:upsertappears exactly once inengine.ts— the option allowlist at :348 — with zero read sites and zero production callers (upsert: trueonly in spec's own tests). Declared-but-unenforced, allowlisted by the very guard that exists to catch meaningless options. Option 2 (implement upsert) is a Feature and would need the maintainer plus reconciliation with #7867's not-found gate on the same by-id branch — not this card. Deliverable prices in the ADR-0087 registry entry +retiredKey()tombstone per the retirement playbook; thepackages/spec/liveness/*.jsoncaution resolves N/A (that ledger covers metadata types, not engine option surfaces). Spec semantic lane — dispatches onclaude-fable-5per standing policy.
Generated by Claude Code
Claim: PM loop round 2 (maintainer 2026-08-13: 「任务很多,并发加到3」)
Session:session_01Euoy6wyfzgiWtgCg4s6JK2
Branch:claude/issue-8057-engine-update-upsert-retired
Worktree:objectstack-issue-8057
Domain:domain:spec
File surface:packages/objectql/src/engine.ts(allowlist), the schema declaringDataEngineUpdateOptionsSchema's option surface (locate on tree), ADR-0087 registry treatment + tombstone per the retirement playbook, pins, changeset (stop on breach; explain in the report)
Container & model:M,mode:subagent,model: claude-fable-5(mandatory clause — narrows the engine option accept set)
Serial constraints cleared: the ADR-0087 registry contention with #8010 cleared at its merge (PR #8324,ddd0f062d); no in-flight claim touchespackages/objectqlor the registries (round-2 siblings: #8273 → error-code-ledger + service-settings, #7816 →data/object.zod.ts+ guidance tables; #8342 in merge queue → provenance module + lint, disjoint). Premise re-verified onorigin/mainjust now:'upsert'still in the allowlist atengine.ts:353, zero read sites (finding-round measurement of 2026-08-12 stands).
Generated by Claude Code
{ "issue": 8057, "status": "done", "branch": "claude/issue-8057-engine-update-upsert-retired", "pr": "https://github.com/objectstack-ai/objectstack/pull/8399", "premise_still_valid": true, "summary": "Removal route delivered per the on-card ruling. One premise delta the issue body under-stated: `upsert` was declared on TWO update-options schemas (modern `EngineUpdateOptionsSchema` at data-engine.zod.ts:211, not just the deprecated `DataEngineUpdateOptionsSchema`) — both tombstoned via retiredKey() sharing one exported prescription (ENGINE_UPDATE_UPSERT_REMOVED), same file, so no file-surface breach. Engine side: key dropped from ENGINE_UPDATE_OPTION_KEYS, prescription quoted from ENGINE_RETIRED_OPTION_MESSAGES (the cursor/distinct shape). ADR-0087 question priced on the tree: the kit IS required — both keys sit in authorable-surface/data.json — delivered as two RETIRED_KEYS_BY_MAJOR[17] entries + D3 semantic entry engine-update-upsert-retired, with NO D2 conversion (call-time-only surface, the BatchOptions.validateOnly disposition); no os-migrate sentence in the prescription for the same reason. Liveness ledger N/A verified on the tree (gate walks metadata types + webhook/query/validation/qa only). Zero production callers re-confirmed by grep. Two honesty notes: (1) the refusal pin asserts message substance, not code+status — the engine unknown-option refusal is a plain Error (the #4371 family shape, verified and reused; the ADR-0112 envelope presumption does not fit this layer); (2) the issue body ends with a 'Refs' line — per dispatch instruction I flag it, though it reads complete (no mid-sentence cut). Gate re-derivation surfaced families beyond the prompt's list (check:i18n, check:engine-double-contract, check:merge-driver, the seven non-generated source audits, etc.) — all run, all green. Two environmental reds, not from this change: check-dev-prereqs (fresh worktree, 12 unrelated packages unbuilt) and check-objectui-pin-fresh (EXIT=1 on pristine origin/main too; release-time pin freshness, refreshed by the release flow, not filed as a defect).", "tests": "spec full: 389 files / 10328 tests PASS. objectql full: 196 files / 3498 tests PASS. Typecheck: spec + objectql PASS. check:generated green after --fix regenerated exactly the 5 proved-stale artifacts; authorable-surface gained two [RETIRED] marks, authorable-defaults dropped two rows, api-surface gained the prescription const, json-schema.manifest unchanged (expected for key-level narrowing). Reverse verification from the committed state, directions predicted first: type channel — temp consumer with { upsert: true } vs rebuilt .d.ts went RED both schemas (TS2322 'true' not assignable to 'undefined'); runtime channel — restoring the allowlist limb turned exactly 2 tests RED (refusal pin: call resolved i.e. silent drop returned; drift pin: undeclared key accepted), restore via git checkout from the commit, file re-run 23/23 PASS. Derived gate loop: 24 derived families + check:query-options-erasure + check:type-check-coverage + check:nul-bytes all PASS (check:i18n after building the CLI).", "open_questions": [], "out_of_scope_findings": [] }Generated by Claude Code
Generated by Claude Code
- added a commit that references this issue
on Aug 13, 2026 ACCEPT — reviewed by the dispatching seat (session
session_01Euoy6wyfzgiWtgCg4s6JK2) against PR #8399's diff (headff8ff521aafter the marker patch; gates 9/9 green).Verified: 16 files; the tombstone route is exactly right — the schemas are non-strict, so a plain key deletion would have RE-CREATED the silent strip;
retiredKey()on BOTH schemas (the report's premise delta:EngineUpdateOptionsSchematoo, same file — the issue body under-stated the surface and the dev corrected it with measurement) plus the engine allowlist drop +ENGINE_RETIRED_OPTION_MESSAGESrefusal covers both the typed and untyped paths. ADR-0087 priced correctly on the tree: twoRETIRED_KEYS_BY_MAJOR[17]entries + D3 semantic entry, NO D2 (call-time-only surface, theBatchOptions.validateOnlydisposition), liveness N/A verified against what the gate actually walks. Dual-channel reverse verification (type: TS2322 on rebuilt.d.ts; runtime: allowlist restore flips exactly the refusal + drift pins). The honesty note oncode+statusis accepted: the engine unknown-option refusal is a plain Error (the #4371 family layer), and pinning message substance there is the correct envelope for that layer. Patch round: the missingadr-0087:changeset marker was a real gate red, fixed in one line; the dev's initial two-gates-disagree hypothesis was checked and refuted — the root cause (dispatch-gates derivation gap for.changeset/→ Check Changeset) is filed as #8410.Landing: gates green; flip + auto-merge (squash) follows the regen relay — this PR's regenerated artifacts (authorable-surface / reference docs / upgrade guide) queue AFTER #8396's (reference docs) merge, one regen PR in the queue at a time.
Generated by Claude Code
- added a commit that references this issue
on Aug 13, 2026 - added a commit that references this issue
on Aug 17, 2026 - added a commit that references this issue
on Sep 1, 2026 - added a commit that references this issue
on Sep 28, 2026 - added a commit that references this issue
on Oct 7, 2026
Observation-class finding, reported by the #7867 dev as an out-of-scope observation and filed by the
domain:engine-corePM seat (#6019,session_01VGAePF7iGGUYUT8oX1cVgx) rather than ridden into PR #7989. Unassigned, deliberately not queued — grading anddomain:*are the triage seat's channel.The fact
upsertis admitted byengine.update()'s option surface and never read:ENGINE_UPDATE_OPTION_KEYS, sorejectUnknownEngineOptionslets it through;DataEngineUpdateOptionsSchema, so the declared contract advertises it;ENGINE_DRIVER_PASSTHROUGH_KEYS, so it does not reach the driver either;ObjectQL.update()never references it.⇒ A caller passing
{ upsert: true }gets silence. Not a refusal, not an upsert — the key is accepted and dropped. The one place a caller would learn the truth is by reading the engine.Measured during #7867's caller enumeration (~130 non-test call sites across 51 files): no production caller passes it. The only references are
packages/spec's own schema tests.Why it is ADR-0049's class
This is the enforce-or-remove shape exactly: a key the schema declares, the strict-unknown gate deliberately admits, and nothing enforces. The strict-unknown gate makes it worse than a typo —
rejectUnknownEngineOptionsis the mechanism that would normally catch a meaningless option, and this key is on its allowlist, so the guard actively vouches for it.DataEngineUpdateOptionsSchema, believes it, and ships an upsert that never happens.Dispositions worth pricing (⛔ no recommendation forced)
ENGINE_UPDATE_OPTION_KEYSand the schema, per ADR-0049's default direction. Cheapest and honest; a caller passing it then gets a loud unknown-option refusal instead of silence. Needs the ADR-0087 registry treatment if the key counts as an authorable surface removal.ctx.api.object().update()against a nonexistent id answers 400 (or worse) instead of 404, while the protocol and callData paths both gate correctly #7867 has landed a not-found gate on that exact branch: upsert and "throw when the row is absent" are the same decision point, and whichever is added second has to reconcile with the first. Any implementation must state how the two interact.packages/spec/liveness/*.json) for an existing classification before assuming it is unrecorded.Refs #7867, PR #7989, ADR-0049, ADR-0087.