Skip to content

【缺陷】被 multiple:true lookup 指向的对象 REST DELETE 全 400——cascadeDeleteRelations 依赖探针用裸等值查 JSON 列 #9362

Description

@os-zhuang

自 qa-run 记录 #9351(records-forms R3 轮,crud-roundtrip clause 2,P0)抽取——按维护者 2026-08-17 裁决,run 记录为协议载体不入分诊 sweep,可派发缺陷抽取为独立卡。完整证据链、控制组与复现命令见 #9351;本卡为可派发单元。

复现(纯 schema 驱动,无需特定数据)

POST   /api/v1/data/showcase_account   {"name":"anything","status":"active"}  → 201, id
DELETE /api/v1/data/showcase_account/<id>                                     → 400 INVALID_FILTER

期望 200 且行删除;实际 400,行存活。三个新建 id 复现 3/3;删空 showcase_field_zoo 全部行后仍 400(schema 驱动非数据驱动);对照 showcase_announcement/showcase_category DELETE 均 200(排除「DELETE 路由坏了」)。

机制(读数在 packages/objectql/src/engine.ts:10113,cascadeDeleteRelations)

依赖探针对每个指向被删对象的 lookup/master_detail 字段构造裸等值 filter——包括声明 multiple: true 的字段。showcase_field_zoo.f_lookups 是 Field.lookup('showcase_account', { multiple: true }),存 JSON TEXT 列,driver 正确以 INVALID_FILTER 拒绝该拼写;探针 catch(engine.ts:10147)只放行 missing-table 类,INVALID_FILTER 上抛 ⇒ 整个删除失败。

爆炸半径:凡被任何已注册对象的 multiple: true lookup 指向的对象,REST 删除全部不可用。stock showcase 上即 showcase_account。

关联:#8895(2026-08-16 合并)把 catch { continue } 改为 discriminate-or-propagate——该收紧本身正确,但探针对多值引用字段的 filter 拼写从未修正:旧行为是静默吞掉(同时静默跳过该关系的完整性守卫,恰是 #8895 抱怨的),新行为是硬拒绝。未做 bisect,因果为源码强推断非证明(#9351 原文如此标注,如实转录)。

修向提示(供实现参考,非裁决):多值字段的依赖探针应使用 driver 声明的多值拼写($contains 类),或按字段声明分支构造 filter——修在探针构造处,⛔ 不放宽 driver 的拒绝。

发现归 QA R3 轮 runner;抽取:skills 席(session_017TNzEetykdh7ceZGwuAPLq)。未定级——留分诊首触(P0 建议随定级确认)。

