Skip to content

Add Always allow for a Bot or a group to tool approval cards - #544

Merged
Birfy merged 7 commits into
ThinkFlowLab:mainfrom
Birfy:fix/always-allow-approval
Oct 20, 2026
Merged

Birfy merged 7 commits into
ThinkFlowLab:mainfrom
Birfy:fix/always-allow-approval

Conversation

@Birfy

@Birfy Birfy commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #540

Tool approval cards had only Reject / Allow once, and DSH's approval outcomes are one-shot (ApprovalOutcome = allowed-once | rejected | cancelled | unavailable, ApprovalPolicy = ask | never in @deepseek-ai/dsh-user-approval 0.2.0-rc.2), so a Bot asked for the same sandbox escalation every time. Owner decision 2026-10-20 20:11 / 20:12 (Asia/Shanghai): permanent approval, per Bot and per group.

What changes

  • Cards (core/src/client/member-status.js ApprovalCard, core/src/client/inbox.js InboxCard) add Always allow for this Bot (此 Bot 始终允许) and, for an ask from a group, Always allow for this group (本群始终允许). Never on a friend's Bot.
  • Host (core/src/host/approvals.js): permission kind = tool + sandbox mode parsed from DSH's reason (bash:escalate:danger-full-access). approval-answer takes always-bot / always-room; the grant is kept on the Bot / room record (standingApprovals, in state.json), and this ask plus other waiting asks it covers are answered allowed-once. FlyBot's approval/request listener (documented waterfall, no DSH patch) answers later matching asks itself, only for a turn the owner started (turn-origin.js settled() === 'owner'; friends' messages, other members' lines, rooms with other people and verdict-less turns still ask). A Bot grant covers the Bot's Sessions; a group grant covers the owner's Bots working in that group (not their own chats). DSH's audit pair still logs each ask.
  • Revoke: Always allowed card with Remove in Bot settings and Group settings (core/src/client/standing-approvals.js, endpoint standing-approval-remove).
  • Authz (core/src/host/authz.js): an always-* answer needs the Bot's owner (owning the room is enough only for Allow once); removing a group grant needs the member who gave it, and removes only theirs.
  • Docs: core-host "Standing tool approvals (Approval card has no way to approve permanently (per Bot or per group) #540)", core-client row. Changelog fragment 540-always-allow-approval.added.md.

Risk tier: T3 (risk-gate: core/src/host/approvals.js, authz.js, channel.js). Waits for the maintainer's approval; not merged by the agent.

Verification (tip f7e6a7e789005365befd6bb44d55f88d5871607a = b6236f8 merged with main at cf61952, Node 24.21)

  • ARCH_DOCS_BASE=upstream/main TZ=UTC npm run verify: exit 0; core 1122/1122, plugins 313, 83, 93, 93, 37, 16 all pass; arch-docs OK (31 files); no new lint warnings.
  • Red at base (worktree at b6e89d2 + only the test files copied): 7 new/changed tests fail; green at tip.
  • Smoke (CONTEXT_WINDOW=100000 CHECK_RATIO=0.45 dev/bench.sh smoke 3196 8896 reset && node dev/smoke.mjs smoke 3196, DSH 0.2.0-rc.2): 42/42 at b6236f8; the merge to f7e6a7e changes only client files from Name a friend's Bot one way and approval cards by the receiving person #502, core/src/host and core/index.js are identical.
  • An earlier verify run hit tasks.test.js "an untagged hold stamps the latest working task…" (also 1 of 6 isolated runs at this branch); that test and its code are untouched here: a pre-existing flake.

Not run

  • Smoke at the merge tip f7e6a7e (host unchanged since the smoked tip).
  • A live run in the owner's FlyBot with a real bash escalation (covered by unit tests with DSH's escalate sandbox to <mode>: … reason format).

Birfy added 3 commits October 20, 2026 20:21
DSH's approval outcomes are one-shot, so a Bot asked for the same
sandbox escalation every time. Keep the owner's standing grant on the
Bot or the group and answer later matching approval/request asks with
allowed-once from FlyBot's own answerer; revoke from Bot or Group
settings.

