Repository navigation
feat(connectors): a Feishu message starts a turn here (#1266) - #1278
Merged
Merged
Conversation
Feishu phase 1 only posted out. Phase 2 is the other direction: the instance dials Feishu over the long connection and a message the owner types there starts a turn here. The socket is injected (rt.feishuTransport), so the client behind it is one adapter and the dependency decision changes nothing else. feishu-inbound.js owns the policy: off unless feishu.enabled is true, the owner is the only sender that may start a turn, a bounded ring of 512 event_ids makes a replay land once, and App ID + App Secret are read as one credential. A message lands through remote-chat.deliver — the seam a friend's Bot already uses — and the reply leaves through the Feishu sender, so there is no second path into or out of a chat. The Settings row grows inbound: off|connecting|ready|error, the last event, the last error, and whether the app pair is set or missing (never a value).
- `src/host/feishu-sdk.js` is the default adapter: the package is imported only when a socket is dialled, the app pair is read at that moment, and the SDK's flattened v2 event is rebuilt into the envelope `readEvent` already reads. - `@larksuiteoapi/node-sdk@1.76.0` is `core/`'s first runtime dependency; the lock was regenerated with `npm install --package-lock-only` and the hunk is in `0007-version-and-tooling.patch`. - `rt.feishuTransport` is read on every `start()`, so an adapter that arrives late comes up on the row's Try again, and a stop that lands while a socket dials hangs up the socket that arrives instead of reporting the row ready. - `Fixes #1266`; a changelog fragment now that the chain is usable.
Collaborator
Author
|
Verdict: INC at 02e3ed6 Blocker (code): a reply has no way out. The body says the answer "leaves through the Feishu sender already in
So Fix, one of two:
Medium: a message whose delivery fails is lost for good. Low: with no What I checked, and that held:
|
) The Reviewer's INC at 02e3ed6, two code defects: 1. A reply had no exit. `remote-chat.forward` keys its sender by `bot.remote.plugin`, and the inbound path called `deliver` without one, so every chat was opened with `remote.plugin === undefined` and every answer read "Not sent: the plugin that reaches this Bot is not running." The chat now carries the `feishu` plugin, and `connectors.js` registers that plugin's sender on the seam (`onSend`) the first time a message lands — `remote-chat` installs after this module, so it cannot be registered while module install runs. The sender posts on the `FEISHU_WEBHOOK` phase 1 already uses, and a missing webhook or a refusal is a "Not sent:" sentence in the chat, never a silent drop. `open` refreshes a chat an older build opened without the plugin. 2. A message whose delivery failed was lost for good. `handleEvent` wrote the `event_id` into the ring before `await deliver`, so a throw left the id there and every replay read `duplicate`. The id now enters the ring only after the handover returned; an `inFlight` set keeps two copies arriving at once from starting two turns in that gap. Tests: a message whose delivery throws is handed over again (ring written after), two copies at once still one turn, the chat opens on the feishu plugin, the sender is registered when the first message lands, the answer leaves on the webhook (missing webhook / refusal worded as "Not sent:"), and a composed check that a reply typed into a `feishu:` chat reaches that sender.
Collaborator
Author
|
Verdict: PASS at 0946f4c The two defects the INC at The way out now exists. The ring is written after the handover. Also checked, not taken from the body:
Ran: Not run: The body's own numbers I re-read rather than trusted: Two notes, neither blocking:
|
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 27, 2026
…hinkFlowLab#1252) (ThinkFlowLab#1289) fix(board): archive finished task cards and complete the overall card (ThinkFlowLab#1252) Written-by: Dev-1 Owner: Dev-1 Reviewer: Reviewer Fixes ThinkFlowLab#1252 Tip SHA: `5b832b91f8ebdb1b7de782e87d40f35b24f6549d` ## What changed - `core/src/host/tasks.js`: the `tasks` read lists active work only — a card that is `accepted`, `cancelled` or `failed` leaves the list a board polls. `{ archive: true }` is the same read including those rows; `{ archive: true, limit: N }` returns the newest N **finished** rows by `updatedAt` (tiebreak `createdAt`) and no active rows, so the board's 15 s poll carries the slice and not the history while the default read keeps carrying the active work. `limit` is a number or nothing: a string, boolean or object is refused (`bot/invalid`) instead of coerced. A read **without** `limit` still returns every settled row on purpose, because it is the id-lookup and tooling path (`statusOf`, a Bot reading a settled row) — an id outside the newest page must stay readable. - The same file: new `settleOverall(child)` completes an overall group card when every child has ended, through the same `transition`/`applyColumn` write a person's move uses. It completes `accepted` (with `progress: 100`) when each child ended `accepted` or `cancelled`, and `failed` when any child ended `failed` — a failed part is not hidden behind a green overall card. It is called where a move lands on `accepted` (`applyColumn`), from `task-accept-result`, from `task-cancel`, and on the two other doors a child ends through — `taskBotDeleted` (deleting a Bot fails its open cards) and `taskDecided` (an owner's reject cancels the card that waited on the approval) — so neither path can leave the overall card `working` behind a child that already ended. An overall card already `blocked` or `result-ready` is left to its owner. - `core/src/client/kanban.js`: the board keeps an `archive` list from its second read and sorts it newest first by `updatedAt` (tiebreak `createdAt`) **before** cutting, renders `boardTasksFor(tasks, archive)` — active work plus the twenty newest finished cards — and moves a finished card off the active list at once in `replace`, before the next poll. A **Bot** board names its Bot in that read (`tasks { botId, archive: true, limit: ARCHIVE_SHOWN }`): the host spends the page's budget before the board can filter by `ownerBotId`, so an unfiltered page lets another Bot's newer cards empty this Done column. The default read stays unfiltered on purpose — it is active work, a superset the board filters locally without losing a card. Ordering by `createdAt` would drop exactly the card a member just finished, which has an old `createdAt` and a fresh `updatedAt`. - `core/src/client/inbox.js`: ThinkFlowLab#1252 made the default read active-only, but the Inbox is a second consumer that classifies finished and failed cards (`inboxModel`: `failed` → Needs you, `accepted`/`cancelled` → Done) and counts them in `needsCount`. The one shared `useTasks` hook behind the Inbox row and the dialog now asks `tasks { archive: true }` — the uncapped list — and publishes that, so those cards reach the badge and both groups again. The board's bounded second read is untouched. - `docs/subsystems/core-client.md`: the `kanban.js` row said the cap twice in one line (once for the read, once for the column); that is now one sentence that also says a Bot board names its Bot, and the archive read without `limit` staying uncapped is said once. ## Red first All of them were written before the fix and run red on the unfixed tree (`node --import ./test/harness-resolve.mjs --test --test-name-pattern="1252" test/bot-tasks.test.js`). First round, the original ThinkFlowLab#1252 behaviour: ``` a done card leaves the room board by default true !== false expected: false actual: true ``` ``` the overall card completes with its last child 'open' !== 'accepted' expected: 'accepted' actual: 'open' ``` Second round, after Security's INC on the previous tip: the failed child, the unbounded payload, and the oldest-first slice. ``` a failed child makes the overall card fail, not pass + 'accepted' - 'failed' ``` ``` the requested limit bounds the payload 200 !== 20 expected: 20 actual: 200 ``` ``` the column starts at the newest finished card + 'task-done-19' - 'task-done-36' ``` Third round, Security's second INC: a busy room spent the whole budget on active rows. ``` the bounded archive carries finished rows only false !== true expected: true actual: false ``` Fourth round, Reviewer2's INC on the previous tip: the two other doors a child ends through, and the Bot-scoped archive page. Each knife was cut on the `5385491f` tree, the file restored from a copy, and the pair re-run green. Knife 1 — delete `settleOverall(task)` from `taskBotDeleted` (`core/src/host/tasks.js`), which reddens `ThinkFlowLab#1252: deleting a child's Bot fails the overall card, read back from the archive`: ``` a failed child closes the overall card as failed 'open' !== 'failed' expected: 'failed' actual: 'open' ``` Knife 2 — put the Bot board's archive read back to the unfiltered `query` (`core/src/client/kanban.js`), which reddens `ThinkFlowLab#1252: a Bot board asks the host for that Bot's archive, not the whole scope`: ``` the archive read names the Bot + actual - expected { archive: true, - botId: 'a', limit: 20 } ``` Restored: `bot-tasks.test.js` + `kanban.test.js` `20/20` green again. Fifth round, Reviewer2's INC at `5385491f`: the Inbox is a second consumer of `tasks`, and the active-only default cut its finished and failed cards off. The drive posts `{ archive: true }` and asserts the settled rows reach the badge and the two groups; on the unfixed tree the read posted **no body at all**, so the fake host answered the active-only default and every assertion fell over: ``` the Inbox read asks for the settled rows expected: 1 actual: 0 ``` The same run, measured before the assertions: `posted null`, badge `"4"` (the base count, unchanged), Done card `null`, Needs-you card `null`. ## Reviewer finding `6415648677` (`Verdict: INC at 86e9795`) — answered at this tip - **Item 1 — two terminal paths ended a child without settling the overall card.** `settleOverall(task)` now runs directly after the terminal `transition` in both `rt.taskDecided` branches (the waiting-input branch and the blocked branch) and in `rt.taskBotDeleted`, with the `continue` / `return save()` kept and no second gate added. Proven by `ThinkFlowLab#1252: deleting a child's Bot fails the overall card, read back from the archive` — child `failed`, parent `failed`, `columnOf` done, and the progress the card already carried (the failed branch only calls `transition`; `progress: 100` is set on the accepted branch alone) — and by `ThinkFlowLab#1252: a rejected approval cancels the child and completes the overall card, read back` — child `cancelled`, parent `accepted`, `progress 100`, `columnOf` done. Both read the parent back through `tasks { archive: true }` / `{ roomId, archive: true }`, not through a return value. - **Item 2 — the Bot-scoped page was cut before the `botId` filter.** A Bot board now sends `archiveQuery = { botId: scope.botId }` with the archive read: `{ botId, archive: true, limit: ARCHIVE_SHOWN }`; the group board keeps `roomId`, and the default read stays unfiltered. Proven by `ThinkFlowLab#1252: a Bot-scoped archive page is cut after the Bot filter` (the Bot-scoped host page carries its own card while the unfiltered page does not) and by the client drive that captures the posted body as exactly `{ botId: 'a', archive: true, limit: ARCHIVE_SHOWN }`. - **Non-blocking, docs.** The duplicated cap sentence in `docs/subsystems/core-client.md` is collapsed to the sentence that states the behaviour; the literal call syntax is gone. ## Reviewer finding `6416140014` (`Verdict: INC at 5385491`) — answered at this tip - **Blocking — the Inbox stopped receiving finished and failed cards.** The chain at `5385491f`: `core/src/client/inbox.js` `useTasks` called `actions?.tasks?.()` with no body, `core/src/client/index.js` sent it as `{}`, and `core/src/host/tasks.js` answered the active-only default (`archived ? rows : rows.filter(openTask)`), while `inboxModel` classifies `failed` → Needs you and `accepted`/`cancelled` → Done and `needsCount` counts them. Fixed in the one shared hook: `useTasks` now sends `{ archive: true }` and publishes the list it gets; the board's bounded second read is unchanged. Red at `5385491f` is quoted under *Red first*, fifth round (`expected: 1, actual: 0` — no body posted; badge unchanged at the snapshot count; both card lookups `null`). Green at this tip: the same test passes, `core/test/inbox.test.js` **27/27**, and the assertion is the behaviour — `failed` lands in Needs you, `accepted` in Done, and the row badge counts the failed card. - **Non-blocking — the bounded page is `settled`, the Done column keeps only `accepted`.** Recorded as a decided residual under *Decisions recorded, not omissions* below: newer `cancelled`/`failed` rows can spend a scope's page budget and push an older Done card out of the twenty. Not changed at this tip, and the reason is written down there. - **Non-blocking — the Bot board's default read is still unfiltered client-side.** That read is the **active** list: unfiltered is a superset of the Bot's own rows and the board filters it in memory, so no card can be lost — a host-side cut is what made the archive read need `botId`, and that one sends it. - **Non-blocking — the stale base line.** The old sentence naming a `main` base that was only true at claim time is gone; the base history below is the real path to this tip. Advisory rounds: none at this tip yet — Chief freezes before a PASS request. Superseded as evidence and kept as record: `6415907830` (Test) and `6415965572` (Security) at `5385491f`; `6415574602` and `6415627981` at `86e9795`; `6415419164` at `bdf867b2`; `6415205741` and `6415207539` at `150aeaa`. ## Why these files, and the base `main` moved several times while this was built: the branch was cut at `99b1fab`, hand-re-applied onto ThinkFlowLab#1276's new card model (`taskCard` / `BoardCardFrame`), then rebased onto `b4e1faa` (ThinkFlowLab#1284, which rewrote `docs/subsystems/core-host.md`, a file in this diff), `63f81f0` (ThinkFlowLab#1278), `ef5dae42` (ThinkFlowLab#1291) and now **`e37f1f83`** (ThinkFlowLab#1296 and ThinkFlowLab#1297, both landed after `ef5dae42`). The tip is therefore the merged tree: current `main` + this diff. `ThinkFlowLab#1297`'s delivery-board test in `core/test/kanban.test.js` was kept alongside this board's tests — the conflict was resolved by keeping both sides — and the board text still sits in `### Bot task board (ThinkFlowLab#1058)`, with no contradiction of ThinkFlowLab#1267's mid-turn-line subsection. At this tip the pull request is **19 files, +1007/−104, one commit** on base `e37f1f83`. - `core/src/host/tasks.js` — the read contract, the bounded archive read, the overall-card completion, and the settle on every door a child ends through. - `core/src/client/kanban.js` — the archive read (Bot-scoped on a Bot board), newest-first ordering, the twenty-card column. - `core/src/client/inbox.js` — the one shared task load asks for the archive, so the Inbox keeps its Done and Needs-you cards. - `core/client.js` — generated by `npm run build` in `core/`, never edited by hand. - `core/test/bot-tasks.test.js` — the eight ThinkFlowLab#1252 host cases (active-only board and explicit archive; last child completes the overall card; a failed child fails it; the requested archive bound with the unbounded read still seeing every row; a busy room's bounded read carrying finished rows only, with the `limit` type refusals; a deleted Bot failing the overall card; a rejected approval cancelling the child and completing the overall card; a Bot-scoped page cut after the Bot filter) plus one compat read: the post-restart read of a finished card now asks for the archive. This is where the active-only/archive case lives, not `core/test/tasks.test.js`. - `core/test/inbox.test.js` — the Inbox drive: the read posts `{ archive: true }`, a `failed` card lands in Needs you, an `accepted` card lands in Done, and the row badge counts the failed card. The shared harness keeps its no-op-effect stub for every other case; only this drive runs its effect. - `core/test/tasks.test.js` — only the `statusOf` helper changes: it reads `{ archive: true }` (no limit) because it looks tasks up by id and some are finished. - `core/test/kanban.test.js` — the client fixture: 37 finished cards give a literal twenty-card column starting at the newest, the oldest is outside the slice, a card with an old `createdAt` and a fresh `updatedAt` leads the column, and a driven load asserts the posted body (Bot board `botId`, group `roomId`, default read unfiltered). - `docs/subsystems/core-host.md` — the read contract with and without `limit`, the bot/room filter before the cut, and when an overall card completes accepted or failed (owned page for `tasks.js`). - `docs/subsystems/core-client.md` — the board's two reads, the newest-first cut, a Bot board naming its Bot, and the uncapped id-lookup path; the duplicated cap sentence is merged (owned page for `kanban.js`). - `changelog.d/1252-board-archive.changed.md` — user-visible board and Inbox change. - `patches/ds-bot/0008-client-bundle.patch` and the `patches/ds-bot/files/` mirrors — written by `node scripts/ds-bot-patches.mjs generate` from that same `core/` tree; the generator's context-only rewrite of `0005`, `0012` and `0014` was reverted, and `check` re-run green. ## Decisions recorded, not omissions - The board's archive read is capped at the same limit as the column (`{ ...query, archive: true, limit: ARCHIVE_SHOWN }`, room-scoped for a group, `{ botId }` for a Bot board, newest-settled first, finished rows only — the default read carries the active ones), so the poll carries the slice and not the history. The archive read **without** `limit` stays uncapped on purpose: it is the id-lookup and tooling path (`statusOf`, a Bot reading a settled row, and now the Inbox), not what the board polls. - The `counts` loop still runs over `[...own, ...archive]` unchanged, and no counts assertion moved: the finished-only page is what stops a card appearing in both lists and being counted twice. - A child's requester can complete the room's overall card: the settle path runs the same transition with no actor, so the card's status moves and the requester gate is not consulted. It is status-only — nothing is deleted, and the card's actors and history are kept. - The bounded page is `settled`, while the Done column keeps only `accepted`: twenty newer `cancelled`/`failed` rows in one scope can spend the page's budget and push an older Done card out of the column. Left as it is at this tip, deliberately: the page is the recent-activity slice ordered by `updatedAt`, and narrowing it to `accepted` would make a "newest finished" read lie about cancelled/failed work other callers read. An older card is still readable without `limit`, which stays uncapped on purpose. ## What I ran At `5b832b91` (tree `f14bf9fd`, base `e37f1f83`). - `TZ=UTC npm run verify` — exit 0: core **1784 tests, 1784 pass, 0 fail**, plugins/social 525, plugins/tree 125, plugins/timeline 125, `.github/scripts` 509, interop 37, live-e2e 24; every suite green. - the ThinkFlowLab#1252 suites on their own at the same tree: `core/test/bot-tasks.test.js` 14/14, `core/test/kanban.test.js` 7/7, `core/test/inbox.test.js` 27/27. - `npm run build` in `core/` before `generate`; `node scripts/ds-bot-patches.mjs generate` then `check` — `ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)`, `ok 10 patches apply cleanly in order`, `ok core/ equals ds-bot/ + patches/ds-bot/`. The generator's context-only rewrite of `0005`, `0012` and `0014` was reverted and `check` re-run green. - `node core/scripts/build-client.mjs --check` inside `verify` — the committed `core/client.js` matches `core/src/` at this tip. ## Not run - The smoke bench (`core/dev/bench.sh smoke`): it needs a live DSH instance and a free port; this touches `src/host/tasks.js`, `src/client/kanban.js` and `src/client/inbox.js`, not `index.js`, and the host behaviour is covered by `core/test/bot-tasks.test.js`, `core/test/tasks.test.js`, `core/test/inbox.test.js` and `core/test/task-drive.test.js`. - Packaging, deploy and the desktop app — untouched. ## Verification at the tip Everything above ran at `5b832b91f8ebdb1b7de782e87d40f35b24f6549d` — the tip this pull request pushes, tree `f14bf9fd5a0e75746d39c4168f57cb0ad1b719b2`, base `e37f1f838023ce81e673d8fbec66a55d81dfe5e3` (19 files, +1007/−104, one commit). ``` ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5) ok 10 patches apply cleanly in order ok core/ equals ds-bot/ + patches/ds-bot/ ``` ## Checkpoint The generated mirrors — `patches/ds-bot/files/src/{client/inbox.js,client/kanban.js,host/tasks.js}` and `patches/ds-bot/files/test/{bot-tasks,inbox,kanban,tasks}.test.js` — are copies written by the generator from the files above, so the block lists the 12 paths a reviewer should open first; the diff itself is the full 19. <!-- flybot:checkpoint --> ``` { "pr": 1289, "claim": "Dev-1", "issue": 1252, "summary": "A finished card no longer ships with every board read: the tasks endpoint lists active work, { archive: true } adds the finished rows, and { archive: true, limit: N } returns the newest N finished rows, so the board's poll carries twenty settled cards and no active ones. A Bot board names its Bot there, and the Inbox asks for the archive so its Done and Needs-you cards return. The overall group card completes when every child has ended — accepted if each ended accepted or cancelled, failed if any failed. Tests were red first; docs, the client bundle and the patches ride in the same commit.", "files": [ "changelog.d/1252-board-archive.changed.md", "core/client.js", "core/src/client/inbox.js", "core/src/client/kanban.js", "core/src/host/tasks.js", "core/test/bot-tasks.test.js", "core/test/inbox.test.js", "core/test/kanban.test.js", "docs/subsystems/core-client.md", "docs/subsystems/core-host.md", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/src/host/tasks.js" ], "checks": { "ran": "TZ=UTC npm run verify; node scripts/ds-bot-patches.mjs check", "result": "green", "at": "5b832b91f8ebdb1b7de782e87d40f35b24f6549d" }, "next": "none — ready for review" } ``` <!-- /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.
Fixes #1266
Owner: Dev
Written-by: Dev
Reviewer: Reviewer
Base:
7de4edf2a513667d0339a64c4129ee142bc06a66— the parent of this branch's first commit and currentmainwhen it was cut (commits/main, read 2026-10-26T10:47Z).Tip SHA: 0946f4c
What changed
Phase 1 (#1263, merged as #1269) only posts out. This branch is the other direction, end to end: a message the owner types in Feishu starts a turn here, and the answer goes back out to Feishu — over the long connection the instance dials out to, so nothing needs a public callback URL, a platform allowlist, or a signature check.
core/src/host/feishu-inbound.jsis where the policy lives:feishu.enabledmust betrue; while it is off no socket is dialled at all. An inbound socket listens for traffic, so turning it on is a decision, not an upgrade side effect.feishu.ownerOpenIdnormalises toundefinedand refuses everyone; an emptyopen_idnever compares equal to it. Anyone else is one log line and no turn — a stranger's message must not spend the owner's tokens.event_ids, dropped oldest first: a reconnecting socket replays a tail, and the ring is what makes that one turn. The id enters the ring only afterdeliverreturned, so a handover that threw is retried instead of being refused as a duplicate for ever; aninFlightset covers two copies arriving while the first is still being handed over. It is memory only — no state write, no secret.start()while the first dials shares it; a dial that failed is not remembered, so the next start tries again.FEISHU_APP_ID+FEISHU_APP_SECRETare read through the samert.secretValuephase 1 uses; either half missing or blank answersundefined, so a half-saved app dials nothing instead of failing at the handshake. Neither value leaves the function.connectorsView()'s Feishu entry growsinbound: { state, lastEventAt, lastError, app }—off|connecting|ready|error,appissetormissing, and the error isrt.redactSecrets-ed and cut to 300 characters. No value ever reaches the view.remote-chat.deliver({ handle: 'feishu:<chat_id>', text, name, plugin: 'feishu' })— the seam a friend's Bot already uses, which fences the text as outside input. The answer leaves through the same seam:connectors.jsregisters a sender for thefeishuplugin (remote-chat.onSend) the first time a message lands, and it posts on theFEISHU_WEBHOOKphase 1 already uses. Nothing new was added to groups or delivery.readEvent()accepts only theim.message.receive_v1text envelope:header.event_id,sender.sender_id.open_id,message.{chat_id, message_type, content}, wherecontentis a JSON string. A v1 shape, a card callback, an image or an unparseable body reads as null, logs one line and starts nothing — a connector that invents a message out of an unknown body is worse than one that says it does not know it.The client commit on top of that seam:
core/src/host/feishu-sdk.jsis the default adapter over@larksuiteoapi/node-sdk. The package is imported withawait import(...)insideconnect(), never as a top-level static import, sofeishu-inbound's tests — and a host that never turns the long connection on — run without it installed; a missing package is one readable line naming it. The app pair is read at dial time, the SDK'sonReady/onErrordecide whenconnect()resolves or rejects (a socket that never opens gives up at a 15 s deadline), andclose()hangs the client up once. The SDK flattens a v2 event before a handler sees it (event_idbesidemessage); the adapter rebuilds Feishu's own{ header, event }envelope rather than teaching the policy a second shape.rt.feishuTransportis read on everystart(), not copied while the modules install.connectors.jsputs the SDK adapter on the runtime before the policy reads it, and passes a getter (transport: () => rt.feishuTransport) instead of the value, so an adapter installed after boot — a plugin's client, a test's stub — is the one the next Try again dials. With the snapshot the row would stay aterror: no long-connection transport is installedforever while every injected test passed.stop()no longer loses the socket it cancels. A generation counter is bumped bystop(); a dial already in flight compares its generation on return and, if it lost, hangs up the socket that arrived instead of adopting it. Without that, a stop landing beforeconnectresolved leftcloserempty (socket alive) and then wrotestate: 'ready'andconnected += 1behind the owner's back.feishu.test.jsnow spell out theinboundfield they gained.@larksuiteoapi/node-sdk@1.76.0(exact, no caret) iscore/'s first runtime dependency:package-lock.jsonwas regenerated withnpm install --package-lock-only --no-fund --no-audit, andds-bot-patches generateput the hunk in0007-version-and-tooling.patch— no patch hunk was written by hand andfiles/gained no mirror for these two files.The INC at
02e3ed67(Reviewer, comment 6414433193)Two code defects, both fixed in
0946f4c1— the 15 inbound tests could not see either, because they never leave the chat:remote-chat.forwardkeys its sender bybot.remote.plugin, and this module calleddeliverwithout one, so the chat was opened withremote.plugin === undefinedand every answer readNot sent: the plugin that reaches this Bot is not running.Switchfeishu.enabledon, type in Feishu: the turn starts and its answer goes nowhere. The chat now carries thefeishuplugin,open()refreshes a chat an older build opened without it, and thefeishusender is registered on the seam when the first message lands (remote-chatinstalls afterconnectors, so it cannot be registered while module install runs). A missing webhook or a refusal is aNot sent:line in the chat, never a silent drop.handleEventwrote theevent_idinto the ring beforeawait deliver, so a throw left the id there and every replay readduplicate. The id is remembered only after the handover returned, and one already in flight is refused by aninFlightset while it waits, so two copies arriving together still start one turn.feishu:oc_…and drawn by Settings as a friend's Bot) is untouched and still listed under Risk as an open question.Red without the source change
Seventeen knives at this tip. Each removes exactly one guard, runs one test file, and the file is restored from the commit afterwards:
not ok 1 - a replayed event_id starts one turn, and the second one says duplicate,not ok 2 - a ring this size still forgets the oldest id…,not ok 16 - a message whose delivery throws is handed over again…(14 pass)not ok 3 - a message from anyone but the owner starts no turn(16 pass)not ok 5 - switched off, it dials nothing at all(16 pass)not ok 6 - a failed connect is one redacted line in the row…andnot ok 12 - the view never carries either half of the app pair(15 pass)not ok 4 - no owner and no open_id never compare equal(16 pass)not ok 11 - the app pair is one credential…(16 pass)not ok 13 - with no socket adapter installed it says so…andnot ok 14 - an adapter installed after the first start…(15 pass)connectors.jssnapshots the adapter (transport: rt.feishuTransport) →not ok 16 - an adapter installed after the connectors module…, and the two new seam tests below fail to reach a chat at all (30 pass)not ok 14 - an adapter installed after the first start is the one the next start dials(16 pass)stop()'s path removed →not ok 15 - a stop while the socket dials hangs up the one that arrives…(16 pass)not ok 1 - the SDK is imported at dial time…(4 pass)not ok 3 - with no app pair saved nothing is imported and nothing dials(4 pass)feishuplugin →not ok 17 - a Feishu message opens its chat on the feishu plugin(32 pass)not ok 18 - the answer sender is registered on the chat seam when a Feishu message arrives(32 pass)not ok 19 - the answer to a Feishu message leaves on the webhook phase 1 already posts to(32 pass)event_idis remembered before the handover →not ok 16 - a message whose delivery throws is handed over again, not remembered as handled(16 pass)not ok 17 - two copies of one event arriving together still start one turn(16 pass)Knives 8–10 are the two defects the first tip shipped green (a snapshot transport, a stop racing a dial); 13–17 are the two the Reviewer found at
02e3ed67(no reply sender; the ring written before the handover, and a repeat copy while one handover is in flight). Each now has a test that fails only on that defect.Verification at the tip
Tip SHA:
0946f4c13a87ba36df58fdd3381ee2e487450bc3Ran:
test/feishu-inbound.test.js17/17,test/feishu-sdk.test.js5/5,test/feishu.test.js8/8,test/connectors.test.js33/33,test/connector-access.test.js19/19,test/remote-chat.test.js18/18,test/friend-chat-awaiting.test.js7/7 — 107/107 across the seven files this diff touches.coresuite (test/*.test.js --test-concurrency=1): 1709 tests, 1704 pass, 5 fail. The five are the known base-red ones, unchanged by this branch:check-arch-docs'sempty diff passes(no diffableHEADin this checkout),composer-label/header-effects/read-page(jsdomnot installed here), andusage-fields'sfixture usage.sqlite has one real calls row with all five fields.src/client/model-test.jsis outside that glob and also red here for a missingreact.node scripts/ds-bot-patches.mjs check— three lines, the pin line real because this checkout has git:ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)/ok 10 patches apply cleanly in order/ok core/ equals ds-bot/ + patches/ds-bot/.cd core && node scripts/check-arch-docs.mjs→arch-docs: OK (20 changed file(s)).oxlint -c .oxlintrc.json src test→Found 58 warnings and 0 errors.(all pre-existing)..github/scripts/changelog.mjs check→11 fragment(s) … are well formed;check-empty-catch.mjsexit 0;check-mojibake.mjs→ 0 occurrences of U+FFFD in 1100 text files;testing-layers.mjs→ 411 test files in 16 workflows, every one classified;core/scripts/build-client.mjs --checkexit 0 (no shared change, soclient.jsis untouched).Not run (and why):
im.message.receive_v1subscribed over the long connection, then saveFEISHU_APP_ID/FEISHU_APP_SECRETthroughsecret-set; both values stay out ofstate.jsonand out of the view.npm cilocally:core/node_modulesin this checkout is a shared symlink, so aciwould have deleted it. The lock is in sync —npm install --package-lock-onlywrote it and a second run wrote no change — andflybot-testsrunsnpm ciincore/, which is where the install is proved.plugins/*,interop,live-e2e: this diff's only shared file isconnectors.jsand its tests, both run above.Risk
FEISHU_WEBHOOKposts to the chat that webhook belongs to, so the reply lands there, not in whatever conversation the app bot was messaged in. With the app bot and the webhook in the same group that is the same chat; otherwise the answer is visible but in the webhook's chat. Sending back into the exactchat_idwould need the app's send scope on top of the receive scope, which is why it is not done here.@larksuiteoapi/node-sdk@1.76.0pins its own 7 direct dependencies with caret ranges, so two consumers installing at different times can get different trees; we can only lock the tree ourpackage-lock.jsonrecords.core/gains its first runtime dependency: one entry inpackage.json, about 40 packages installed.check-packand the workflows are unchanged — the tests action already runsnpm ciincore/and caches oncore/package-lock.json.remote-chat.delivercarriesSOURCE_KIND/REMOTE_FORMfrom Bot Social, so a Feishu conversation appears as "a friend's Bot" in Settings, and with no profile name the chat is namedfeishu:oc_…. That is a display decision, not a side effect to settle inside a transport, so it is untouched here and listed as an open question.connect(onEvent)must hand over exactly the envelopes Feishu sent;readEventnarrows what it accepts, but a client that invents a message can still start a turn.Files in this diff (20)
core/src/host/feishu-inbound.jscore/src/host/feishu-sdk.jscore/src/host/connectors.jsinboundin the row, thefeishureply sendercore/package.json"@larksuiteoapi/node-sdk": "1.76.0"core/package-lock.json--package-lock-onlycore/test/feishu-inbound.test.jscore/test/feishu-sdk.test.jscore/test/connectors.test.jscore/test/feishu.test.jsinboundcore/test/remote-chat.test.jsfeishu:chat leaves on the webhookdocs/subsystems/core-host.mdchangelog.d/1266-feishu-inbound.added.mdpatches/ds-bot/0001-core-fixes.patchconnectors.js,feishu-inbound.jsand the testspatches/ds-bot/0007-version-and-tooling.patchpackage.json+package-lock.jsonpatches/ds-bot/files/src/host/feishu-inbound.jspatches/ds-bot/files/src/host/feishu-sdk.jspatches/ds-bot/files/test/feishu-inbound.test.jspatches/ds-bot/files/test/feishu-sdk.test.jspatches/ds-bot/files/test/feishu.test.jspatches/ds-bot/files/test/remote-chat.test.jsCheckpoint
{ "pr": 1278, "claim": "Dev", "issue": 1266, "summary": "Feishu phase 2 inbound, both ways: a message the owner types in Feishu starts a turn here over the long connection, and the answer leaves on the FEISHU_WEBHOOK phase 1 already posts with. feishu-inbound.js keeps the policy — off unless enabled, owner-only, a 512 event_id ring written only after a successful handover, one socket, the app pair as one credential — and feishu-sdk.js is the default SDK adapter, so core/ gains its first runtime dependency. The INC at 02e3ed67 found the chat opened without the feishu plugin and the ring written before deliver; both are fixed, with seventeen knives.", "files": ["core/src/host/feishu-inbound.js", "core/src/host/feishu-sdk.js", "core/src/host/connectors.js", "core/package.json", "core/package-lock.json", "core/test/feishu-inbound.test.js", "core/test/feishu-sdk.test.js", "core/test/connectors.test.js", "core/test/remote-chat.test.js", "docs/subsystems/core-host.md", "patches/ds-bot/0001-core-fixes.patch", "changelog.d/1266-feishu-inbound.added.md"], "checks": { "ran": "feishu-inbound 17/17, connectors 33/33, remote-chat 18/18, other four touched files 39/39 (107); full core 1704/1709 (5 known base-red); ds-bot-patches 3 lines ok; arch-docs OK (20); oxlint 0 errors; 17 knives red", "result": "green", "at": "0946f4c13a87ba36df58fdd3381ee2e487450bc3" }, "next": "none — review" }