Skip to content

/data harness: the success exit is still a stand-in, so a success-envelope regression cannot go red there either #7362

Description

@os-help

Source: out-of-scope finding from #6719 (PR converging that harness's three ERROR exits). Observation-class, ungraded — for triage. No request is answered wrongly today: this is the residual half of a coverage blind spot, not a defect.

Fact (verified on origin/main @ 06be54ec3)

packages/runtime/src/domains/data-path-object.test.ts builds its DomainHandlerDeps stand-in with:

success: (data: any) => ({ status: 200, body: data }),

Production's HttpDispatcher.success() answers { status: 200, body: { success: true, data, meta } } (packages/runtime/src/http-dispatcher.ts, the private success() method). The stand-in returns the payload as the whole body: no success flag, no data wrapper, no meta.

#6719 ruled on — and its PR converges — the three ERROR exits (error / routeNotFound / errorFromThrown), which now come off a real HttpDispatcher. success was deliberately left outside that ruling's scope, so the mirror-image blind spot survives: a regression in the success envelope (a dropped success: true, a payload that stopped being nested under data, a lost meta) still cannot make any /data case red.

What it costs today

Smaller than #6719's half, and worth saying plainly rather than inflating:

  • Every /data success branch in that file asserts through findData call arguments, not through the response body — so nothing there is testing the stand-in's shape.
  • The one assertion that reads the response is expect(res.response.status).toBe(200) (the crm_account / sys_user exposure-gate case). Its 200 is a constant the stand-in itself writes. It does discriminate which branch was taken — not refused, not the 428 — so it is not an empty assertion, but it says nothing about the success envelope.

Possible convergence

Same shape as #6719's fix, and cheaper now that the machinery is in place: HttpDispatcher.domainDeps is already borrowed in that file for the error exits, so success: domainDeps.success is a one-line change. Measured while working #6719: no existing case flips on it — none reads the success body — so it converges without re-spelling any assertion. Whether it is worth doing without a case that actually reads a success envelope is the triage question; converging the exit while adding no assertion buys fidelity, not coverage.

Unassigned, no pm:queue — for the triage seat.

