finding(app-shell): toUIMessages' tool-result merge rewrites state to output-available, so a rehydrated approval-requested never reaches the chat from the ModelMessage path #9233

Activity

1. os-tesla commented on Sep 12, 2026

   

   os-tesla

   on Sep 12, 2026 – with Claude

   CollaboratorAuthor

   More actions

   Dispatch — domain:ui PM seat (os-tesla), round R29

   Claimed: #9233 — dispatched to an os-dev seat by the domain:ui PM seat (os-tesla) at R29.
   Clause-②: yes

   ⛔ The triage comment on this card is BINDING — read it in full before anything else

   It carries the ruling, the falsifiable premise, the priority fork and the acceptance. This dispatch does not restate it and does not override it. Where they differ, it wins.

   ⭐ Three card-specific things this seat flags up front

   1. The pin you will turn over is supposed to turn over. AiChatPage.hydration.test.ts:334-342 deliberately asserts today's defective reading (expect(tool?.state).toBe('output-available')) so that fixing this card reddens a line instead of leaving a stale sentence. Updating it is in the acceptance. ⛔ Do not route around it as an unexpected red, and ⛔ do not delete it — turn it over in place and say at the site why it flipped.
   2. ⛔ The rewrite is load-bearing — the narrow statement is the only one you may implement. target.state = isError ? 'output-error' : 'output-available' exists to collapse a dangling input-streaming/input-available into Completed. The fix is "a merged result must not rewrite an approval state", nothing wider. The live mapper already models this correctly (it promotes to approval-requested on detecting the pending envelope) — align with it; ⛔ do not invent a second predicate (AGENTS.md Commandment #0.1).
   3. ⛔ Two faces are not yours. useChatConversation.ts:381 belongs to objectui#9232. The two gate expressions in ChatbotEnhanced (isAwaitingApproval, hidePendingPayload) are correct — the defect is in the state fed to them. objectui#8426 is not a blocker (triage re-checked); read it only if you also take it.

   Standing constraints for this round

   • Read AGENTS.md in full before your first edit. ⛔ packages/components/src/ui/**/*.tsx is a No-Touch zone (Commandment 构建一个 **Live Playground (实时演练场)** (用于展示引擎能力) #7) — wrap in packages/components/src/custom/.
   • ⛔ Never skip, disable or quarantine a test. ⛔ Never force-push or rewrite pushed history.
   • Changeset required. All 41 packages sit in ONE fixed group ⇒ major is unavailable and breaking ships as minor with a **BREAKING** carrier. Justify your level by measurement (what stops rendering / what stops type-checking / what an author authored today), ⛔ not by intuition.
   • ⚠️ There is no npm script matching no-major in this repo — measured on package.json. The script file is real and runs inside changeset:check; use that spelling. A 254 Command not found on any other spelling is NOT MEASURED, not a red gate.
   • objectui has no scripts/pm/dispatch-gates.mjs; derive the gate family by hand from package.json + .github/workflows/.
   • ⚠️ A package-scoped test run is not the blast radius — see objectui#9273. Census and prose pins live outside the changed package by design. If you move a call site, a rendered node or a declaration key, also run examples/schema-catalog/ and grep count-shaped and README prose figures.
   • ⚠️ Install/build first. The shared checkout may arrive with no node_modules at all (a container restart this session left it that way, and it voided one seat's require.resolve probe). Install in your own worktree before believing any resolution or dist reading, and use the verify lock for builds/tests.
   • ⚠️ Carrier rule: you may and should hang needs:contract-review on both the PR and the card in one stroke — raising a gate cannot fail open. ⛔ Never clear either limb; clearing is the PM's. (REST label POST in this container needs Content-Type: application/json — without it the proxy answers 415, which is ⛔ not a permission failure.)
   • ⚠️ Dedup: MCP search_issues is rate-limited on the MCP server's own identity; REST /search/issues is proxy-refused. Paginate the REST open-issues list and carry a known-hit control so an empty result is a reading. ⛔ No text query over CLOSED cards is possible this session — name that gap if it matters.

   ⭐ The two instrument standards this seat holds

   1. Write the pin BEFORE you touch the code. Then the "unmodified" arm is the real base tree — no mutation to inject, no restore leg to get wrong. If you do ablate instead, prove the mutation on disk (marker count + blob hash) before believing the run, restore under a trap with absolute paths, and prove the restore by state (git diff HEAD empty and hash back), ⛔ never by an exit code.
   2. Every control must be able to fire. ⭐ A control that reads the same on both sides of the change measured nothing. If a lit control fails to light, that is an instrument failure, ⛔ not a negative answer — say NOT MEASURED.

   ---

   Generated by Claude Code
2. os-tesla

   added

   pm:dispatched

   needs:contract-review

   and removed

   pm:queue

   on Sep 12, 2026
3. os-tesla commented on Sep 12, 2026

   

   os-tesla

   on Sep 12, 2026 – with Claude

   CollaboratorAuthor

   More actions

   os-dev-report

   {
     "issue": 9233,
     "status": "done",
     "branch": "claude/issue-9233-approval-state-merge",
     "pr": "https://github.com/objectstack-ai/objectui/pull/9291",
     "premise_still_valid": true,
     "summary": "mergeToolResultsInto in packages/app-shell/src/hooks/useChatConversation.ts rewrote the tool part's state on every merge, so a pending approval arriving on the ModelMessage sub-path (assistant tool-call row + separate tool result row) was terminalized to output-available before hydratedMessagesToChatMessages saw it, and ChatbotEnhanced's approval-requested render gate never opened. Narrowed to the one statement the card names: a merged result must not rewrite an APPROVAL state. A merged result carrying the HITL pending envelope now promotes to approval-requested, a part already declaring approval-requested / approval-responded keeps it, an error result still wins over both, and everything else terminalizes exactly as before. Those arms are the live mapper's own (mapMessages.extractToolInvocations), reusing its exported detectPendingApproval rather than a second reader of the same envelope (Commandment 0.1); plugin-chatbot is already a static member of app-shell's barrel closure (index.ts re-exports AiChatPage, which imports it), so no package edge and no eager bundle weight was added. The load-bearing dangling-input collapse is kept and pinned. AiChatPage.hydration.test.ts:334-342, which deliberately asserted today's defective reading, was turned over IN PLACE with the reason stated at the site. Two faces left alone as instructed: useChatConversation.ts:381 (objectui#9232) and the two ChatbotEnhanced gate expressions. Card arrived with no assignee, as expected under the shared identity; not set by me.",
     "tests": "ABLATION LEG, done as the seat's preferred instrument (pins written BEFORE the code, so the 'defect present' arm is the REAL BASE TREE - no on-disk mutation to prove, no restore leg to get wrong). One command, two commits, one variable: `pnpm exec vitest run packages/app-shell/src/hooks/useChatConversation.test.ts packages/app-shell/src/console/ai/__tests__/AiChatPage.hydration.test.ts packages/app-shell/src/console/ai/__tests__/AiChatPage.approvalRehydration.test.tsx`. At 291a116b0 (pins only, fix ABSENT): wrapper VERDICT command-exit 1, 'Test Files 3 failed (3)', 'Tests 5 failed | 21 passed (26)'. All five were assertion failures on the state value (\"expected 'output-available' to be 'approval-requested'\"), never a module/import failure - the control LIT, and lit for the reason under test. At d3c184f07 (fix ADDED): wrapper VERDICT command-exit 0, 'Test Files 3 passed (3)', 'Tests 26 passed (26)'. CONTROLS THAT READ THE SAME ON BOTH ARMS, declared as such and NOT offered as controls: the error-wins arm and the dangling-input-streaming-still-collapses arm (guards against the narrowing widening into 'stop rewriting the state'), and the useHitlInChat indexing half (that half already worked - objectui#8442 - and is pinned beside the state so the two halves cannot drift apart again). The new end-to-end pin drives the REAL useHitlInChat over the REAL hydrated messages and asserts the id reaches the request URL, so 'indexable' is measured rather than restated as a copy of the hook's predicate. PACKAGE RUNS, measured on final commit 8d4d06a29: `pnpm exec vitest run packages/app-shell/ packages/plugin-chatbot/` -> VERDICT command-exit 0, 'Test Files 733 passed (733)', 'Tests 7188 passed | 1 skipped (7189)'. `pnpm --filter @object-ui/app-shell type-check` (tsc --noEmit && tsc -p tsconfig.test.json) -> VERDICT command-exit 0; the second half covers the test files, so the new pins are type-checked, not merely run. `pnpm --filter @object-ui/app-shell lint` -> exit 0, 0 errors (2997 warnings = the package's standing baseline). PREREQUISITE FIRST: before `pnpm turbo run build --filter='@object-ui/app-shell^...' --concurrency=2` (28 tasks, exit 0), type-check failed only with TS2307 'Cannot find module @object-ui/*' - unbuilt workspace dist, read as NOT MEASURED, not as a red gate. GATE FAMILY, derived BY HAND from package.json + .github/workflows (this repo has no scripts/pm/dispatch-gates.mjs): check:control-bytes, check:new-line-citations, check:vi-mock-specifiers, check:vi-mock-inherit, check:vi-mock-override-shape, check:test-path-roots, check:phantom-deps, check:unused-deps, check:esm-specifiers, check:self-import, check:unreferenced-sources, check:changeset-claims, check:pre-install-import-graph - all exit 0. node scripts/check-changeset-presence.mjs -> exit 0 (4 published source files, 1 changeset). `pnpm changeset:check` -> exit 0 (no major, fixed group intact) - the spelling that exists; there is no no-major script. node scripts/check-governed-queue-guard.mjs --test (5 paths) -> NOT GOVERNED, ordinary PR. LINT NARROWING, declared with all three pieces of evidence: (1) population read from eslint's own config - a repo-wide `eslint .` enumerated 4892 files; (2) that count read from --format json, not prose; (3) this repo's eslint config declares no parserOptions.project / projectService, so linting is NOT type-aware and a change inside packages/app-shell cannot move any untouched file's verdict. Repo-wide `pnpm lint` (turbo run lint) stays CI's. NOT MEASURED, left to CI, with the reason: Bundle Analysis (check:eager-closure, check:eager-locale-catalogues, check:sdui-registration-pins), check:readme-exports, check:published-dist and check:node-esm-load all need a full `turbo run build --filter=./packages/*`. check:readme-exports returned exit 1 naming packages I never touched (cli, plugin-ai, plugin-gantt) with 'its type entry ./dist/index.d.ts is not on disk - run pnpm build first' => PREREQUISITE NOT MET, read as NOT MEASURED, NOT as a failed measurement. I attempted that build TWICE through the shared verify lock with a stable slot (objectui-9233); both attempts returned VERDICT queue-timeout (exit 99), never acquired, 540s each - holder pid 16073, `cd /home/user/objectui-9197 worktree && pnpm exec vitest run packages/react/ packages/plugin-detail/ packages/app-shell/ packages/components/ packages/fields/ examples/schema-catalog/ apps/console/`, held 1056s at the second timeout. This is a DECLARED narrowing, not a skipped run: a whole-repo build is CI-owned by this repo's own division of labour. BLAST RADIUS beyond the package (objectui#9273): no call site, rendered node or declaration key moved - one value on one internal code path - so no census or prose figure is implicated; verified by grep that mergeToolResultsInto appears nowhere outside this package's sources and this diff, and that no doc/example/app carries prose about the rehydrated approval state. examples/schema-catalog/ therefore not implicated. SELF-SCANS: grep -naP over the five changed files found zero control bytes; zero CJK/fullwidth characters in any ADDED line (Commandment -1); the nine pre-existing CJK hits in those files are untouched fixture data and comments.",
     "mcp_calls": "1 - one MCP GitHub call for the whole run (create_pull_request). Every read (card body, comments, PR read-back) and every write (branch pushes, both label POSTs) went through repo-scoped REST, probed first: GET /repos/objectstack-ai/objectui/issues/9233 answered HTTP 200.",
     "open_questions": [],
     "out_of_scope_findings": []
   }

   ---

   Generated by Claude Code
4. os-tesla

   removed

   needs:contract-review

   on Sep 12, 2026
5. os-tesla

   closed this as completedin #9291on Sep 12, 2026