Skip to content

Declare DSH_SECRETS_WITHHELD so a withheld turn keeps its shell - #654

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
Birfy:fix-secrets-withheld-undeclared
Oct 22, 2026
Merged

Declare DSH_SECRETS_WITHHELD so a withheld turn keeps its shell#654
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
Birfy:fix-secrets-withheld-undeclared

Conversation

@Birfy

@Birfy Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes #653

Bug. #651 made resolveEnv return DSH_SECRETS_WITHHELD, but the ds-bot-secrets shell-env contribution declared only DSH_SECRET_<NAME> (core/src/host/secrets.js:578-582). DSH's ShellEnvRegistry.collect (@deepseek-ai/dsh-shell-env 0.2.0-rc.2) throws on an undeclared key, and dsh-tool-bash calls it before it runs anything. So every bash call failed on exactly the turns that withhold a key (a shared Bot's scheduled turn, someone else's turn, an unknown turn). The #651 tests missed it because the bench mock called resolve directly.

Change.

  • secrets.js: declare WITHHELD_VAR next to the keys.
  • secrets.test.js: the bench's env() now enforces DSH's declared-key and string-value contract. One new test, and the declared-key list assertion now includes the variable.
  • docs/subsystems/core-host.md: one sentence. changelog.d/653-secrets-withheld-declared.fixed.md.

This does not change which turns get a key; that policy is #601 (needs-human).

Verification (tip ad0c471666da90e4cbc66752288084fb2d61b71d)

Ran:

  • ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify with core/dev/harness from npm run harness → pass (core 1201/1201, plugins all green, arch-docs OK, patches check OK).
  • The same with DSH_HARNESS=/workspace/tools/deepseek-harness-0.2.0-rc.2 → 1189/1193. The 3 failures (at-menu, time-context "a loaded DSH clock…", schedules "a recurring task missed…") are ERR_MODULE_NOT_FOUND for @deepseek-ai/dsh-session-reference/dsh-schedule in that checkout. They fail identically on origin/main a5a887b (environment, see fix-issue §1).
  • Red at base: core/test/secrets.test.js from this branch on an origin/main worktree → 6 fail / 12 pass. On this branch → 18/18.
  • node scripts/ds-bot-patches.mjs generate && check → ok. node .github/scripts/changelog.mjs check → ok.

Not run:

  • Smoke test (dev/bench.sh + dev/smoke.mjs): the change is one declaration in src/host/secrets.js; the bash path through the real ShellEnvRegistry is covered by the contract-enforcing mock only.
  • package CI job checks (pack, typecheck, actionlint).