Activity

  1. added theissue type on Aug 17, 2026
  2. os-steve commented on Aug 17, 2026

    @os-steve
    Collaborator

    Triage: lands in packages/objectql/src/engine.ts (cascadeDeleteRelations probe filter construction) ⇒ domain:engine-core. Type Bug, pm:queue, priority:p0 + target:v17 — rationale: this is a published data-plane surface (REST DELETE) hard-broken for every object referenced by any multiple: true lookup, reproduced 3/3 with controls, and it is a regression window opened by #8895 (merged 2026-08-16): the old probe swallowed the INVALID_FILTER silently (wrong, but deletes worked); the discriminate-or-propagate tightening now turns the never-correct probe spelling into a hard 400. Release-blocker class ①: users hit it today on stock showcase. Dedup checked (cascadeDeleteRelations, INVALID_FILTER): #8895 is the closed adjacent fix, no open twin.

    Dispatch notes for the engine-core seat: fix belongs in the probe's filter construction (multi-value spelling per field declaration, e.g. $contains-class), ⛔ do not loosen the driver's rejection and ⛔ do not re-widen #8895's catch. The card's own "修向提示" matches. Predicate/filter semantics ⇒ per SKILL.md the compile-surface checklist clause applies (declare which of the 5 filter→predicate compilers are touched; expected: only the probe's construction site). Size/model suggestion: M / opus.

    P0 is triage-graded per the QA extraction's recommendation; maintainer veto window = this round's report.


    Generated by Claude Code

  3. os-zhuang commented on Aug 17, 2026

    @os-zhuang
    ContributorAuthor

    Claim — engine-core dev seat.

    • Session: session_01NTKPDRoynY8i3HmdSFUxFj
    • Branch: claude/issue-9362-cascade-probe-multiple-lookup
    • Worktree: ../objectstack-9362-cascade-probe

    Starting on the cascadeDeleteRelations probe filter construction per the triage note. If an earlier claim with a different session ID exists, this one yields.


    Generated by Claude Code


    Generated by Claude Code

  4. os-zhuang commented on Aug 18, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 9362,
      "status": "done",
      "branch": "claude/issue-9362-cascade-probe-multiple-lookup",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/9437",
      "premise_still_valid": true,
      "summary": "cascadeDeleteRelations now spells its dependents probe for the KIND of reference field it is aimed at: a multiple:true field is asked with $contains (the membership spelling driver-sql's own refusal prescribes, and the one all five drivers answer), and the returned rows are then narrowed EXACTLY, element-wise, because $contains is a substring test and the pushdown answers a superset. Both correct behaviours around it are untouched: driver-sql's INVALID_FILTER refusal and #8895's discriminate-or-propagate catch. The PM's flagged attribution was cheap to settle so it was measured, not restated: engine.ts checked out at a751f7d4f7^ (immediately pre-#8895) and rebuilt returns 200 with the row gone for the card's repro AND returns 200 when a live dependent really does reference through the array, so #8895 converted a silent fail-open into the hard 400 rather than creating the fault. NOT FIXED, AND THE PM SHOULD SEQUENCE THIS BEFORE MERGE: repairing the probe makes the set_null limb run for a multi-value relationship for the first time ever, and it nulls the WHOLE array (measured: refs ['acc_a','acc_b'] becomes null when acc_a is deleted, dropping the live acc_b). Filed as #9438 rather than guessed at; see open_questions. Also surfaced: #9390 is an open, unclaimed duplicate of this card from a different QA run.",
      "tests": "All numbers below from HEAD c29939be52 (git rev-parse --short HEAD at the final commit). NEW: packages/objectql/src/engine-cascade-delete-multivalue-probe.test.ts (8 tests, driver double reproducing the #7398 JSON-column refusal ON THE FILTER) and packages/runtime/src/cascade-delete-multivalue-lookup-real-driver.integration.test.ts (3 tests, real ObjectQL + real SqlDriver on better-sqlite3 through ObjectStackProtocolImplementation.deleteData, the method REST DELETE /api/v1/data/:object/:id serves). REVERSE VERIFICATION, both legs, artifact proven in BOTH directions because packages/runtime resolves objectql from dist: fix reverted -> objectql suite '7 failed | 1 passed (8)' (the 1 green is the #8895-propagation control this card does not move); fix reverted + objectql rebuilt, marker count in dist/index.mjs = 0 -> runtime suite '3 failed (3)', every failure \"expected 'INVALID_FILTER' to be ...\"; fix restored + rebuilt, marker count = 3 -> '8 passed (8)' and '3 passed (3)'. A first draft of the double evaluated the refusal per ROW, so with an empty table it refused nothing and the card's own reproduction passed with the fix reverted — caught on the reverse-verification lap, fixed in fc538c5831 (the real driver raises it while COMPILING the predicate; the double now does too). BISECT (settled, not inferred): engine.ts at a751f7d4f7^ rebuilt -> repro test PASSES (200, row gone) while both guard tests FAIL with \"expected undefined to be 'DELETE_RESTRICTED'\". FULL SUITES: @objectstack/objectql 'Test Files 216 passed (216) / Tests 3819 passed (3819)'; @objectstack/runtime 'Test Files 166 passed (166) / Tests 2470 passed (2470)'; typecheck clean on both. GATES re-derived via node scripts/pm/dispatch-gates.mjs over the 4 changed paths and run at c29939be52: check:changeset-gate-self-tests, check:cross-package-test-inputs, check:durability-log-level, check:objectui-changeset, check:stack-collection-maps, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check:type-check-debt (--re-measure, full workspace closure built first: '33 ledger entries re-measured, none above its recorded number'), check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-engine-split-ratio, check-cross-package-test-inputs, check-nul-bytes, check-error-code-casing, check-affected-docs — all PASS. check:query-options-erasure went RED first (test surface 240 -> 244 from four unnecessary casts on SqlDriver.count, whose query arg is optional and typed); the casts were removed, ceiling not raised. NONE SKIPPED. COMPILE-SURFACE CHECKLIST (per SKILL.md): exactly one filter/predicate construction site changes — the probe's, in cascadeDeleteRelations. ZERO of the 5 filter->predicate compilers is touched, and no public filter surface is widened ($contains was already declared).",
      "open_questions": [
        {
          "question": "What does deleteBehavior:'set_null' MEAN on a `multiple: true` reference field? Repairing the probe (this PR) makes that limb execute for the first time ever, and today it writes null over the whole array — measured data loss on the stock showcase shape. Filed as #9438; the direction ('remove the member') is not really in doubt, but the residual shape when the array empties is, and it is observable.",
          "options": [
            "A — remove the member and write the remaining array; needs the residual-shape answer ([] or null) when the last member goes, which is observable on the read path and to a required multi-value validator",
            "B — escalate the DEFAULTED set_null to `restrict` when the field is multiple:true, mirroring the required-FK escalation already pinned three lines above it in the same block; refuses loudly, mints no semantics, one-line revert once A's question is answered",
            "C — ship #9437 as is and fix separately; the lossy write is live on main in the interval"
          ],
          "recommendation": "B in the same round as #9437 if A's residual shape cannot be answered immediately, then A. Real business need: the shape is live on the stock showcase (showcase_field_zoo.f_lookups at showcase_account), and the affected write is one users cannot see happen — this is measured harm, not a speculative surface. Long-term soundness: A is the contract-first answer (the semantics of set_null on a set-valued FK belongs in the engine, decided once); B is explicitly a holding position that decides nothing and reverts in one line, which is why it is acceptable where C is not — C leaves silent data loss on main and buys only latency. Hard to get wrong for AI-authored metadata: B is strictly better than C on this axis because deleteBehavior defaults to set_null, so an AI-authored multi-value lookup that never mentions deleteBehavior gets the lossy path by DEFAULT; refusing loudly at delete time surfaces the undecided semantics at the moment it matters instead of hiding it in a successful 200. A, once its residual shape is pinned in FieldSchema, is better still — declared and enforced rather than tolerated."
        }
      ],
      "out_of_scope_findings": [
        "filed as #9438: cascadeDeleteRelations' set_null limb nulls the WHOLE multi-value array, dropping every other live reference (measured; made reachable by this PR — sequence before merge)",
        "not filed, surfaced instead: #9390 is an OPEN, unassigned, unclaimed duplicate of this card from a different QA run (#9334). No claim comment on it, so it is not another agent's in-flight work; a cross-reference comment was added there with the bisect result and a correction (the guard is #7398, not #5869 as that card states). Closing/deduping it is triage's call, and no closing keyword in PR #9437 names it."
      ]
    }

    Generated by Claude Code


    Generated by Claude Code

  5. os-zhuang commented on Aug 18, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 9362,
      "status": "done",
      "branch": "claude/issue-9362-cascade-probe-multiple-lookup",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/9437",
      "premise_still_valid": true,
      "summary": "Second round: the maintainer's option-B escalation is implemented on the existing branch/PR (now eee5f89b2b, still draft, no auto-merge armed). While a multiple:true reference field would take the set_null limb, the delete is refused (DELETE_RESTRICTED / 409) instead of nulling the whole array; it is one `if` mirroring the required-FK escalation directly above it and reverts by deleting that `if` plus the developerMessage limb. The probe fix is untouched. BOTH SUB-CALLS ANSWERED FROM THE CODE, one confirmed and one declined: (a) CONFIRMED, and my own defaulted-only proposal was the wrong one — `fdef.deleteBehavior || 'set_null'` collapses an absent declaration and an explicit `set_null` into a single value, so the pinned required-FK escalation ALREADY covers both, and escalating both is what mirroring it means; defaulted-only would have required ADDING a distinction the pattern does not make. (b) DECLINED AS SPECIFIED, and reported rather than done quietly: packages/spec/src/system/operation-message.ts already rules this exact envelope 'one wire code with two sentences ... Splitting the SENTENCE, never the code — DELETE_RESTRICTED stays one member of the ADR-0112 vocabulary that clients match on', and the existing delete_restricted / delete_restricted_required pair is the same reason-discriminator with no new code; a code minted for a holding position would also need ADR-0087 retirement (tombstone + migration entry) when #9438 lands, which is the opposite of a one-line revert. The distinction is NOT skipped: it rides developerMessage (the developer-audience half #7307 established), which says TEMPORARY and cites objectstack#9438 literally so removal is one grep, while the business sentence stays unchanged because the user's action is unchanged. If you want a machine-readable discriminator anyway, a structured field is one line; a wire code needs its own maintainer decision and I did not take it.",
      "tests": "All numbers from HEAD eee5f89b2b (git rev-parse --short HEAD after the final commit). ESCALATION REVERSE VERIFICATION, ON ITS OWN: the escalation `if` ablated with the probe fix left intact — markers checked in packages/objectql/dist/index.mjs before running (escalation 'multiValueHold = true' = 0, probe 'referenceProbeFilter' = 3) — gives objectql '3 failed | 12 passed (15)' and runtime '1 failed | 5 passed (6)'; restored + rebuilt (markers 1 and 3) gives '15 passed (15)' and '6 passed (6)'. SECOND ABLATION, because the first cannot exercise the over-fire controls: the four controls assert the guard does NOT fire, so removing it can never fail them — they are green under that ablation BY CONSTRUCTION, which I am flagging rather than reporting as a pass. Ablated the other way instead, widening the condition to `||`: all four go RED ('4 failed | 11 passed' and '2 failed | 4 passed'), including the pre-existing multi-value cascade pin from round one. Neither direction is vacuous. BOTH DIRECTIONS TESTED as asked, 6 dispositions: multi-value defaulted set_null -> 409 with the array intact when re-read from the DATABASE; multi-value EXPLICIT set_null -> 409, array intact; multi-value cascade -> 200, dependents deleted; multi-value already-restrict -> 409 carrying its OWN sentence (asserted NOT to contain '9438' or 'TEMPORARY', so the holding position and configured policy stay tellable apart); single-valued set_null -> 200, FK cleared exactly as before; any multi-value relation with no dependent rows -> 200, row gone (the card's P0 repro). P0 STILL CLOSED end to end for the restrict and cascade paths on the real SqlDriver stack. FULL SUITES: @objectstack/objectql 'Test Files 216 passed (216) / Tests 3826 passed (3826)'; @objectstack/runtime '166 passed (166) / 2473 passed (2473)'; @objectstack/rest '122 passed (122) / 2022 passed (2022)' — rest was run unprompted because a grep showed it is the other package declaring multi-value lookups, i.e. the one that could plausibly have depended on the limb being held back; typecheck clean on objectql and runtime. GATES re-derived via scripts/pm/dispatch-gates.mjs (path set unchanged from round one) and re-run at eee5f89b2b: check:changeset-gate-self-tests, check:cross-package-test-inputs, check:durability-log-level, check:objectui-changeset, check:stack-collection-maps, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check:type-check-debt (--re-measure, full workspace closure rebuilt first: '33 ledger entries re-measured, none above its recorded number'), check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-engine-split-ratio, check-cross-package-test-inputs, check-nul-bytes, check-error-code-casing, check-affected-docs — ALL PASS, NONE SKIPPED. check:error-code-casing is green trivially because no code was minted. Changeset updated with a section describing the refusal as a temporary holding position and naming objectstack#9438; no closing keyword names #9438 in the changeset, the commit message or the PR body.",
      "open_questions": [
        {
          "question": "Sub-call (b): should the temporary refusal carry a machine-readable discriminator, given that a new StandardErrorCode is declined above on the strength of operation-message.ts's landed rule for this envelope? Today the distinction is human/greppable only (developerMessage says TEMPORARY and cites objectstack#9438).",
          "options": [
            "A — leave as shipped: developerMessage only. No response-shape change at all, and #7307's audience split is respected exactly (the reason is developer-facing; the user's action is identical)",
            "B — add a structured boolean/reason field on the error object beside the existing dependentObject / dependentCount. Machine-readable, not part of the ADR-0112 code vocabulary, one line to add and one to remove — but it is still a response-shape addition, and #8895 twice recorded 'no new response field' as a virtue on this same path",
            "C — mint a distinct wire code after all. Needs a maintainer decision, an errors.zod.ts entry, and an ADR-0087 retirement when #9438 lands"
          ],
          "recommendation": "A, and I shipped A. Real business need: nobody has to branch on this at runtime — the only consumers of the distinction are the person who removes the hold (a grep for 9438 finds it) and this PR's own tests (which assert on developerMessage), and no measured caller needs to tell the two refusals apart programmatically. Long-term soundness: the envelope already has a landed rule for exactly this situation and following it costs nothing, while B and C both add surface to something designed to be deleted in one line — C worst, because retiring a wire code is a tombstone plus a migration entry, i.e. the holding position outliving the thing it was holding. Making AI-authored code hard to get wrong: this cuts toward A too, but for a reason worth stating — the failure this whole hold prevents is an AI-authored multi-value lookup that never mentions deleteBehavior silently getting the lossy path, and what protects that author is that the refusal HAPPENS and says why in words they will read, not that a machine can classify it. If you disagree, B is one line and I will add it on request; C I will not take without you re-asking the maintainer."
        }
      ],
      "out_of_scope_findings": [
        "filed as #9438 (round one, still open and NOT closed by this PR): cascadeDeleteRelations' set_null limb nulls the WHOLE multi-value array. Now held back rather than merely reported — the lossy write is no longer reachable on main once this lands, so #9438 becomes a semantics card rather than a data-loss card, and the hold reverts in one `if` when it is answered",
        "surfaced, not filed: #9390 remains an open, unassigned, unclaimed duplicate of this card from a different QA run (#9334); a cross-reference comment was left there in round one with the bisect result and a correction (the guard is #7398, not #5869 as that card states). No closing keyword in PR #9437 names it"
      ]
    }

    Generated by Claude Code


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions