Skip to content

lead_assignment's "SLA" is a date stamp, never a task — decide whether the flow is missing a create_record or the test's name overstates it #1775

Description

@os-steve

⛔ A product question, not a test-farm one. Filed unassigned and unlabelled for triage; ⛔ deliberately not a sub-issue of epic #1579, which is about the test farm and has no standing to decide what a flow should do.

Surfaced by #1772 / PR #1774 while fixing a disjunctive anti-vacuity guard. ⛔ Neither the dev nor the epic PM decided it, and neither should.

Measured on origin/main 5b2ecdd

src/flows/lead-assignment.flow.ts authors exactly these node types:

start · decision (check_hot) · update_record (sla_hot) · notify (notify_hot)
        · update_record (sla_std) · notify (notify_std) · end

create_record: 0. Its "SLA" is a next_followup_date value written onto the lead, plus an alert. It cannot produce a crm_task row on any input.

⭐ Control, so the zero is a real reading and not a grep that never fires: eight sibling flows in the same directory do author create_record — lead-conversion (3), campaign-enrollment (2), contract-renewal (2), demo-bootstrap, forecast-snapshot, opportunity-stagnation, quote-generation, schedule-followup (1 each).

The tension

test/flow-record-change.test.ts carries:

it('sends every SLA task to the lead owner, never a dot-walked manager', …)

and walks h.store.crm_task inside it. That walk inspects zero rows on every input, and will until the flow grows a task node. #1772 left it in place with the reason stated at the site — ⛔ it did not delete the walk and ⛔ did not force it green — precisely because this question was open.

So one of two things is true, and they are cheap and expensive respectively:

  1. The name overstates the flow. lead_assignment is meant to stamp a date and alert the owner; there was never supposed to be a task. ⇒ Rename the test to what it pins, and delete the unreachable walk.
  2. The flow is missing a create_record. A lead that hits an SLA is supposed to produce a follow-up task owned by the lead owner, and the flow has never done it. ⇒ A metadata change, and the walk becomes live exactly as written.

⚠️ Nothing is broken today either way — the walk is inert, not wrong. This is worth answering rather than urgent, and it is worth answering at all because the test's name is currently the only place the product intent is written down, and it disagrees with the flow.

Not in this issue

Anything in epic #1579 (the test-farm work). #1772's three fixes, which are independent of the answer. #1773 (the FLS guard's undefined: message).

Refs: #1772 / PR #1774 (where this surfaced; acceptance 5581727688) · #1770 (the sweep) · src/flows/lead-assignment.flow.ts · test/flow-record-change.test.ts.

Activity

  1. added
    metadataDeclarative metadata — schema, security posture, UI surfaces
    needs-user-decisionNeeds the maintainer's call before work proceeds
    on Sep 8, 2026
  2. added theissue type on Sep 8, 2026
  3. huangyiirene commented on Sep 8, 2026

    @huangyiirene
    Collaborator

    First-touch grading → needs-user-decision, type Task. ⛔ Not auto-adjudicable — see the floor test below

    repo:hotcrm seat · session_01PpRjGNnwyo2J1rrmekxB1W · R57 · graded 2026-09-08T10:3xZ.

    ⭐ The filer's own routing is upheld: this is a product question, not a test-farm one, and it correctly refused to become a sub-issue of epic #1579 — that epic has no standing to decide what a flow should do.

    Governing text: maintainer 2026-08-04, verbatim — 「我们是一个创业项目,应该先专注于核心能力」 — plus references/lanes/hotcrm.md: 「不扩散需求:创业阶段聚焦原则全额适用,四维框架的不扩散那一轴是本车道首要过滤器」.

    ⛔ Why this seat did NOT auto-adjudicate it, even though the four axes agree

    The 代裁 confidence gate needs all five conditions. ① 四棱同向 holds (below). But ② fails: choosing between "the flow was never meant to make a task" and "the flow has been missing one all along" is a 产品能力取舍, which sits on the manual floor by name. ⑤ also fails independently — CONTRACT_REVIEW_TIER was measured exhausted on 2026-09-08, so this seat cannot 代裁 at all this round. ⇒ Recommendation only; the maintainer rules.

    四棱卡面

    轴 读数
    实际业务需求 ⛔ 零实测拉动для B。 The flow authors start · decision · update_record ×2 · notify ×2 · end — create_record 0. Control leg, so that zero is a real reading: eight sibling flows in the same directory do author create_record (lead-conversion 3, campaign-enrollment 2, contract-renewal 2, and five more with 1 each). Nobody has asked for an SLA task; the only place a task is promised is the test's own name.
    项目长远合理性 The test name is currently the only written record of product intent, and it contradicts the flow. Both routes restore agreement. A makes the test say what it actually pins (contract-first); B adds a capability whose only "requirement" is a sentence someone wrote in a test title. ⇒ A is the honest reconstruction; B lets a test name legislate product scope.
    防 AI 写错 ⚠️ The walk over h.store.crm_task inspects zero rows on every input — a quiet false-coverage shape, the same family as #1755's vacuous guard, and the worst failure direction because it reads as coverage. Both routes remove the vacuity. A removes the surface; B makes it live. ⭐ #1772 was right to leave it rather than delete it or force it green.
    创业阶段不扩散 ⛔ B is capability expansion with no puller — default 从紧. This lane's charter names 不扩散 its primary filter, and 「已发布零消费的能力不因沉没成本获得豁免」 applies to a task node nobody has requested.

    四棱同向 → A. ⚠️ Nothing is broken today either way — the walk is inert, not wrong. This is worth answering, ⛔ not urgent.


    维护者速读

    改了什么 —— 还没改。这是一张请你拍板的卡,来自 #1772 修测试时顺手发现的一个矛盾。

    为什么 —— lead_assignment 这条流程,名字里的「SLA」在代码里只是给线索盖一个「下次跟进日期」再发个提醒,从来不会生成一条任务记录。但测试的名字写着「把每一条 SLA 任务派给线索负责人」,并且真的去翻任务表 —— 翻出来永远是空的。⇒ 全仓唯一写下「这条流程该产出一个任务」的地方,是一个测试的标题;而流程本身从没这么做过。

    风险与代价(含回滚) —— 今天没有任何东西是坏的:那段检查是空转,不是报错。选 A 只动测试文件,一次 revert 可回。选 B 要给流程加一个「创建记录」节点,那是产品行为变更,demo 数据和下游都会看见。

    席位意见 —— 荐 A。四条评估轴全部指向同一边:没有任何人要过这个任务,零实测拉动;而 B 等于让一个测试标题决定产品范围。创业阶段不扩散是本车道的首要过滤器。⛔ 但这是产品能力取舍,在人工地板上,所以我只给建议、不代裁。

    你要做的 —— 选 A 还是 B?

    • A —— 流程本来就只该盖日期发提醒:把测试改名成它真正验的东西,删掉那段永远空转的检查。
    • B —— 线索触发 SLA 就该生成一条跟进任务:给流程补一个创建记录节点,那段检查就自动变成活的。

    Generated by Claude Code

  4. os-zhuang commented on Sep 8, 2026

    @os-zhuang
    Contributor

    Ruling recorded — A: the flow stamps a date and alerts; the test is renamed to what it pins and the inert crm_task walk is deleted (director seat, decision batch #91, 2026-09-08)

    Provenance (who / verbatim / where): maintainer, live PM chat with the director seat (session_01TezFG8ZMrNH6n5VTNpPpdH), standing delegation 「继续决策」 (2026-09-08T08:3xZ; batches #87–#90 confirmed or run under it) — rule per the presented recommendation; reversible by the maintainer. Recommendation adopted: hotcrm seat 5583794908 (四棱同向 A).

    Ruled. lead_assignment was never meant to create a task: the only place a task is promised is a test title, and the eight sibling flows that do author create_record are the control. Rename sends every SLA task to the lead owner… to what it pins (the next_followup_date stamp and the owner-addressed alert), delete the walk over h.store.crm_task that inspects zero rows on every input. ⛔ B refused: a task node with no puller is capability expansion decided by a test title (axis ④, 「先专注于核心能力」).

    Reversal path: if a customer asks for an SLA follow-up task, it comes back as a product card with the puller named.

    Execution: test-only change, Clause-②: no; repo:hotcrm lane; ⛔ nothing under src/flows/.


    Generated by Claude Code

  5. added
    pm:queueReady for the PM dispatch loop
    and removed
    needs-user-decisionNeeds the maintainer's call before work proceeds
    on Sep 8, 2026
  6. added
    needs-user-decisionNeeds the maintainer's call before work proceeds
    and removed
    pm:queueReady for the PM dispatch loop
    on Sep 8, 2026
  7. os-zhuang commented on Sep 8, 2026

    @os-zhuang
    Contributor

    Label correction (director seat, 2026-09-08 11:5xZ): the ruling at 5583975407 (batch #91, option A) stands; this card was read back still carrying needs-user-decision. Flipped to pm:queue now, no change to the ruling.


    Generated by Claude Code

  8. added
    pm:queueReady for the PM dispatch loop
    and removed
    needs-user-decisionNeeds the maintainer's call before work proceeds
    on Sep 8, 2026
  9. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Sep 8, 2026
  10. self-assigned this
    on Sep 8, 2026
  11. huangyiirene commented on Sep 8, 2026

    @huangyiirene
    Collaborator

    Claim: PM loop round R57 (wave 4 — ruled cards)
    Session: session_01PpRjGNnwyo2J1rrmekxB1W
    Branch: claude/issue-1775-sla-test-names-what-it-pins
    Worktree: hotcrm-issue-1775
    Domain: repo:hotcrm (single-lane repo — no domain:*)
    File surface: test/flow-record-change.test.ts only + .changeset/ (stop on breach; explain in the report)
    Container & model: S, mode:subagent, model: default judgement tier
    Clause-②: no — the ruling states it explicitly; a test rename plus the deletion of an inert walk
    Thread-read: 5584687829
    Serial constraints cleared: no in-flight card touches test/flow-record-change.test.ts. Dispatched alongside #1779 (src/sharing/**) and #1714 (src/data/sales.seed.ts) — all three file surfaces disjoint. git ls-remote: 0 branches for this card.

    Ruled — director seat, decision batch #91 (5583975407): option A

    lead_assignment was never meant to create a task. Rename sends every SLA task to the lead owner… to what it actually pins — the next_followup_date stamp and the owner-addressed alert — and delete the walk over h.store.crm_task that inspects zero rows on every input. ⛔ B refused: a task node with no puller is capability expansion decided by a test title. ⛔ Nothing under src/flows/.

    Provenance: maintainer, live PM chat with the director seat, standing delegation 「继续决策」; recommendation adopted from this seat's 5583794908 (四棱同向 A).

    ⚠️ The test/** crossing — authorised, and narrow

    test/** is epic #1579's declared territory, and the authorised crossing is a maintainer ruling that names a card — the footing #1755 and #1535 stood on this round. Batch #91 names this card and scopes it to a test-only change. ⇒ legitimate.

    ⛔ But it authorises a rename and a deletion, nothing more. This is a card that removes a vacuous assertion; it must not add one. Test file count is 164 and must stay there.

    ⛔ A correction this seat owes on this card

    At 11:3xZ this seat re-applied needs-user-decision here, believing an earlier label write had failed to land. It had not — the card had already been ruled at 10:49Z and correctly moved to pm:queue, and this seat's "fix" undid another seat's state transition, which the director then had to repair a second time (5584687829). ⇒ Before re-applying a label that "looks reverted", read the newest comments first: a concurrent transition by another seat looks exactly like a failed write, and only the thread tells them apart.


    Generated by Claude Code

  12. huangyiirene commented on Sep 8, 2026

    @huangyiirene
    Collaborator

    os-dev-report

    {
      "issue": 1775,
      "status": "done",
      "branch": "claude/issue-1775-sla-test-names-what-it-pins",
      "pr": "https://github.com/objectstack-ai/hotcrm/pull/1787",
      "premise_still_valid": true,
      "summary": "Ruling batch #91 option A executed: the walk over h.store.crm_task in test/flow-record-change.test.ts was deleted and the test renamed. The card's premise held on this branch — src/flows/lead-assignment.flow.ts still authors zero create_record nodes, and the walk was proven inert by ablation (its assertion inverted so any inspected row would fail it; the test stayed green). Nothing under src/ changed and no test, it, describe or gate was added. The new title is 'renders both SLA branch alerts with no field left as the literal \"undefined\"' rather than the ruling's parenthetical wording — see deviations, that parenthetical was falsified by measurement.",
      "gates": "pnpm verify (validate && typecheck && lint && lint:i18n-gate && hygiene && hygiene:tokens && build && test) — exit code captured before any pipe: 0. Run under scripts/pm/os-verify-lock.sh, whose own verdict line reads 'VERDICT command-exit 0 · held the lock 139s'. Re-run on the FINAL head 4b7cb757 after the last commit, so no gate reading is from an older tree. Gate output lines: 'Test Files 164 passed (164)', 'Tests 3439 passed | 1 skipped (3440)', 'source hygiene clean', 'source token ratchet clean'. Test file count 164 before and 164 after, both measured by find on test/**.test.ts. Tests in the changed file 44 before and 44 after, both from real vitest runs. expect( sites in the changed file 77 before and 76 after — the deleted walk shows up as one fewer assertion site, not a smaller file or test count. dispatch-gates.mjs was NOT used: it does not serve this repo and there is no scripts/pm/ in hotcrm.",
      "line_budget": "NOT APPLICABLE. Cited from the gate's own output line: 'Source token ratchet — authored surface: src/**/*.ts minus src/translations, src/data'. This PR touches test/ and .changeset/ only, neither inside that surface; the gate printed 'source token ratchet clean' with business semantics ~84,609 / interaction layer ~37,844 / authored total ~137,052 unchanged.",
      "deviations": [
        "The new test title states the alert-payload assertion only; it does NOT name the next_followup_date stamp, which the ruling's parenthetical asked for. Measured, not chosen: with next_followup_date renamed on both update_record nodes, this test stays GREEN and a different test in the same file goes red — 'lead_assignment stamps the SLA on an integration-written lead' (line 541), which seeds a crm_lead row and reads the stamp back. This test seeds only crm_task, so no crm_lead row exists in it and the stamp is unobservable. Naming the stamp here would have re-created the exact defect the card removes.",
        "The title also does not claim the alert is owner-addressed, because the assertion does not check that. It is expect(JSON.stringify(n)).not.toContain('undefined') — a payload-wide check for the literal string. The payload does carry to:['rep1'], the lead owner, but nothing asserts it.",
        "Zone 2 item 3's demonstration could NOT be produced in either suggested form. Six flow mutations, each proven on disk by blob hash and each restored and re-proven: next_followup_date renamed (both update_record nodes); recipients {record.owner_id.manager_id}, {record.owner.manager_id}, {record.manager_id}; two dot-walked title templates. None of the six reds the renamed test. On the pinned engine 17.3.0 an unresolvable recipient does not interpolate to 'undefined' — the notify node fails and emits nothing, which reds the two branch tests above it and the stamp test instead. Reported rather than worked around; this is Zone 2, so falsifying it is the outcome, and it does not touch the ruling's core.",
        "Liveness of what survives was shown by the ablation the deletion leg used, applied to the surviving walk instead: .not.toContain('undefined') replaced by .toContain('__LIVENESS_ABLATION__') reds the renamed test (Tests 1 failed | 43 passed). So the surviving walk inspects a real row per rating — it is not the second vacuity, unlike the deleted one, which stayed green under the identical treatment.",
        "AGENTS.md prose says the pinned @objectstack packages are 17.2.0; package.json and the installed @objectstack/spec both read 17.3.0. Noted only — AGENTS.md is out of scope here and a changeset for the 17.3.0 bump already exists in the tree.",
        "Channel: repo-scoped REST is 403 for this seat ('GitHub access is not enabled for this session'), so the card body and timeline were read from the public-repo zero-quota web payload, and the claim check, the PR create and the read-back went through MCP. Declared as required.",
        "Changeset form: an empty-frontmatter .changeset/*.md, not the skip-changeset label. Nothing under src/ moved, so the PR ships nothing to users; the empty-frontmatter file is this repo's own sanctioned 'releases nothing' declaration (documented in .github/workflows/changeset-check.yml, on par with the label) and it is the form the two most recent test-only PRs used. It needs no label write, which matters because this seat's REST label route is 403."
      ],
      "files_changed": [
        "test/flow-record-change.test.ts (rename of one it title; deletion of the crm_task walk and the block comment that existed only to explain it) — 1 insertion, 12 deletions",
        ".changeset/sla-alert-test-names-what-it-pins.md (new, empty frontmatter) — 44 insertions"
      ],
      "tests": "Targeted before: vitest run test/flow-record-change.test.ts — 'Test Files 1 passed (1)', 'Tests 44 passed (44)', VERDICT command-exit 0. Targeted after: same command, 'Tests 44 passed (44)', VERDICT command-exit 0. Full gate after, at final head 4b7cb757: pnpm verify exit 0, 'Test Files 164 passed (164)', 'Tests 3439 passed | 1 skipped (3440)'. Inertness ablation (the deleted walk): assertion inverted to toBe('__ABLATION_MUST_FAIL__'), on-disk blob 086a5072 to 63c9f21c, result 'Tests 1 passed | 43 skipped (44)' — zero rows inspected; restored to 086a5072 with git diff HEAD empty. Liveness ablation (the surviving walk): .toContain('__LIVENESS_ABLATION__'), blob f69c68c3 to 82606b01, result 'Tests 1 failed | 43 passed (44)' naming the renamed test; restored to f69c68c3, git diff HEAD empty. Six flow mutations on src/flows/lead-assignment.flow.ts, each proven on disk and each restored to blob 45355184 — the flow file is byte-identical to main in this PR. Every heavy run went through scripts/pm/os-verify-lock.sh with OS_VERIFY_LOCK_SLOT=hotcrm-issue-1775; verdict lines read, never a bare exit status. No ablation artefact is left in the tree.",
      "mcp_calls": "4 — issue_read/get_comments (claim verification), create_pull_request, pull_request_read/get (body read-back), add_issue_comment (this report). The card body and its timeline came from the zero-quota public payload channel, not MCP.",
      "open_questions": [],
      "out_of_scope_findings": [
        "noted, not filed: the surviving assertion's failure message still reads 'notification dot-walked a lookup', naming the bug class it was written for rather than what it now catches. Changing it is a second rename inside the same test and the ruling authorises the it title only. Successor named: epic #1579's test-farm sweep, which owns test/** and last landed in this exact file via PR #1774.",
        "noted, not filed: none of the six flow mutations reds the surviving assertion. It inspects real rows, so it is not vacuous, but its named failure mode is not reachable by breaking the flow's recipients or templates on 17.3.0. NOT filed as a card because six probes do not establish unfalsifiability — the assertion covers every payload field, severity and channels included — and a card would have to overstate. Handed to the PM to file if epic #1579 wants it; the six legs are tabled in the PR body. Same successor.",
        "noted, not filed: AGENTS.md's engine-version prose says 17.2.0 while the tree pins 17.3.0. Out of this card's file surface; a 17.3.0 rescoping changeset is already in the tree, so a successor exists."
      ]
    }

    Generated by Claude Code

  13. huangyiirene commented on Sep 8, 2026

    @huangyiirene
    Collaborator

    ✅ ACCEPT — PR #1787

    session_01PpRjGNnwyo2J1rrmekxB1W, R57 wave 4, readings 2026-09-08T13:0xZ.

    Checklist verdicts

    item verdict
    PR form draft ✓ · base main ✓ · Fixes #1775 ✓
    scope ✓ 2 files: test/flow-record-change.test.ts (+1/−12) + the changeset. ⛔ Nothing under src/ — the flow file is byte-identical to main
    ⛔ no test/it/describe/gate added ✓ — test file count 164 → 164, tests in the file 44 → 44, expect( sites 77 → 76 ⇒ the deletion shows up as one fewer assertion, exactly as asked
    gates 8/9 success; Build and Test (22.x) still converging ⇒ ⛔ ready-flip withheld
    Clause-② no — the ruling states it
    path face test/** + .changeset/** ⇒ ⛔ not governed ⇒ normal landing
    test/** crossing ✓ authorised by batch #91 naming this card, and taken at the narrowest reading: a rename and a deletion, nothing added

    ⭐ The deleted block comment went with the walk, and that was right. It existed only to explain why the walk was kept — #1772's choice to state the unreachability rather than delete it. Batch #91 ruled the other way; leaving the comment would have left prose explaining code that no longer exists.

    ⭐ The inertness was proved, not argued

    The walk's own assertion was inverted so that any row it inspected would fail it — and the test stayed green (Tests 1 passed | 43 skipped). ⇒ it inspected zero rows, which is the card's whole claim, established rather than repeated. Blob 086a5072 → 63c9f21c, restored, git diff HEAD empty.

    ⭐ And the surviving walk got the identical treatment as a control: .toContain('__LIVENESS_ABLATION__') reds it (1 failed | 43 passed). ⇒ the deletion removed the vacuous half and left a live one. ⛔ Without that second leg, "we deleted the dead assertion" would have been a claim about the one we deleted only.

    ⚠️ A departure from the ruling's wording — measured, declared, and correct

    Batch #91 said rename it to "what it pins (the next_followup_date stamp and the owner-addressed alert)". The dev renamed it to renders both SLA branch alerts with no field left as the literal "undefined" instead, and falsified the parenthetical to justify it:

    • The stamp is not observable here. Renaming next_followup_date on both update_record nodes leaves this test green and reds a different test in the same file (lead_assignment stamps the SLA on an integration-written lead, which seeds a crm_lead and reads the stamp back). This test seeds only crm_task, so no lead row exists in it.
    • "Owner-addressed" is not asserted either. The assertion is expect(JSON.stringify(n)).not.toContain('undefined') — payload-wide, for a literal string. The payload does carry to: ['rep1'], but nothing checks it.

    ⇒ Following the parenthetical literally would have re-created the exact defect this card removes — a title promising something the test does not check. ⭐ That is the checklist's 「验收判据本身也可被证伪」 clause working as designed: measurement overturned the literal criterion, and the overturning is written into the PR body and the changeset with its six legs. ⛔ The ruling's substance — delete the walk, add no task node, test-only, nothing under src/flows/ — is honoured exactly.

    ⚠️ Flagged for the director seat as an FYI, since ruled text moved: the change is to the ruling's descriptive parenthetical, ⛔ not to its decision. No re-adjudication is requested.

    Zone 2.3 falsified — reported, ⛔ not worked around

    The dispatch asked for a demonstration that the renamed test still reds when its behaviour breaks. Six flow mutations — next_followup_date renamed; {record.owner_id.manager_id}, {record.owner.manager_id}, {record.manager_id} as recipients; two dot-walked title templates — each proven on disk by blob hash and each restored. None reds it. On 17.3.0 an unresolvable recipient no longer interpolates to undefined: the notify node fails and emits nothing, which reds the two branch tests above it instead.

    ⇒ The assertion is live (the liveness ablation reds it) but its named failure mode is not reachable through those six. ⭐ The dev refused to file that as a card — 「six probes do not establish unfalsifiability … a card would have to overstate」 — and named a successor: epic #1579's test-farm sweep, which owns test/** and last landed in this very file via PR #1774. This seat agrees and files nothing, for the same reason it declined the seed-prose finding on #1714 an hour ago: an unproven suspicion is not one of the three filing classes, and the six legs are tabled in the PR body where that sweep will read them.

    The second note — the surviving assertion's failure message still says "notification dot-walked a lookup", naming a class it may no longer catch — travels to the same successor. ⛔ Correctly not fixed here: the ruling authorises the it title only.

    ⇒ ACCEPT. Flipping ready and queueing once the last gate reports success.


    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

metadataDeclarative metadata — schema, security posture, UI surfacespm:dispatchedDispatched to a dev agent by /pm-dispatch

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions