scripts/demo.mjs still tells the operator that clm_contract.legal_owner resolves against the dev admin — PR for #47 made that false #54

Activity

1. zhuangjianguo

   added

   pm:queue

   pm:dispatched

   and removed

   pm:queue

   on Sep 10, 2026
2. zhuangjianguo

   self-assigned this

   on Sep 10, 2026
3. zhuangjianguo commented on Sep 10, 2026

   

   zhuangjianguo

   on Sep 10, 2026 – with Claude

   CollaboratorAuthor

   More actions

   Claim: session session_01R3n3GGzobdegM4HUzah1iR · branch claude/issue-54-demo-mjs-stale-note

   PM dispatch. The assignee and this claim are set by the PM seat on behalf of the dev that will work this card; the dev inherits both, checks that this is the newest Claim: on the thread and that it names its branch, and ⛔ posts no second claim and never writes the assignee field.

   Unblocked as of now. The card said "land it after both PR #51 and the PR for #47". Both have merged — 2807b3b and d34b225 — so scripts/demo.mjs is unheld. Branch off current origin/main (2d63324).

   Dispatch notes:

   • Verify the premise on your own tree first. PR pnpm demo prints its "now create the three requesters" instruction ~290 lines and 120 ERROR lines before the ready banner, so nobody reads it #51 rewrote large parts of scripts/demo.mjs (it added announceOperatorSetup() and the port probe) after this card was written. Read the two passages as they exist now — the DEMO_USER doc block and the failure detail on the Dev Admin name check — and confirm both still enumerate clm_contract.legal_owner. If PR pnpm demo prints its "now create the three requesters" instruction ~290 lines and 120 ERROR lines before the ready banner, so nobody reads it #51 already dropped one of them, say so and fix only what is actually stale.
   • Fix the enumeration, keep the mechanism. The card is explicit and it is right: clm_review.reviewer is required: true, so a name drift still costs all 60 review rows and the check still earns its place. Only the three-item list is stale — it is two references now. ⛔ Do not weaken the failure text.
   • Grep rather than trusting the card's two locations. This is a card about a stale enumeration; a second stale copy elsewhere in the file — or in src/data/keys.ts, which makes the same argument — would be the same defect surviving the fix. Report what the grep found, including "nothing else".
   • Roughly four lines. ⛔ Do not widen: no tidying of neighbouring comments, no changes to the probe or the banner.
   • Gates all four, exit codes captured before any pipe. scripts/demo.mjs is outside tsconfig's include and outside the metadata graph, so none of them can move — say that rather than implying they verified the change. No browser run is owed; if you can cheaply exercise the failure path (run the name check against a wrong name), that reading is worth more than the gates and is welcome, not required.

   ---

   Generated by Claude Code
4. claude commented on Sep 10, 2026

   

   claudebot

   on Sep 10, 2026 – with Claude

   Last edited by claude

   Contributor

   More actions

   os-dev-report

   {
     "issue": 54,
     "status": "done",
     "branch": "claude/issue-54-demo-mjs-stale-note",
     "pr": "https://github.com/objectstack-ai/hotclm/pull/60",
     "premise_still_valid": true,
     "summary": "Premise verified on my own tree at 2d63324 before any edit: PR #51 did NOT drop either passage, so both were still stale and both were fixed. scripts/demo.mjs:68-69 (the DEMO_USER doc block) and scripts/demo.mjs:262-263 (the failure detail on the 'Dev Admin' name check) each enumerated clm_review.reviewer, clm_obligation.owner AND clm_contract.legal_owner; each now names the two that really do resolve against the dev admin. Cross-checked on origin/main that dropping it is right, not just the card's word: src/data/contract.seed.ts:110 seeds legal_owner via legalOwnerOf() which returns LEGAL_OWNERS ('Legal Counsel 1'/'Legal Counsel 2'), never DEMO_USER, while negotiation.seed.ts:51 (reviewer) and post-signature.seed.ts:38 (owner) still seed DEMO_USER. The mechanism is untouched: clm_review.reviewer is required:true with storage.notNull (review.object.ts:48-53), so a name drift still costs all 60 review rows; the three lines saying what the drift costs, and the recovery command, are byte-identical. Diff is exactly 4 insertions / 4 deletions in one file - no probe, no banner, no neighbouring comment touched.",
     "tests": "GATES (each redirected to a file, exit code read into a variable immediately after, no pipe between command and $?): pnpm validate=0, pnpm lint=0, pnpm typecheck=0, pnpm lint:i18n-gate=0. HONEST READING: none of the four can move on this diff and they verify nothing about it. tsconfig include is ['objectstack.config.ts','src/**/*']; `tsc --noEmit --listFiles` names 305 files and ZERO under scripts/, so scripts/demo.mjs is not in the typecheck program; validate/lint/i18n-gate read the compiled metadata stack and translation surfaces, and .mjs tooling is outside the metadata graph. The gates prove only that nothing else was disturbed. REAL VERIFICATION - the failure path was executed, three legs, via a scratchpad-only harness (nothing added to the repo) that puts a fake `objectstack` on PATH which stays alive without booting and stubs globalThis.fetch so the sign-in probe answers with a chosen account name, driving the real primeAdminAccount in the real file. LEG A (pre-fix file from `git show HEAD:scripts/demo.mjs`, wrong name 'Site Administrator'): exit 1, printed the stale list - \"resolves `clm_review.reviewer`, `clm_obligation.owner` and / `clm_contract.legal_owner` against a `sys_user` named \\\"Dev Admin\\\".\" - defect reproduced. LEG B (this branch, same wrong name): exit 1, same shape, corrected list - \"resolves `clm_review.reviewer` and `clm_obligation.owner` / against a `sys_user` named \\\"Dev Admin\\\".\" - followed by the untouched cost sentence \"every review row is refused while the rest of the seed reports success\" and the rm -rf recovery block. LEG C (control, this branch, correct name 'Dev Admin'): the check does NOT fire - prints '1/2 done - admin account ready.' and '2/2 starting HotCLM...' - so leg B is a guard firing, not a guard that fires always. Also: `node --check scripts/demo.mjs` clean; `grep -n legal_owner scripts/demo.mjs` now returns nothing (exit 1); control-char scan `grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]' scripts/demo.mjs` clean (exit 1); no harness process outlived the run (verified with ps; only PIDs the harness itself recorded were ever signalled, never by name). No changeset: this repo has no changeset gate and no .changeset/ directory. No browser run owed - this file is operator-facing terminal output, not a UI surface.",
     "mcp_calls": "0 - issue body and comments read from the public single-card web page embedded JSON payload; the labels/PR write went through authenticated REST (probe: GET /repos/objectstack-ai/hotclm -> 200); branch and commit through git.",
     "open_questions": [],
     "out_of_scope_findings": [
       "noted, not filed: clm_deviation.decided_by is a THIRD live DEMO_USER reference neither list has ever named - negotiation.seed.ts:79 seeds `decided_by: deviation.status === 'open' ? null : DEMO_USER` and deviation.object.ts:93 is Field.user (optional). After this fix the doc block enumerates two of the three user references that actually resolve against 'Dev Admin'. Not class (a): an omitted member is incompleteness, not a false statement, it predates PR #55, and the field is optional so an unresolvable name lands NULL rather than costing the row. Adding it would widen a card the dispatch explicitly fenced. Successor: whoever next edits either enumeration decides whether the list wants to be exhaustive or wants to name only the required reference the priming boot exists for.",
       "noted, not filed: negotiation.seed.ts:101 also writes DEMO_USER into clm_signature.signers[].name, but that is a plain JSON string inside the signers payload, not a user reference the loader resolves - correctly absent from both lists. Recorded so the next reader of the grep does not mistake it for a fourth miss.",
       "noted, not filed: the DEMO_USER doc block's second paragraph still contrasts only against clm_contract.owner_id ('NOT `clm_contract.owner_id`'). Post-#55 legal_owner belongs in that same optional-and-names-operator-accounts category, but the sentence is not false as written and tidying a neighbouring comment is exactly what the dispatch ruled out. Successor: whoever next touches scripts/demo.mjs or src/data/keys.ts.",
       "nothing else: grep over src/, scripts/, docs/, README.md and objectstack.config.ts for DEMO_USER|LEGAL_OWNERS|legalOwnerOf|legal_owner found no other stale copy of the enumeration. src/data/keys.ts:33-34 carries the same claim and PR #55 already corrected it ('clm_contract.legal_owner is no longer one of them - see {@link LEGAL_OWNERS}'); README.md:93 already names Legal Counsel 1/2 as the legal_owner. Every other legal_owner hit is object/view/flow/dataset code operating on the field, not a claim about which account name it resolves against."
     ]
   }

   ---

   Generated by Claude Code
5. zhuangjianguo commented on Sep 10, 2026

   

   zhuangjianguo

   on Sep 10, 2026 – with Claude

   CollaboratorAuthor

   More actions

   ACCEPT — PR #60

   CI green on head 7c56003 (two Validate runs, both success), diff is scripts/demo.mjs only at exactly +4/−4, no governed surface, first round. Squash-merging.

   I checked the diff line by line against the card's fence: both passages changed, the cost sentence and the rm -rf .objectstack/data && pnpm demo recovery byte-identical, the port probe and announceOperatorSetup untouched. Nothing widened.

   What makes this PR worth reading rather than just merging

   Leg C. Legs A and B — reproduce the stale output, then show the corrected output on the same input — are what any careful fix produces. Leg C is the one most fixes skip: running the guard with the correct account name to show it does not fire. Without it, A and B together prove only that a string changed; they are equally consistent with a guard that now fires unconditionally. That is the ablation discipline applied to a control-flow change, and it is the difference between "I changed the text" and "the guard is still a guard".

   Refusing to let the gates take credit. You could have written "four gates green" and left it. Instead: tsc --noEmit --listFiles names 305 files and zero under scripts/, so this file is not in the typecheck program at all, and the other three read the metadata graph. The conclusion you drew — the gates prove only that nothing else was disturbed; the three legs are the verification — is the correct relationship between a gate and a change it cannot see. On a repo whose CLAUDE.md opens with "⛔ Gate every metadata change", knowing precisely when the gates are not evidence is the harder half of that rule.

   The premise check found the answer the dispatch asked for. I told you PR #51 might have already dropped one of the two passages. You read both at 2d63324 and reported that neither had — with the line numbers — rather than quietly fixing whatever was there. And you did not stop at the card's word that dropping legal_owner is right: contract.seed.ts:110 → legalOwnerOf() → LEGAL_OWNERS, never DEMO_USER, while negotiation.seed.ts:51 and post-signature.seed.ts:38 still do. The card asserted the direction; you established it.

   The grep came back with a real answer, including the negative. src/data/keys.ts:33-34 carries the same claim and PR #55 already corrected it — so the fix that would have been a second defect was not needed. Reporting "already correct, not touched" is worth as much as reporting a hit.

   The three out-of-scope findings

   All three calls are right, and the reasoning on the first is the one I want on the record:

   Not class (a): an omitted member is incompleteness, not a false statement … Adding it would widen a card the dispatch explicitly fenced.

   That distinction — a list that omits a member versus a list that names a wrong one — is exactly the line this card was drawn on, and it is why clm_deviation.decided_by was correctly left alone here. But the underlying data fact deserves a card of its own, and it is getting one: the fixture writes decided_by: DEMO_USER on every non-open deviation, so in the demo the administrator adjudicated every deviation legal ever raised. I verified it independently — negotiation.seed.ts:79 is deviation.status === 'open' ? null : DEMO_USER, and deviation.object.ts:93 is Field.user whose own description says it is "Stamped with the acting user when the deviation is decided, unless set explicitly". The seed is that explicit set, so this is not a stamping hook falling through — it is the fixture naming the admin on purpose.

   That is the same family as #47 (legal_owner) and #52 (obligation.owner), with one difference worth stating: decided_by is an audit stamp, not a recipient. Nothing is routed to it, so no reminder lands on an account that cannot act — the cost is that a screen legal opens says the administrator made legal's decisions. Filed as its own card, queued behind #52 so that whatever route #52 lands on becomes the precedent rather than two cards inventing two conventions for the same fixture.

   Your other two — the signers[].name string that is correctly absent from both lists, and the doc block's second paragraph that is incomplete but not false — are recorded for the successor exactly where they belong.

   ---

   Generated by Claude Code
6. zhuangjianguo

   added a commit that references this issue on Sep 10, 2026

   docs(demo): drop legal_owner from the dev-admin reference lists (#54)

   ...

   6e53468
7. zhuangjianguo

   closed this as completedin #60on Sep 10, 2026
8. zhuangjianguo

   added a commit that references this issue on Sep 10, 2026

   Merge origin/main into claude/issue-52-obligation-owner

   ...

   79f0da7