Fixes ThinkFlowLab#540
@Birfy Birfy added bug Something isn't working agent-pr Pull request opened by an agent area:ui Web/desktop UI and copy area:security Auth, grants, approvals, secrets labels Oct 20, 2026
@Birfy Birfy self-assigned this Oct 20, 2026
@github-actions github-actions Bot added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Oct 20, 2026
Birfy added 3 commits October 20, 2026 20:30
…proval

# Conflicts:
#	core/client.js
#	patches/ds-bot/0003-flybot-api-client.patch
#	patches/ds-bot/0008-client-bundle.patch
…ab#540)

Review found that a Bot's grant would also answer a turn a friend or
another room member started, and that on a host nobody signed in to a
group grant did not list or cover Bots made after the identity update.
Check turn-origin before answering, and treat every local Bot as the one
person's until authz is enforced.
…proval

# Conflicts:
#	core/client.js
#	patches/ds-bot/0008-client-bundle.patch
@Birfy

Birfy commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at f7e6a7e789005365befd6bb44d55f88d5871607a (self-review by the owner's reviewer agent, Birfy; self-PASS per owner decision 2026-10-19). Not merge authority: T3, waits for the maintainer's GitHub approval.

Tip SHA: f7e6a7e789005365befd6bb44d55f88d5871607a
Ran:
  - ARCH_DOCS_BASE=upstream/main TZ=UTC npm run verify → pass (core 1122/1122; plugins 313, 83, 93, 93, 37, 16; arch-docs OK; build-client --check and ds-bot-patches check ok)
  - red at base: worktree at b6e89d2 + only the test files copied → 7 #540 tests fail; green at tip → pass
  - smoke (dev/bench.sh smoke 3196 8896 reset && node dev/smoke.mjs smoke 3196, DSH 0.2.0-rc.2) at b6236f8 → pass 42/42; f7e6a7e differs from it only by client files merged from main (#502)
  - CI at tip: tests-ok, test, plugins (tree/social/timeline), package → pass
  - lint warnings vs base → no new ones
Not run:
  - smoke at f7e6a7e — host and index.js identical to the smoked b6236f8
  - live run in the owner's FlyBot with a real bash sandbox escalation — needs the owner's instance
Verdict: PASS

Checked, line by line

  • No DSH patch: ds-bot/ untouched; the grant answers through the documented approval/request waterfall (prepend), returning DSH's own allowed-once; DSH's approval/asked / approval/decided audit pair still records each ask.
  • Owner only: always-* needs botOwnedBy in authz (owning the room is not enough, test authz.test.js Approval card has no way to approve permanently (per Bot or per group) #540); buttons never render on a friend's Bot (bot.remote), and the host refuses a grant for a remote Bot.
  • No leak to friends: standingGrant returns false for bot.remote; and the auto-answer requires settled(rt, sessionId) === 'owner' (turn-origin.js), so a friend's remote-chat message, another member's line, a room with other people in it, or a turn with no verdict still gets a card (test "never answers a turn someone other than the owner started"). Found and fixed in this review (commit b6236f8).
  • Scope: Bot grant = that Bot's Sessions, that exact kind (tool + sandbox mode; workspace-write does not inherit danger-full-access). Group grant = Sessions of that room only, Bots of the granting owner (ownerId while authz is enforced; on a host nobody signed in to, every local Bot is the one person's — also fixed in this review, Bots made after the v2 migration carry no ownerId). Not the Bots' own chats.
  • Revocation: standing-approval-remove (Bot: Bot's owner; group: the giver, removes only their own), Remove buttons in Bot and Group settings; tests confirm the next ask shows a card again.
  • Persistence: grants on the Bot / room record, saved to state.json (test reads the file).
  • Approval stays an explicit UI control ([Feature RFC] Agent inbox board: one place for tasks, approvals and failures (personal first, team second) #349): no typed-reply path added.

Residual risks (for the owner)

  • A non-escalation ask is keyed by tool name only, so an Always allow on, e.g., an MCP tool's approval covers every later ask of that tool.
  • A standing grant on danger-full-access lets that Bot run any escalated command in owner-started turns without a prompt; that is the requested behavior, revocable in settings.
  • Pre-existing flake tasks.test.js "untagged hold stamps the latest working task" (unrelated; follow-up).

@Birfy Birfy added the awaiting-approval PR is green and waits for the maintainer's approval (T2/T3) or a review PASS at its tip label Oct 20, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 20, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 20, 2026
@Birfy

Birfy commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Self-review (review-pr, Birfy reviewer agent, 2026-10-20 21:20 Asia/Shanghai): PASS (merge-main conflict fix; no feature change)

Tip SHA: be15d5f
Ran:

  • git merge origin/main (11f80dd, after Show every Bot at work in a group as one row with its step #548 group work rows) into the branch, no rebase or force-push → pass. core/src/client/member-status.js merged without a conflict: Show every Bot at work in a group as one row with its step #548's dock rows (one left row per Bot with its step and time) and this PR's Always allow buttons on ApprovalCard (alwaysChoices) are both present. Real conflicts only in core/test/group-status.test.js and core/test/member-status.test.js (both sides added tests at the same spot; kept all of them unchanged) and the generated files
  • generated files regenerated, not hand-merged: git checkout origin/main -- patches/ds-bot, then this PR's topics.json entry restored by hand (src/host/approvals.js in 0001-core-fixes, next to Show every Bot at work in a group as one row with its step #548's member-status entries; main's file drops it), (cd core && npm run build), node scripts/ds-bot-patches.mjs generate, check → pass (10 patches apply cleanly; core/ = ds-bot/ + patches)
  • ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify (core lint + arch-docs OK with 31 changed files, plugin lint, build-client --check, ds-bot-patches check, all suites) → pass, 0 failures (core 1124, social 313, tree 83, timeline 93, .github scripts 93, interop 37, live-e2e 16)
  • smoke (PR touches core/src/host/): CONTEXT_WINDOW=100000 CHECK_RATIO=0.45 dev/bench.sh smoke544 3396 9096 reset && node dev/smoke.mjs smoke544 3396 against DSH 0.2.0-rc.2 → pass, all 42 steps; bench stopped afterwards
  • CI on this tip → all checks pass (tests-ok, ubuntu-latest · Node 24 · DSH 0.2.0-rc.2, plugins social/timeline/tree, package, risk tier, reviewed tip); MERGEABLE / CLEAN
  • re-read the diff against main at this tip: same 18 source/test/doc files as before the merge plus generated client.js and patches; one changelog.d fragment (540-always-allow-approval.added.md); no CHANGELOG.md, version or ds-bot/ edits; no node_modules links committed
    Not run:
  • live Mac instances / DSH Desktop / real-model live-e2e (no key; mock model only)
  • production Bot Social (not in scope)
    Verdict: PASS

Findings: none to act on. This review does not authorize a merge: risk:T3 still needs the maintainer's approval (the owner queues it).

@Birfy
Birfy added this pull request to the merge queue Oct 20, 2026
Merged via the queue into ThinkFlowLab:main with commit 17af764 Oct 20, 2026
8 checks passed
@Birfy
Birfy deleted the fix/always-allow-approval branch October 20, 2026 13:41
Birfy added a commit to Birfy/FlyBot that referenced this pull request Oct 20, 2026
Generated patches regenerated from the merged core/; topics.json keeps
main's src/host/approvals.js and this branch's src/client/secret-card.js.
Birfy added a commit to Birfy/FlyBot that referenced this pull request Oct 20, 2026
The merge queue run for ThinkFlowLab#551 failed "warnings are kept, cleaned, written
to the log, and read back after a restart" with ENOENT on
diagnostics.jsonl: the log is written in the background, and on a busy
runner the write had not landed after the test's fixed 50 ms sleep.
Neither ThinkFlowLab#551 nor ThinkFlowLab#544 touches diagnostics; a slowed mkdir reproduces the
same ENOENT locally. Poll for both lines (up to 5 s) instead.

Docs-not-needed: test-only timing fix, no behavior change.
Birfy added a commit to Birfy/FlyBot that referenced this pull request Oct 20, 2026
…kFlowLab#551)

A Bot that works on something handed out in a group (a member there
sends it the work with `message_bot`) runs it in a hidden turn of its
own chat. Its `ask_user` card therefore landed in the Bot's private
chat, and an answer there never went back to the group. Now the card
sits in that group's composer dock, named for the Bot that asks. The
answer reaches the Bot as a hidden request from the member that handed
the work over, so the Bot's reply goes back to the group the same way
the work does. The group shows a quiet notice of the answer, and the
Bot's private chat gets no card and no visible turn.

Fixes ThinkFlowLab#550

- `tools.js`: `ask_user` in a turn that answers a group member's request
(`senderRoomId`, not `audience: 'user'`, not a friend's) writes
`state.questions[<room>]` (`header`, `botId`, `replyTo`, the turn's
`origin`, `allowCustom`). Only one card per group: while one is open, a
second Bot is told to ask in its reply. `rt.answerGroupQuestion`
delivers the answer and posts the notice.
- `channel.js` `answer-question`: routes a room card to it.
`delivery.js`: a header for it (`answeredIn`). `turns.js`: a typed group
line no longer clears the group's card.
- `authz.js`: only the asking Bot's owner or the room's owner sees or
answers such a card. A non-owner's answer carries `origin: other`.
- `roster.js`: deleting the room or the Bot drops the card. Face-to-face
rooms use the same path on the local home.
- Docs: `core-host.md`. Changelog fragment
`550-group-questions.fixed.md`.

Not changed: `ask_user` inside a group Session is still refused (the Bot
asks in its reply, which already shows in the group). Secret cards stay
private. `reply_to: "user"` hand-offs are still answered in the Bot's
own chat by design.

## Verification (tip 6dfe7ba)

- The second commit, 6dfe7ba, runs the answerer's display name through
`cleanName` before it goes into the group notice.
- Red at base: 11f80dd with only `test/group-work.test.js` and
`test/authz.test.js` copied in gives 4 fail / 27 pass. The 4 new ThinkFlowLab#550
tests fail and the own-chat guard test passes. At the tip they all pass.
- `ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify` passes: core
1121/1121, social 313, tree 83, timeline 93, scripts 93, interop 37,
live-e2e 16; arch-docs OK; patches check ok. oxlint warnings are 53 at
both base and tip.
- On the first verify run (at 368b4fb), `tasks.test.js` "an untagged
hold stamps the latest working task…" failed once under full-suite load.
It passed 3 of 3 alone and the rerun was fully green. That test uses
millisecond timestamps, and this PR does not touch that code.
- DSH smoke (`dev/bench.sh` + `dev/smoke.mjs`, DSH 0.2.0-rc.2, mock
model) passes all 42 steps at 6dfe7ba.

Not run: Desktop app, a real model, a live two-home face-to-face run,
and a browser check of the group dock rendering. The client is
unchanged: `QuestionDock` already reads `questions[sessionId]` for the
open conversation and shows `header`.

File overlap with open PRs: ThinkFlowLab#544 touches `authz.js`, `channel.js`,
`topics.json`, `core-host.md` and the 0001/0005 patches; ThinkFlowLab#547 touches
`0001-core-fixes.patch`, `topics.json` and `core-host.md`. The source
hunks are in different places. `topics.json` adds
`test/group-work.test.js` to topic 0001.

---------

Co-authored-by: Birfy <15378933+Birfy@users.noreply.github.com>
@Birfy Birfy removed awaiting-approval PR is green and waits for the maintainer's approval (T2/T3) or a review PASS at its tip agent-pr Pull request opened by an agent labels Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:security Auth, grants, approvals, secrets area:ui Web/desktop UI and copy bug Something isn't working risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Approval card has no way to approve permanently (per Bot or per group)

1 participant