@Birfy
Declare DSH_SECRETS_WITHHELD to shell-env so a withheld turn keeps it…
ad0c471 
…s shell (ThinkFlowLab#653)

resolveEnv returns DSH_SECRETS_WITHHELD (ThinkFlowLab#651) but the ds-bot-secrets
contribution declared only DSH_SECRET_<NAME>. DSH's ShellEnvRegistry.collect
throws on an undeclared key, so every bash call of a turn that withholds a key
failed. Declare the variable with the keys, and make the test bench's shellEnv
mock enforce the same declared-key contract that hid this.

Fixes ThinkFlowLab#653
@Birfy Birfy added bug Something isn't working agent-pr Pull request opened by an agent area:security Auth, grants, approvals, secrets labels Oct 22, 2026
@Birfy Birfy self-assigned this Oct 22, 2026
@Birfy Birfy added the area:automation Scheduled tasks and automation label Oct 22, 2026
@Birfy Birfy mentioned this pull request Oct 22, 2026
Closed
@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 22, 2026
@Birfy Birfy mentioned this pull request Oct 22, 2026
Merged
@Birfy
Birfy added this pull request to the merge queue Oct 22, 2026
Merged via the queue into ThinkFlowLab:main with commit ee1739e Oct 22, 2026
8 checks passed
@Birfy
Birfy deleted the fix-secrets-withheld-undeclared branch October 22, 2026 00:44
This was referenced Oct 22, 2026
Closed
Merged
Closed
Closed
Merged
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 22, 2026
Birfy added a commit to Birfy/FlyBot that referenced this pull request Oct 23, 2026
@Birfy
Give the owner's keys to every turn of the owner's own Bots (ThinkFlo…
4f6e3e5 
…wLab#656)

Fixes ThinkFlowLab#601

Rebased onto `main` after ThinkFlowLab#654 merged (`ee1739e`). Two commits:
`480c22d` (secrets) and `fb70b22` (connectors + GitHub guidance).

**Owner decisions** (recorded on ThinkFlowLab#601):
- 2026-10-22 08:32 Asia/Shanghai: 「放开这个权限管理，我的bot都应该能看到」. Every one of
the owner's own Bots gets the owner's saved keys on every turn, wherever
the turn came from.
- 2026-10-22 08:42 Asia/Shanghai: 「放开，主要应该用连接器工具」. Connector tools
follow the same rule, and Bots should use them first for GitHub work.

The boundary that stays: a friend's Bot or a remote home never gets the
keys or the connector tools. A per-key Chosen Bots scope and a per-Bot
connector switch are still respected.

## What changed
**Secrets (`core/src/host/secrets.js`)**
- `withheldReason` and `ownersTurn` are removed, and secrets no longer
import `turn-origin.js`. `resolveEnv` hands out every key in the Bot's
scope on any turn: group rounds of any origin, relays, scheduled turns,
shared Bots' turns, and turns with no recorded origin.
- A sub-agent's shell follows its parent chain to its Bot through
`rt.whoseAgent`, the same chain the connector guard uses.
- `DSH_SECRETS_WITHHELD` now names only the real boundaries:
`remote-bot`, `not-in-scope` and `unknown-bot`.
- `list_secrets` and `request_secret` no longer say a key is "withheld
this turn".
- `canUseSecret` (capabilities) follows the same rule.

**Connectors (`core/src/host/connector-access.js`, `turns.js`, client)**
- The guard no longer reads the turn's origin. Every turn of the owner's
own Bots may use the connector tools (`mcp__github__*`,
`mcp__composio__*`), including sub-agents, which resolve to their
parent. A friend's Bot gets none, and a Bot whose connector the owner
turned off gets none.
- The "also when others use it" switch is removed, along with
`connectorsShared`, the sharing-follow machinery, `connectorTurnChanged`
and its calls in `turns.js`. This removes the most dead code.
  - `setBotConnector` ignores a `scope` sent by an older client.
  - A legacy `connectorsShared` flag is dropped.
- The UI (`connector-bots.js`) shows one switch per Bot, and the related
i18n strings are removed or reworded.

**GitHub guidance (`core/src/host/prompt.js`)**
- One new line: use the GitHub connector tools first, and
`$DSH_SECRET_GITHUB_TOKEN` with gh/git only as the fallback (for example
`git push`).

**Docs**
- `AGENTS.md` (hard rules 2 and 4)
- `docs/subsystems/core-host.md`, `team.md`, `core-client.md`
- `README.md` and `README.zh.md`
- `changelog.d/601-secrets-every-turn.changed.md`

**Tests**
- `secrets.test.js` and `capabilities.test.js` cover:
  - a group round with origin `other`
  - a turn someone else started
  - a shared Bot's scheduled turn
  - a shared Bot outside a known turn
  - sub-agent bash, plus `unknown-bot` when the parent chain is lost
  - Chosen Bots scope
  - a friend's Bot getting `remote-bot`
- `connector-access.test.js` covers:
  - every turn origin, including a friend in the room
  - turning a connector off and on for one Bot
  - a sub-agent following its parent
  - legacy `connectorsShared` and `scope` being dropped
  - disconnecting a connector
  - a new Bot or a copy starting on
  - a friend's Bot getting nothing
- `connector-bots.test.js`: the single-switch UI.

## Left open
- **The trade-off the owner accepted:** a friend talking to a shared
Bot, or anyone in a group round, can now steer a turn that holds the
owner's key and the GitHub connector.
- `turnOrigin` is still used by standing approvals and resumed turns.
The optional cleanup was skipped.

## Verification (head `fb70b22e26565be67f2498caeb43b09d3b648767`,
rebased on main `ee1739e`)
- `ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify` → **pass**: core
1201/1201, plugin suites 317/89/104/132/37/16, arch-docs OK on 29 files,
patches check ok, and 85 lint warnings (unchanged).
- Smoke: `dev/bench.sh … reset && node dev/smoke.mjs` with
deepseek-harness 0.2.0-rc.2 → **all 42 steps passed** on the rebased
head.
- Red at base:
- the new `secrets`/`capabilities` tests on ThinkFlowLab#654's head → 12 fail,
including all 7 `ThinkFlowLab#601` cases.
- the new `connector-access`/`connector-bots` tests against `480c22d`'s
`src` → 8 fail / 13 pass.
- Not run: the CI `package` job, the desktop app, and a real-model or
real DSH sub-agent run.
Birfy added a commit to Birfy/FlyBot that referenced this pull request Oct 23, 2026
@Birfy
Judge a merge-queue entry at its pull request head, not at the queue …
c74e120 
…base (ThinkFlowLab#658)

Fixes ThinkFlowLab#657

The `<sha>` in `gh-readonly-queue/<base>/pr-<N>-<sha>` is the entry's
**base** (`merge_group.base_sha`), not the pull request head.
`merge-gate.mjs` read it as the head (`merge-gate.mjs:33-35`, `:44`,
`:89`, `:95-96`, `:173` on main), so every `merge_group` run refused —
run 39701316520 (ThinkFlowLab#654): "head moved to ad0c471 after this entry was
built at d105f22; no Verifier report names the tip d105f22".
d105f22 was main's tip, ad0c471 ThinkFlowLab#654's head, ee1739e the entry commit.

Queue mode now judges the pull request's live head exactly as the
`pull_request` report does. The base (`BASE_SHA`) and entry
(`ENTRY_SHA`) are passed by the workflow and only logged. Policy
unchanged: `hold` refuses; T2/T3 need a PASS naming the PR head (a PASS
naming the base or the entry commit refuses); T0/T1/release are computed
from the diff. `--sha` on a hand run still refuses a different live
head. Also fixes the report text that said "null's PASS" for an exempt
PR. No changelog fragment: CI-only, not user-visible (as ThinkFlowLab#640).

## Verification at 4654707
- `ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify` → pass (core
1201/1201, social 317, tree 89, timeline 104, `.github/scripts` 137,
interop 37, live-e2e unit 16; 0 fail).
- `node --test .github/scripts/merge-gate.test.mjs` → 26/26 pass. Same
test file against main's script → 8 fail (incl. the run-39701316520
replay through `main()`).
- Not run: a real `merge_group` run (only the queue can produce one);
smoke (no host code changed).

## Landing this PR (T3: `.github/`)
The queue runs **main's old** `base/.github/scripts/merge-gate.mjs`, so
this PR's own entry will refuse one last time. `merge-gate` is **not** a
required check (the `main merge queue` ruleset requires only `tests-ok`
and `ubuntu-latest · Node 24 · DSH 0.2.0-rc.2`), so the queue still
merges it once those are green: after a self-PASS at the tip, `gh pr
merge <n> --auto --squash --match-head-commit <tip>` as usual. No
bypass, no admin-merge, no ruleset change needed. Entries after this one
use the new script. Only add `merge-gate` to the required checks after a
green `merge_group` run of the new script.

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

Reviewers

No reviews

Assignees

@Birfy Birfy

Labels

area:automation Scheduled tasks and automation area:security Auth, grants, approvals, secrets bug Something isn't working risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Secrets: a turn that withholds a key fails every bash call (DSH_SECRETS_WITHHELD is not declared to shell-env)

1 participant

@Birfy