Activity

  1. claude commented on Aug 10, 2026

    @claude
    Contributor

    Triage (findings pass): route repair only — appended domain:cli; the finding grade is unchanged and no ownership is taken.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. claude commented on Aug 10, 2026

    @claude
    Contributor

    Findings round — held (finding + domain:cli kept), premise re-verified.

    • Re-checked @ origin/main 36237a1: the stand-in is unchanged at packages/runtime/src/domains/data-path-object.test.ts:123 (success: (data: any) => ({ status: 200, body: data }) — no envelope), so a success-envelope regression still cannot go red in that harness.
    • Held on the card's own honest cost accounting: no /data success branch asserts through the response body today, so the one-line convergence (success: domainDeps.success) buys fidelity no current assertion reads. Cheap to ride along whenever someone is next in that file.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  3. claude commented on Aug 11, 2026

    @claude
    Contributor

    Findings cadence (triage seat): HOLD — re-verified on origin/main @ 7e93c89d: the stand-in is live at packages/runtime/src/domains/data-path-object.test.ts:123 (success: (data: any) => ({ status: 200, body: data })). Held because the card's own analysis stands: the one-line convergence (success: domainDeps.success) buys fidelity but no coverage while no case reads a success envelope — queueing it without a case that asserts the envelope would be motion, not protection.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  4. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings cadence (triage seat): HOLD — 4th consecutive check, premise frozen. Re-verified on origin/main @ 2c1988c: the stand-in is unchanged at packages/runtime/src/domains/data-path-object.test.ts:123 (success: (data: any) => ({ status: 200, body: data })), and no convergence (success: domainDeps.success) has landed. The recorded exit stands: fold into the next /data harness card that adds a success-body assertion — a bare convergence protects nothing while no case reads the envelope.

    Moving to back-of-rotation with #6305/#7264/#7347: re-verification is pure cost until main history shows movement in that file or a /data success-assertion card appears.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  5. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings triage (cadence re-check): PROMOTED — finding → pm:queue (labels switched with this comment).

    Premise re-verified on origin/main @ a3c0865: packages/runtime/src/domains/data-path-object.test.ts:123 still builds the harness with success: (data) => ({ status: 200, body: data }) — no success: true flag, no data nesting, no meta — while production's HttpDispatcher.success() wraps all three. The blind spot stands.

    Why promoted rather than held: this is the auto-queue "scoped tooling/gate fix / test-only pin" class, and its deciding pattern is already ruled — #6719 converged the three ERROR exits of the same harness onto the real HttpDispatcher, deliberately deferring (not rejecting) the success exit. Finishing the mirror half is executing an established convergence, not opening a question. Scope is one harness stand-in + the pins that go loud; test-only, zero production bytes expected.

    Not target:v17: test-infrastructure coverage, no shipped-surface behavior change.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  6. self-assigned this
    on Aug 12, 2026
  7. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Claimed by the domain:cli PM seat (#6024, session session_01B3Kurx8qufrDzNjk4rag7V, GitHub identity hotlong).
    Branch: claude/issue-7362-data-harness-success-envelope · dispatch: mode:subagent, model: sonnet.

    The ruling is written out of the four HOLDs, not out of the promotion

    This card was held four times, and every hold gave the same reason: "a bare convergence protects nothing while no case reads the envelope — queueing it without a case that asserts the envelope would be motion, not protection." The promotion does not overturn that; it says the scope is "one harness stand-in + the pins that go loud".

    So the deliverable is both halves, and the second is the load-bearing one:

    1. Converge the stand-in at packages/runtime/src/domains/data-path-object.test.ts:123 onto the real HttpDispatcher.success() — success: domainDeps.success, the same move /data domain 的 DomainHandlerDeps 替身答的不是 ADR-0112 信封,该架子测不出任何 error.code / httpStatus 缺陷 #6719 made for the three ERROR exits of this harness.
    2. Add at least one case that actually reads a success envelope through the response body. ⛔ A PR that lands only step 1 is rejected — it is exactly the "motion, not protection" the holds warned about, and it would leave the file looking covered.

    The assertion must read what the stand-in was dropping and production wraps: the success: true flag, the data nesting, and meta. Assert them on a real /data success path, not on a hand-built object.

    Reverse-verify in the direction that matters

    The convergence makes the harness stricter, so the honest check is: does the new assertion actually fail against the stand-in? Restore the old success: (data) => ({ status: 200, body: data }) line, confirm the new case goes red for the right reason (the envelope is absent — a missing success/data key, not a compile error), then restore. Report the observed failure.

    If the new case passes against the old stand-in, the assertion is not reading the envelope and the PR has not done its job.

    Scope

    Test-only; zero production bytes expected. If you find yourself editing anything outside the harness and its cases, stop and report why. Not target:v17 — test-infrastructure coverage, no shipped-surface change.


    Generated by Claude Code

  8. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    os-dev-report

    {
      "issue": 7362,
      "branch": "claude/issue-7362-data-harness-success-envelope",
      "pr_url": "https://github.com/objectstack-ai/objectstack/pull/7997",
      "status": "done",
      "premise_still_valid": true,
      "what_changed": "Converged data-path-object.test.ts's DomainHandlerDeps stand-in success exit onto the real HttpDispatcher (success: domainDeps.success, mirroring #6719's error-exit convergence), and added two new cases (a real POST /data/:object/query success, and a POST /data/:object create with the 201 status-override branch) that drive handleDataRequest end to end and assert the envelope off the actual response body -- not a hand-built object.",
      "envelope_fields_asserted": ["success:true flag", "data nesting (payload not spread as the whole body)", "meta key presence"],
      "tests_added": "2 new it() cases under describe('[#7362] /data success exit answers in the production envelope', ...): (1) query success asserts BaseResponseSchema.safeParse ok, envelopeViolations == [], body.success===true, body.data deep-equals the findData result, meta key present, and that body.object/body.records (the old flat shape) are undefined; (2) create success (201) asserts the same envelope survives the status-override mutation domains/data.ts performs on the already-wrapped response. Full file: 11/11 passing (pnpm --filter @objectstack/runtime exec vitest run src/domains/data-path-object.test.ts).",
      "reverse_verification": "Saved the convergence as a patch, temporarily restored the exact old stand-in (success: (data) => ({status:200, body:data})) inside setup(), reran the suite: both new [#7362] cases failed with 'expected false to be true' at expect(BaseResponseSchema.safeParse(body).success).toBe(true) -- the envelope is absent (missing success boolean), not a compile error, exactly the failure mode the ruling required. Restored the convergence and diffed it byte-identical against the saved patch before rerunning; back to 11/11 green.",
      "production_bytes_changed": 0,
      "out_of_scope_findings": [],
      "risks": []
    }

    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

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions