Skip to content

fix(approvals): one decision, one dialog — carry reject/recall confirm questions on description - #7592

Merged
os-help merged 1 commit into
mainfrom
claude/issue-7278-approval-confirm-dialogs
Aug 11, 2026
Merged

os-help merged 1 commit into
mainfrom
claude/issue-7278-approval-confirm-dialogs

Conversation

@os-help

@os-help os-help commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7278

Executes the maintainer's 2026-08-10 ruling (comment 5237211565), quoted verbatim:

Option 1. Drop confirmText from approval_reject and approval_recall; the param dialog carries the confirm question as its description — one condition, one wording, one dialog, nothing sent until its own Confirm.

What changed

Both actions on sys_approval_request declared confirmText and params. The console action runner chains confirmation then param collection, both awaited, so one decision opened a confirm prompt and then a second dialog the approver never asked for — nothing sent until that second Confirm, while the first prompt already read as "the action is running".

Each question now rides the action's top-level description (the key added by #7367 / PR #7430 for exactly this card), which objectui's ActionParamDialog renders under the dialog title. Wording carried verbatim — including the finality warning "A rejection is final for every approver.", whose loss is what disqualified option B in the escalation.

Deliberately not ai.description: the two same-named keys sit one level apart, and the AI one is the LLM-facing tool contract that no human ever sees. A test pins that the question did not land there.

The i18n half, which was not obvious

The four generated locale bundles carried confirmText for both actions, so they were regenerated in merge mode via the command the extract config's own docstring documents.

A renamed key reads to the extractor as a brand-new gap, so --fill=default seeded the new description leaf with English in zh-CN / ja-JP / es-ES — silently discarding the curated translations of the very same sentence, on the most irreversible surface in the product, in three of the four shipped locales. Those were carried across by hand (the bundle header sanctions editing leaf values in place) and are now pinned by a new test, because check:i18n structurally cannot see that loss: an English string in a non-English locale is precisely what a fresh extract produces, so the bundle is "in sync" either way.

Verification

  • pnpm --filter @objectstack/plugin-approvals typecheck — clean.
  • pnpm --filter @objectstack/plugin-approvals test21 files, 458 tests, all passing (was 20/452).
  • node scripts/check-i18n-bundles.mjs --filter=plugin-approvalsin sync (4 bundle(s)), confirming the hand-carried translations survive merge mode.
  • i18n coverage ratchet, measured per-config rather than via a whole-repo build: 0 i18n rule issues, exactly this config's committed baseline.
  • check:docs-audit-scope, check:nul-bytes — green.

Reverse verification. Restoring the old declaration (git checkout origin/main -- <the object file>) turned exactly the two change-detecting pins red, in the predicted direction:

× the confirm question rides `description`, not `confirmText`, so one decision opens one dialog (#7278)
× no declared action pairs `confirmText` with `params` (#7278 ruling, object-wide)
  Test Files  1 failed | 20 passed (21)

The ai.description guard and the translation pins stayed green under that revert, correctly — they are non-regression guards, not change detectors. The pre-existing expect(byName('approval_recall').confirmText).toBeTruthy() also went red on the fix itself and was replaced by the content pins above.

Scope

Held to the two declarations: no packages/spec change (the key already exists and is correct), no InlineActionSchema (deliberately excluded by PR #7430), and none of the other 18 confirmText+params sites — those are #7309, which follows this card. No out-of-scope findings.


Generated by Claude Code

…m questions on `description` (#7278)

`sys_approval_request.approval_reject` and `approval_recall` declared both
`confirmText` and `params`. The console action runner chains confirmation THEN
param collection, both awaited, so one decision opened a confirm prompt and then
a second dialog the approver never asked for — with nothing sent until the second
Confirm, while the first prompt already read as "the action is running".

Per the maintainer's 2026-08-10 ruling on #7278 (Option 1), each question moves
to the action's top-level `description` — the key #7367 / PR #7430 added for
exactly this — which objectui's `ActionParamDialog` renders under the dialog
title. The wording is carried verbatim, finality warning included.

The four generated locale bundles were regenerated (`os i18n extract` merge
mode). Because a renamed key reads as a new gap, `--fill=default` seeded the new
`description` leaf with English in zh-CN / ja-JP / es-ES, silently discarding the
curated translations of the very same sentence; those were carried across by hand
and pinned, since `check:i18n` cannot see that loss — an English string in a
non-English locale is exactly what a fresh extract produces.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy
@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 11, 2026 6:41am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/approvals.mdx (via @objectstack/plugin-approvals)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-approvals)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-approvals)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-approvals)
  • content/docs/releases/v9.mdx (via @objectstack/plugin-approvals)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 11, 2026
@os-help
os-help marked this pull request as ready for review August 11, 2026 06:57
@os-help
os-help added this pull request to the merge queue Aug 11, 2026
Merged via the queue into main with commit 7abdd74 Aug 11, 2026
26 checks passed
@os-help
os-help deleted the claude/issue-7278-approval-confirm-dialogs branch August 11, 2026 07:13
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Aug 17, 2026
…irm questions on `description` (objectstack-ai#7309) (objectstack-ai#7827)

The console action runner chains confirmation THEN param collection, both
awaited, so an action declaring `confirmText` *and* `params` opened two
sequential dialogs for one click — nothing sent until the second, while the
first already read as "the action ran".

Follows the maintainer's 2026-08-10 ruling on objectstack-ai#7278 (shipped in PR objectstack-ai#7592):
carry the confirm question on the action's top-level `description` (objectstack-ai#7367) and
drop `confirmText`. Sweeps the 14 remaining in-repo action sites, all in
`identity/`. Each question moves verbatim — no warning reworded, none dropped.

`confirmText` is left in place where it is still correct: a param-LESS action
has no param dialog to fold into, so the confirm is its only dialog.

The en/zh-CN/ja-JP/es-ES bundles move the same 14 leaves by hand, because
`os i18n extract --fill=default` would seed English over the curated
translations for a renamed key — invisible to `check:i18n`.

Tests pin the consequence in both directions: params ⇒ one dialog, and the
question is still shown. Deleting a warning instead of moving it goes red.

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: os-zhuang <jack@objectstack.ai>
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Aug 17, 2026
… scoped by schema boundary (objectstack-ai#7972)

* feat(spec): refuse `confirmText` + non-empty `params` on ActionSchema

An action declaring both opens TWO sequential dialogs for one decision —
the console runner awaits the confirm, then the param prompt, so the first
already reads as "the action ran" while nothing has been sent.

objectstack-ai#7278 and objectstack-ai#7309 repaired the 16 shipped sites (PRs objectstack-ai#7592, objectstack-ai#7827); neither
stops the next one being written. This is the structural half: a refusal on
ActionSchema whose message names both keys and points the confirm question
at the action's top-level `description`.

Scoped by schema boundary, not by a param-optionality heuristic. The pair is
INTENDED on `BulkActionDefSchema`, where params and confirm render one dialog
(a `required` param blocks that dialog's own Confirm button), so the four
`examples/app-showcase` bulk defs are correct as written. The refusal lives on
ActionSchema's refine chain and is structurally incapable of reaching either
`BulkActionDefSchema` or `InlineActionSchema`; both directions are pinned.

* docs(spec): declare the confirmText/params refusal on the key, + changeset

The `confirmText` describe() now names the refusal, so an author meets the
constraint in the generated reference rather than only in the error. Worded
"on a registered action" deliberately: the same field factory renders the
InlineAction reference table, where the guard does not apply — an unqualified
claim there would be a documented refusal that does not exist.

Regenerates content/docs/references/ui/action.mdx (gen:docs).

* docs(objectui): two action examples taught metadata the spec now refuses

`content/docs/protocol/objectui/actions.mdx` showed `disable_oauth_application`
and `rotate_client_secret` pairing `confirmText` with `params`. Both were
already stale — the real actions moved the question to `description` in objectstack-ai#7827 —
and the guard in this PR makes the documented shape fail to parse.

They are labelled "Real-World Examples", so they now mirror what the platform
objects actually ship, and the `confirmText` bullet above them states the
constraint: param-LESS actions only, question on `description` otherwise, not
`ai.description`, and bulk defs unaffected.

Found via the PR's docs-drift list; the brace-balanced census could not see
these because the examples are YAML.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

approvals: sys_approval_request.approval_reject / approval_recall declare both confirmText and params, so one decision opens two sequential dialogs

2 participants