Repository navigation
Connector tools on by default for your own Bots; sharing a Bot doesn't take them away; others need your opt-in - #370
Conversation
…them for you; others need your opt-in (#369)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3db8c05ccc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // (no running turn found, or a sender that is not a local Bot) passes on nothing. | ||
| const originOf = ({ from, to, fromId, remote, given }) => { | ||
| if (remote?.handle) return OTHER | ||
| if (fromId === undefined) return given === OTHER ? OTHER : OWNER |
There was a problem hiding this comment.
When a friend-driven Main/admin Bot calls post_to_group for a room containing only the owner's Bots, tools.js and groups.js do not carry that turn's origin, and room-participant.js later calls deliver with no fromId; this branch therefore stamps each member request as OWNER. Because settled sees no other occupants in that room, those member turns can use owner-only connectors even when shared use is disabled. Propagate the originating turn verdict through the post, round, and member deliveries instead of defaulting this path to the owner.
Useful? React with 👍 / 👎.
| const now = sharingOf(bot.id) | ||
| const before = sharedBefore.get(bot.id) | ||
| if (before !== undefined && before !== now) changed.push(bot.id) | ||
| sharedBefore.set(bot.id, now) |
There was a problem hiding this comment.
The refresh key is only the display-label list returned by sharedWith, so distinct sharing states can compare equal. For example, if Writer already has a grant named Bob and a Bob participant leaves Writer's room during a turn that began while Bob was present, sharingOf remains "Bob"; no refresh runs, and the connector tools stay masked for the rest of what is now an owner-only turn even though the call guard allows them. Track the room-sharing predicate or membership identity separately rather than using the deduplicated UI labels as the change token.
Useful? React with 👍 / 👎.
…re room membership, not labels (#369 review)
|
Review of #370 (comment only, not an approval). Reviewed at 3fecb86; fixes pushed as f2e72f7, so any earlier PASS no longer applies to the new head. Fixed in f2e72f7
Docs ( Checked, no bug found
Remaining limits (documented, not fixed here)
No conflicts with other open PRs. #367 and #368 are already merged into this branch, and the only other open PR is the release PR #152. This is still T3: not merged, not deployed. Needs a maintainer decision. |
|
Update: head is now e34e7a1 (f2e72f7's fixes, a test-timing fix, and main merged through #372). Earlier CI failure was my new group-round test waiting only a few event-loop turns on a slower runner; it now waits in real time. The #297 presence test failed once on the social job and passed on rerun. All checks are green. Any earlier PASS no longer applies to this head. |
…owLab#547) Fixes ThinkFlowLab#545 Owner decision 2026-10-20 20:21 (Asia/Shanghai), per ThinkFlowLab#359 / AGENTS.md hard rule 4: the owner's connectors and secrets are on by default for all of the owner's own Bots. **Root cause.** `connector-connect` (`core/src/host/connectors.js`) saved `GITHUB_TOKEN` / `COMPOSIO_API_KEY` with `scope: []` on a first connect, so the Secrets page and `list_secrets` showed "No Bot" and no Bot's shell got the token. An empty scope list could not be told apart from an owner's choice. **Change.** - `connectors.js`: no `scope: []`; a connector key gets the normal default (all Bots), a reconnect keeps the existing scope. - `secrets.js`: a scope the owner chose is stored with `scopeSet: true` (Secrets page `secret-update` / `secret-set` with a scope, a card's "Only <Bot>", a deleted Bot leaving a picked list). On load, an empty list **without** `scopeSet` (the old connector default) becomes `'all'` once and is written back by the holding host. Pre-ThinkFlowLab#545 empty lists the owner picked by hand carry no marker and are widened too (not distinguishable). - Secrets page (`secret-card.js`): explicit three-way control **All my Bots / Chosen Bots / No Bot**; the row tag says "All my Bots". zh strings added. - Owner's use only (self-review, second commit): a shell gets keys only on the owner's own turn (`settled` in `turn-origin.js`), and outside a known turn only while the Bot is shared with nobody; a friend's Bot never. A capability (`canUseSecret`) on a Bot shared with anyone gets only a key whose list names that Bot, never an `'all'` key — otherwise a friend's turn could have a shared Bot save an HTTP capability that sends `$DSH_SECRET_GITHUB_TOKEN` elsewhere. - Connector *tools* were already on by default for the owner's own turns (ThinkFlowLab#369/ThinkFlowLab#370), so `connector-access.js` is unchanged. - Docs: core-host "Secret scope defaults (ThinkFlowLab#545)", core-client "Secrets page scope control (ThinkFlowLab#545)". Fragment `changelog.d/545-secrets-all-my-bots.fixed.md`. **Risk tier:** touches `core/src/host/secrets.js` (secret exposure) → expect T2/T3; see the controller's label. ## Verification (tip `2f484fe`) - Red at base `b6e89d2` (tests only): `connectors.test.js` 2 fail (scope `[]` ≠ `'all'`), `secrets.test.js` 2 new fail (migration, explicit No Bot), `tool-cells.test.js` fails (no three-way `ScopeEditor`). Red at `c0d189f` (first commit): the shared-Bot shell test and the shared-Bot capability test. - `DSH_HARNESS=core/dev/harness ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify`: exit 0 — core 1117/1117, social 312/312, tree 83/83, timeline 93/93, .github scripts 93/93, interop 37/37, live-e2e 16/16; lint + arch-docs + `build-client --check` + `ds-bot-patches check` ok. - Smoke: `dev/bench.sh smoke545 3196 8896 reset && node dev/smoke.mjs smoke545 3196` against deepseek-harness `dsh-v0.2.0-rc.2`: all 42 steps passed. (Bench trap: pnpm 11 did not link `ds-bot` into the bench profile; linked `profiles/web/node_modules/ds-bot` by hand.) - CI at `2f484fe`: `tests-ok` green. **Not run:** Desktop app; real-model run; the owner's running instance. --------- Co-authored-by: Birfy <15378933+Birfy@users.noreply.github.com>
Fixes #369
Supersedes part of #346 (the "off by default, never on a shared Bot" rule); builds on #335. Owner's decision, recorded in
docs/subsystems/core-host.md: connected connectors should be on by default for the owner, a shared Bot must still work for its owner, and the default-off applies to other people, who the owner can opt in per Bot.What changed
bot.connectors = { x?: false }= explicit off for the owner's own use (missing or legacytrue= on).bot.connectorsShared = { x?: true }= explicit on for others' use (default off; only counts while the owner's own use is on).bot-connectorstakesscope: 'owner' | 'shared'; turning owner use off drops the shared offer, and turning shared on while owner is off is refused. Sharing no longer clears or locks anything (setGrantsno longer deletes the flags).core/src/host/turn-origin.js, tracked inturns.js), from the messages the turn took in, never from the Bot's own words. The turn's verdict is the strictest over its messages and sticky for the turn.deliverstampssource.origin,message_bot/stop_botpass the caller's verdict);otherturn;bot.remote-> other / no tools.parts.js) and resume records (resume.js, inflight) carry the verdict; a record without one resumes asother.settled ?? OTHER); the tool-list mask is optimistic for idle Sessions and refreshed on turn start / extra message / turn end, switch changes, sharing changes andtools/change.falsestays off. Disconnect clears both flags for that connector on every Bot. The session URL still is not stored instate.json(id only).<name>: also when others use it" (zh "别人使用时也开启"), never disabled by sharing, greyed only while the owner's own switch is off ("Turn it on for yourself first."). Hints name who it is shared with ("Shared with Bob. They cannot use it unless you turn this on."). Footer explains default on for you / off for others, including rooms with other people. All zh strings added; old strings removed.core-host.md(decision + rule),core-client.md,team.md, README en/zh; changelog fragment369-connectors-default-on.changed.md; the unreleased Connector tools (Composio, GitHub) reach every Bot, including Bots shared under a per-Bot grant; make them per-Bot opt-in #335 fragment trimmed so it no longer contradicts.core/andpatches/ds-bot/regenerated together (resume.js,work-dir.test.jsadded to topic 0001).Tests
connector-access.test.js(18) andconnector-bots.test.js(5); newturn-origin.test.js(3); new part-switch test inwork-dir.test.js.core/src(19 failures there, pass on this branch).ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify(Node 24): all green, arch-docs OK, ds-bot-patches check OK.Known limits
Review notes
T3 (touches authz/reach). Not merged, not deployed, nothing run on the Mac. Needs maintainer approval; please review
turn-origin.jsand the guard inconnector-access.jsfirst.