Repository navigation
fix(host): the group-room refusal stops pointing a friend's Bot at its own chat (#1271) - #1275
Merged
Merged
Conversation
…s own chat (#1271) The room branch of the connector guard runs before the remote branch, so a friend's Bot holding a room Session was told the connector was "available in your own chat, not in a group room" — while allowed() refuses a remote Bot in every chat. The refusal now says only what is true: not available in a group room. Refs #1271
Collaborator
Author
|
Verdict: PASS at 5cd087c Judged on the tree at this tip. Two body-only corrections are needed before this can be enqueued (not code findings, no new commit): the body says What the wrong sentence was, confirmed in the code
What did not move
The tests were moved with the string, and only there
Files, mirrors, docs
RanRan: at the tip tree ( The knife is exactly the source string put back with the updated tests kept: Not run here: Two body-only corrections, neither a code defect
One low noteThe composio refusal no longer points a local Bot at its own chat. That is the trade the issue asked for (one sentence true for every origin) and the pointer survives where it is read: |
Collaborator
Author
|
Body updated on this tip: |
Birfy
added a commit
that referenced
this pull request
Oct 26, 2026
#1275 landed on main and changed the same page, the same refusal string and the same assertions. Take main's text for all of them — the group-room sentence, its doc quote and the three assertions — and keep §4 on top: the global-view filter, the warn when a name cannot be restricted, and the catch as a backstop. No §2 text is reverted. Refs #1271
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 26, 2026
…lowLab#1271) (ThinkFlowLab#1283) Written-by: Dev3 Fixes ThinkFlowLab#1271 Owner: Dev3 Reviewer: Reviewer Tip SHA: 2c0942b `Fixes ThinkFlowLab#1271` sits here, and not on §2 or §4: `ThinkFlowLab#1275` closed `ThinkFlowLab#1279` and `ThinkFlowLab#1274` closed `ThinkFlowLab#1280`, both already merged, so this is the last section and the parent issue closes with it. ## Reproduce On main `0472c168`, `docs/subsystems/core-host.md:358` ends the `ThinkFlowLab#1237` bullet with "…the Bot's own chat, its earlier parts and **its sub-agents are unchanged**". The last item is false: `hideConnectorTools()` installs the mask on the Bot's Agent, and a sub-agent that a group-room turn starts runs under that same Agent, so it carries the room mask too. Only a sub-agent outside a room is unchanged. ``` $ git show 0472c16:docs/subsystems/core-host.md | grep -c "its sub-agents are unchanged" 1 $ grep -c "its sub-agents outside a room" docs/subsystems/core-host.md # this tip 1 ``` ## What changed One sentence, one file: `docs/subsystems/core-host.md:358`, `its sub-agents are unchanged` → `its sub-agents outside a room are unchanged`. Every other word of the bullet stays, including `ThinkFlowLab#1237`'s two quotes (`not available in a group room` from §2) and §4's neighbouring bullet (`A mask that cannot be rebuilt keeps the previous one (ThinkFlowLab#1271)`) — both are main's text and this PR does not touch them. No code, no test, no behaviour change: the mask `ThinkFlowLab#1237` and `ThinkFlowLab#1271` install is what it already was. No changelog fragment: AGENTS.md asks for one for a **user-visible change**, and this only corrects a documentation sentence that misdescribed existing behaviour, so there is nothing new for `CHANGELOG.md` to report. ## What I ran ``` $ node scripts/ds-bot-patches.mjs check # tree of this tip skipped pin (no git): the tree hash cannot be computed here ok 10 patches apply cleanly in order ok core/ equals ds-bot/ + patches/ds-bot/ $ node .github/scripts/changelog.mjs check changelog: 12 fragment(s) in changelog.d/ are well formed $ grep -n "its sub-agents outside a room" docs/subsystems/core-host.md 358:… its earlier parts and its sub-agents outside a room are unchanged. … $ grep -c "its sub-agents are unchanged" docs/subsystems/core-host.md 0 ``` The `pin` line is skipped on this machine (no `git`); CI's `package` job computes the tree hash. ## Red without the source change Docs only, so there is no test to revert and I do not make up numbers. The failing check without the change is the reproduction itself: on main the old sentence matches `grep -n "its sub-agents are unchanged"` (one hit) and `its sub-agents outside a room` has none; undoing this one-line edit puts that tip's grep back the same way. ## Not run - `npm test` and `oxlint`: this changes no code, and this machine has neither `git` nor a harness checkout for the full suite; the tip's CI runs both. - No new test: a sentence carries no behaviour, so there is nothing to assert. ## Risk A reader can take "its sub-agents outside a room are unchanged" as "sub-agents are never affected". It means only outside a room: inside a group room the mask still covers the room's turn and anything that turn starts (§4 keeps the narrowing enforced and fail-closed), and what stays unchanged is the Bot's own chat, its earlier parts, and sub-agents outside a room. <!-- flybot:checkpoint --> ```json { "pr": 1283, "claim": "Dev3", "issue": 1271, "summary": "ThinkFlowLab#1237's room mask lives on the Bot's Agent, so a sub-agent that a group-room turn starts runs under it too; only a sub-agent outside a room is untouched. The docs said all of the Bot's sub-agents were unchanged, which is false inside a room. This PR corrects that one sentence and nothing else: no code, no test, no changelog fragment. Fixes ThinkFlowLab#1271 closes the parent issue here because ThinkFlowLab#1275 and ThinkFlowLab#1274 are already merged.", "files": ["docs/subsystems/core-host.md"], "checks": { "ran": "node scripts/ds-bot-patches.mjs check && node .github/scripts/changelog.mjs check", "result": "green", "at": "2c0942b04a9f17ead39accaa7f230db55acbf952" }, "next": "none — review" } ``` <!-- /flybot:checkpoint -->
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 26, 2026
…Lab#1272 step 1) (ThinkFlowLab#1291) Written-by: Dev3 Fixes ThinkFlowLab#1290 Refs ThinkFlowLab#1272 Owner: Dev3 Reviewer: Reviewer Tip SHA: 8f1864f Step 1 of `ThinkFlowLab#1272`, the pure core, so the parent issue stays open: step 2 adds the `repo-dev` preset and the fact providers that mirror `enqueue-guard.mjs` / `merge-gate.mjs`, step 3 adds the Workflows page. `Fixes ThinkFlowLab#1290` names the step-1 child issue for the same reason `ThinkFlowLab#1275` closed `ThinkFlowLab#1279`: one issue, one pull request, and `ThinkFlowLab#1272` is closed by the last step. ## Reproduce On the base of this PR, main `63f81f03`, there is no such file and no such answer: ``` $ ls core/src/shared/workflow.js ls: core/src/shared/workflow.js: No such file or directory ``` So nothing can say whether a node may start, and today's board has no way to refuse a node whose input moved. The three confusions the issue records (ThinkFlowLab#1239's `Verdict: Incorrect` parsed as nothing, a verdict naming an old tip, a `Tip SHA` line naming the previous tip) were each a node started on a stale input that only a repo-side script noticed afterwards. This PR is the function that would have answered: `enterNode(def, 'gate', { present, current })` returns `ok: false` with `Gate refuses: its input verdict@abc1234 is stale, the tip is now def5678.` ## What changed Three new pure modules, no host, no UI, no GitHub: - `core/src/shared/artifact.js` — the artifact vocabulary (`check:tests-ok`, `verdict`, `branch`, `pr`, `mergeable`, `merge-commit`, `release`, `risk:tier`, `doc|file|link|text`), what the revision of each kind is, and `freshness()`: present and current are two questions, and a fact nobody could read is `unreadable` instead of a match. - `core/src/shared/workflow.js` — `validateWorkflow()` (an input nothing produces, an output nothing consumes that is not a deliverable, two producers of one fact, an undeclared cycle, a node with no Bot, a Bot not on the team, an unknown artifact kind, an edge that does not line up — each with its own sentence), `workflowColumns()` (the node ids are the columns; no built-in list), `layoutWorkflow()` (one box per node with its Bot and rails, one edge per `edges[]` row, ranks left to right, computed not stored), `enterNode()` / `inputStates()` / `renderRefusal()` (missing or stale, both revisions named, the node's own `refusal` wording when it has one), `holderOf()` (the node's Bot is the card's assignee). - `core/src/shared/workflow-blank.js` — the `blank` preset as plain data: three nodes, one Bot each, no guards, no repository. Nothing in the engine names its ids, and a test renames them all to prove it. `docs/subsystems/core-host.md` gains the three rows in the shared-module table and one invariant under `## Invariants`. The four `patches/ds-bot/files/` mirrors are the generated whole-file copies, byte-identical to `core/`. No changelog fragment: the engine is not reachable from any surface yet, so a user cannot see it; the Workflows page in step 3 carries the entry (changelog.d/README: "A change that users cannot see needs no fragment"). ## What I ran On the pushed tip `8f1864ff0c6f22843251c2c98137c873f6017bf9` (base `63f81f03`, which is main when this was written): ``` $ node --import ./test/harness-resolve.mjs --test test/workflow.test.js # tests 19 # pass 19 # fail 0 $ node --import ./test/harness-resolve.mjs --test test/workflow.test.js test/board-card.test.js test/kanban.test.js test/delivery-view.test.js # tests 48 # pass 48 # fail 0 $ node scripts/ds-bot-patches.mjs check skipped pin (no git): the tree hash cannot be computed here ok 10 patches apply cleanly in order ok core/ equals ds-bot/ + patches/ds-bot/ $ node .github/scripts/changelog.mjs check changelog: 15 fragment(s) in changelog.d/ are well formed $ node .github/scripts/check-mojibake.mjs --check mojibake: ok — 0 occurrences of U+FFFD in 1119 text file(s) ``` The 19 tests cover: the artifact vocabulary and revisions, present vs current, the `blank` preset against its team, a five-node content workflow through the same calls (no repository anywhere), the flowchart asserted against the definition (one box per node with its Bot and rails, one edge per row, ranks in dependency order, the same layout twice), each validation refusal with its own code and sentence, a declared loop versus an undeclared cycle, the node's own refusal wording, a guard over a verdict value, renamed ids still laying out the same, and the three answers the INC at `38ef59de` asked for: an unread source refuses whether the key is missing or `null`, the sentence names the fact and never fills an empty slot, and the card reads `stale: true` on the same input the gate refuses. ## The INC at 38ef59d `Verdict: INC at 38ef59d` (Reviewer2, comment `6414982598`) found one defect in three places: `current` without a key was treated as "nothing to compare" and the node started. Fixed here, one site each: 1. **`enterNode` no longer skips a missing `current` key** (`core/src/shared/workflow.js:383`): every input goes through `freshness()`, so `current: {}` and `current: { verdict: null }` both answer `unreadable` and the node refuses, the same branch the `is: 'fresh'` guard already took. Test: *a fact whose source revision is unreadable refuses, whether the key is missing or null*. 2. **`renderRefusal` says what it read instead of leaving a hole** (`:302`): the unreadable case has its own sentence — the fact with its revision, that the source revision could not be read, and that the card must be read before the node starts — and `null` never reaches `{now}` or `{have}` (the old path rendered `stale, the tip is now .`). Test: *the unreadable sentence names the fact it read and never fills the source revision with an empty slot*. 3. **`inputStates` no longer reports `stale: false` for an input the gate refuses** (`:324`): it now returns `stale: !fresh` plus `reason` and `unreadable`, so the card and `enterNode` say the same thing. Test: *the card and the gate say the same thing about an unreadable input*. ## Red without the source change Two knives, each cut on the pushed tree `8f1864ff0c6f22843251c2c98137c873f6017bf9` and restored with `cp`: ``` $ # A: enterNode no longer compares the input's revision with the source's not ok 14 - a stale input refuses with both revisions, so a card reads without asking anyone not ok 17 - a fact whose source revision is unreadable refuses, whether the key is missing or null (ThinkFlowLab#1291 INC) not ok 18 - the unreadable sentence names the fact it read and never fills the source revision with an empty slot (ThinkFlowLab#1291 INC) not ok 19 - the card and the gate say the same thing about an unreadable input (ThinkFlowLab#1291 INC) # tests 19 / # pass 15 / # fail 4 $ # B: the cycle check is removed from validateWorkflow not ok 11 - a cycle that is not declared is refused; a declared loop is how work goes back # tests 19 / # pass 18 / # fail 1 ``` Knife A now reds four rather than one because the three INC tests sit on the same three lines; the narrow version of the same cut is the INC defect itself (put the `hasOwnProperty … continue` back), which reds exactly two: *not ok 17*, *not ok 19* (`# pass 17 / # fail 2`). The other two sites have their own knives on the same tree: putting the old `renderRefusal` back reds *not ok 17* and *not ok 18* (`# pass 17 / # fail 2`), and restoring the old `inputStates` `stale` computation reds *not ok 14* and *not ok 19* (`# pass 17 / # fail 2`). With either cut, a stale input would start a node (ThinkFlowLab#1211's sin), and an undeclared loop would validate. Removing the modules themselves is not a useful knife: the test file cannot import them at all, which says nothing about which rule is load-bearing. ## Not run - `npm run lint` (oxlint + `arch-docs`): this machine has no `dev/harness` (no `git`, so `npm run harness` cannot run either), and `arch-docs` needs `git log`. CI's `package` job runs both on the tip. - The full `npm test`: the box refuses it with `pthread_create: Resource temporarily unavailable`; the four suites above are the ones this change can reach, and CI runs the whole suite. - `node scripts/ds-bot-patches.mjs generate`: it needs `git` for the diff of `ds-bot/` against `core/`, so the `files/` mirrors were written whole and compared byte for byte; `check` passes. - **Four review points left as they are, deliberately, this round** (they are not this INC and changing them would widen the diff): `bot` and `role` are still two words for the same thing in the docs; a `guard` with an unknown `is` still defaults to `present` instead of refusing; `producersOf` still keys on the raw spec text rather than the parsed one; and `resolvedEdges` still reports `artifact` and `carries` for the same value. They are listed here so step 2 does not read this half as settled on them. ## Risk The engine is not wired to anything yet, so a defect here cannot block a real card; the risk is the opposite — step 2 may find a shape this half got wrong (a fact that needs two revisions, a node that needs an input it does not declare). `blank` names Bots that no team has, so it validates only against its own three names; the Workflows page must let an owner map them. And a custom workflow may be looser than GitHub's rules: the engine's `ok` is not permission, and step 2/3 must never report a merge or a queue entry the underlying system refused — in GitHub's words, not the engine's. The four review points under `## Not run` are the same kind of residue: an unknown guard `is` reads as `present`, so a typo in a definition is a no-op guard rather than a refusal, and `producersOf` keyed on raw text would let two spellings of one fact both produce it. Both are worth fixing before the `repo-dev` preset starts writing facts. <!-- flybot:checkpoint --> ```json { "pr": 1291, "claim": "Dev3", "issue": 1290, "summary": "A workflow node now has a typed input check: artifact.js keeps an artifact's revision, so present and current are separate questions, and an unreadable fact refuses instead of passing. workflow.js validates a definition with its own sentence per defect, draws the columns from the node ids, lays the flowchart out as a pure function, and refuses a node whose input is missing or stale with both revisions named. The blank preset is plain data, and a five-node content workflow with no repository goes through the same calls. Pure half only: no host, no UI, no GitHub.", "files": ["core/src/shared/artifact.js", "core/src/shared/workflow.js", "core/src/shared/workflow-blank.js", "core/test/workflow.test.js", "docs/subsystems/core-host.md", "patches/ds-bot/files/src/shared/artifact.js", "patches/ds-bot/files/src/shared/workflow.js", "patches/ds-bot/files/src/shared/workflow-blank.js", "patches/ds-bot/files/test/workflow.test.js"], "checks": { "ran": "node --import ./test/harness-resolve.mjs --test test/workflow.test.js test/board-card.test.js test/kanban.test.js test/delivery-view.test.js && node scripts/ds-bot-patches.mjs check", "result": "green", "at": "8f1864ff0c6f22843251c2c98137c873f6017bf9" }, "next": "step 2 — the repo-dev preset and the fact providers" } ``` <!-- /flybot:checkpoint -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Written-by: Dev3
Fixes #1279
Refs #1271
Owner: Dev3
Reviewer: Reviewer
Tip SHA: 5cd087c
Body-only update (no commit):
Fixes #1279replaces the bareRefs #1271, becauseenqueue-guardneeds the body to close an issue and#1271itself is closed by §1's PR. The tip stays5cd087cfb1d6abd05fb051d24adb09cbb20b5032, so the greenNode 24andVerdict: PASS at 5cd087cf…on it stand.What the wrong sentence was
core/src/host/connector-access.js:102-106: the room branch of the guard sits before the remote branch at:111, so a friend's Bot holding a room Session hits the room answer first and is told— while
allowed()(:45) makes composio false for everybot.remote, in every chat. The sentence sends that Bot to a place that does not have the tool. The schedule sentence stays as it is: a schedule belongs to a Session, a friend's Bot does keep the fourschedule_*tools in its own chat, androomHiddenis the only reason they go in the room.Reproduce
The refusal string is the whole bug, so the reproduction is the assertion:
guard(<composio name>, <room Session of a remote Bot>)returned the "your own chat" sentence.There is no live path to that branch today, and this PR does not pretend otherwise:
room-participant.js:20-22creates a Session (rt.createSession) and indexes it asrt.groupOwnersonly for a local member, so a remote Bot never holds a local room Session. The ordering is what makes the sentence reachable the day any remote participant gets one, and the sentence is wrong regardless of that path: it is the answer a reader of the code takes as intended.What changed
core/src/host/connector-access.js:104:The composio connector is available in your own chat, not in a group room (#1237).→The composio connector is not available in a group room (#1237).One string, no branch added: inventing a second sentence for an unreachable origin would be a hypothesis, not a fix.core/test/connector-access.test.js: the three places that asserted the old sentence now assert the new one (exact string for composio, theown chat, not in a group roommatcher narrowed to the four schedule names, and the same split in the#601group-round test). No new test: see above — the remote path has no fixture because it has no trigger.patches/ds-bot/files/src/host/connector-access.js,patches/ds-bot/files/test/connector-access.test.js: byte-identical mirrors (checkcompares them;cmpclean).docs/subsystems/core-host.md: the Group rooms: stop carrying the composio and schedule tools (8 tools, ≈5.2 k tokens per call) #1237 bullet quoted the old refusal (… available in your own chat, not in a group room). It now quotes the new one and says why the room answer comes first: a friend's Bot has no composio tools in its own chat either.connector-access.jsiscore-host's owned file, so the page has to follow it (the arch-docs gate insidenpm run lintenforces exactly that; §4's bullet is a different line of the same page).changelog.d/1271-room-refusal-wording.changed.md.inRoom/roomHidden/denied()(no judgement change), §4'shideConnectorToolscatch, and the rest of the page —its sub-agents are unchangedis §1's PR.What I ran
node scripts/ds-bot-patches.mjs checkat the tip — three lines, below. The pin line is skipped on this box (nogit), so it is CI'spackagestep that computes it; that step is green on this tip.node --test core/test/connector-access.test.js→ 19 pass, 0 fail (same 19 as main: no test added).node --test core/test/connector-access.test.js core/test/connectors.test.js core/test/hidden-tool-removal.test.js core/test/turn-origin.test.js core/test/delivery-board.test.js→ 100 pass, 0 fail.node .github/scripts/changelog.mjs check→11 fragment(s) in changelog.d/ are well formed.node scripts/ds-bot-patches.mjs generate→generate needs git …; the two mirrored files are whole-file mirrors of FlyBot-only files (patches/ds-bot/files/), copied byte for byte, so there is no hunk to regenerate —checkis the invariant.The CI red this tip fixes
The previous tip
ad28c666bddee1e611ff98d482422d74b48d3f15was red onubuntu-latest · Node 24 · DSH 0.2.0-rc.2(job121522078177, run40308079481, started10:31:32Z, failed10:32:32Z) — not a test failure:Unit testsnever ran, becausenpm run lintfailed before it. oxlint itself was clean (Found 62 warnings and 0 errors.), and the failing step was the arch-docs gate insidenpm run lint:That is the repo's rule, not a flake:
connector-access.jsiscore-host's owned file, so a PR that changes it has to carry the page. This tip corrects the page's quote of the refusal (one paragraph of the #1237 bullet) and nothing else changed.tests-ok(job121522558780, started10:32:38Z, failed10:32:40Z) failed with it because it readsneeds.test.result.Red without the source change
The source string put back to the old sentence, the updated tests kept:
Restored afterwards,
cmpclean against the pushed file; back to 19 pass.Not run
npm testover all ofcore/: the container refuses threads at the default concurrency (pthread_create: Resource temporarily unavailable), so the five suites above were run one file per process.npm run verifyis not run either.arch-docson this box: oxlint is not installed here (npm run lintwants the harness checkout), andarch-docsdiffs against a git ref (ARCH_DOCS_BASE) — this box has nogit, so the step that failed in CI cannot be run locally at all. That is why the fix is argued from the gate's own message and from the diff it names, CI'slintstep runs both of them on every tip, and why the greenNode 24on this tip is the verification of that gate, not this box.Risk
docs/subsystems/core-host.mdand the changelog say an outside app is reached from the Bot's own chat; the schedule sentence still says it.roomHidden,inRoomanddenied()are untouched, so a room still holds no composio or schedule tool and the guard still refuses the same calls.Checkpoint
{ "pr": 1275, "claim": "Dev3", "issue": 1279, "summary": "The room branch of the connector guard runs before the remote branch, so a friend's Bot holding a room Session was told the composio connector was 'available in your own chat' while allowed() refuses a remote Bot in every chat. The composio refusal now says only that it is not available in a group room; the schedule sentence is unchanged because a schedule belongs to a Session and a friend's Bot keeps those tools in its own chat. No judgement moves, and no test was added: room-participant.js only creates a Session for a local member, so the remote path has no trigger today.", "files": ["docs/subsystems/core-host.md", "core/src/host/connector-access.js", "core/test/connector-access.test.js", "patches/ds-bot/files/src/host/connector-access.js", "patches/ds-bot/files/test/connector-access.test.js", "changelog.d/1271-room-refusal-wording.changed.md"], "checks": { "ran": "node scripts/ds-bot-patches.mjs check && node --test test/connector-access.test.js test/connectors.test.js test/hidden-tool-removal.test.js test/turn-origin.test.js test/delivery-board.test.js", "result": "green", "at": "5cd087cfb1d6abd05fb051d24adb09cbb20b5032" }, "next": "§1 doc wording last" }