Skip to content

tasks: keep a card waiting for its rework out of the role pool - #1815

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/3-handoff-claim-queue-role
Oct 30, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/3-handoff-claim-queue-role

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1804

Owner: Dev-3
Reviewer: Birfy
Written-by: Dev-3

What was wrong

A card its Reviewer sent back for rework still carries queueRole: Reviewer, while openRework rewrites the original's handoff to { from: 'Reviewer', to: 'Dev' }. When the Reviewer's lease on the original expired, expireLeases treated the card as abandoned work and returned it to the Reviewer queue:

  • the wake path reads queueRole, so the Reviewer was offered the card again;
  • claim matches handoff.to, so the Reviewer could not take it (no card for Reviewer in the handoff pool) while the Dev could (Claimed task-…).

Two members, one card, the same work twice — while the rework child was still open.

The fix

One guard in expireLeases (core/src/host/tasks.js, +5): a card with an open rework child is not a queue item its holder abandoned, so the expiry skips it. The card keeps its holder and its working status until closeRework lands the child and closes it; the claim path is untouched, and the card sits in no pool while the rework is open, so wake and claim cannot disagree about it.

This also removes a dead end: an expired lease used to turn the original back into open, after which closeRework (it requires working) could never close it.

Why not the one-line fix

The obvious one-liner makes the handoff claim path match queueRole instead of handoff.to. It fixes which pool the two paths look at and leaves the duplicate in place: the card is still recovered while its rework is open, and now the Reviewer — the member who sent it back — is the one who can take it. The work is done twice either way, only by the other party. So this change removes the recovery, not the disagreement: a card waiting for its rework is not abandoned work in the first place.

Red first, re-taken on this base (46327e8)

Case: a card waiting for its rework is not re-offered when its lease expires (#1804) in core/test/task-handoff-route.test.js. With only core/src/host/tasks.js checked out from this base:

not ok 5 - a card waiting for its rework is not re-offered when its lease expires (#1804)
  error: |-
    the card held for rework is not recovered
    
    1 !== 0

It asserts the card is not recovered (taskExpireLeases returns 0), the Reviewer is not offered it again, the Dev is offered the rework child, and the Dev can claim the child but not the original.

Tests I ran

At this tip:

  • core/test/task-handoff-route.test.js — 5/5, fail 0
  • core/test/task-drive.test.js — 26/26, fail 0
  • core/test/tasks.test.js — 31/31, fail 0
  • TZ=UTC ARCH_DOCS_BASE=46327e82 npm run verify from the repo root — exit 0:
    • core lint 0 errors / 71 warnings (pre-existing), arch-docs: OK (6 changed file(s))
    • plugin lint 0 errors (36 / 9 / 2 / 16 warnings), build-client.mjs --check green
    • ds-bot-patches check green — 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/; run twice, identical
    • core 2171/2171, social 541/541, tree 127/127, timeline 135/135, workflow 294/294
    • .github/scripts 549/549, interop 37/37, live-e2e 24/24

I did not run: the smoke bench (dev/bench.sh smoke), the package job's pack check / typecheck / actionlint, or any CI-only job.

Generated at this tip

cd core && npm run build (bundle byte-identical, core/client.js unchanged), then node scripts/ds-bot-patches.mjs generate. The regenerated series reproduces core/ exactly. The only generated files in the diff are the two patches/ds-bot/files/ mirrors for src/host/tasks.js and test/task-handoff-route.test.js (+29 lines); generate also re-split unrelated hunks in the other patch files, which is representation only, so I left those as they were on the base and check proves the applied product still equals core/.

Follow-ups (not in this pull request)

  1. release still returns a card that has an open rework child: releaseQueued has its own door and can put the original back in the pool while the child is open. One guard where a card would re-enter a pool would cover both.
  2. claimHandoff reads handoff.to while the wake path reads queueRole; the two fields still disagree. They are simply unreachable together on this path now.

Gates at this head

enqueue-guard at f7999174670e06d667e5e08fe5ac5b1aaa938edb, verbatim:

do not enqueue #1815: no Verifier report names the tip f7999174670e

That is the only refusal it lists; it names no body problem, and it notes that the head lives in a fork, so the issue reference and agent-pr rules are not asked of it.

Checkpoint

{
  "pr": 1815,
  "claim": "Dev-3",
  "issue": 1804,
  "summary": "An expired lease on a card whose rework child is still open used to return the card to its Reviewer queue while the claim path matched handoff.to, so the Reviewer was woken for a card it could not take and the Dev could take the card the open rework already covered. expireLeases now skips a card that has an open rework child: it keeps its holder and working status until closeRework closes it, and the claim path is unchanged. Verified with npm run verify at this tip (core 2171/2171, plugins 1097/1097, patches check green twice).",
  "files": [
    "core/src/host/tasks.js",
    "core/test/task-handoff-route.test.js",
    "docs/subsystems/core-host.md",
    "changelog.d/1804-rework-lease.fixed.md",
    "patches/ds-bot/files/src/host/tasks.js",
    "patches/ds-bot/files/test/task-handoff-route.test.js"
  ],
  "checks": {
    "ran": "TZ=UTC ARCH_DOCS_BASE=46327e82 npm run verify",
    "result": "green",
    "at": "f7999174670e06d667e5e08fe5ac5b1aaa938edb"
  },
  "next": "none — ready for review"
}

…FlowLab#1804)

`openRework` rewrites the original card's `handoff` to Reviewer -> Dev but
leaves its `queueRole` at Reviewer, so an expired lease on the card was
recovered into the Reviewer queue while the claim path matched `handoff.to`.
The Reviewer was woken for a card it could not take, and the Dev could take
the card the open rework already covers: the same work twice.

A card with an open rework child is not a queue item its holder abandoned —
the rework decides where it goes, so the expiry skips it and the card keeps
its holder and its `working` status until `closeRework` closes it.
@HugoXK

HugoXK commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy — review request for #1815 at f7999174670e06d667e5e08fe5ac5b1aaa938edb.

At this tip tests-ok is green and merge-gate is green; enqueue-guard's only refusal is no Verifier report names the tip f7999174670e (it names no body problem and notes the head lives in a fork, so the issue-reference and agent-pr rules are not asked of it). Please post the Verdict for this tip.

@Birfy

Birfy commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at f799917
Written-by: Reviewer
Attestation: f799917 role=Reviewer

Ran at this tip:

$ TZ=UTC node --test test/task-handoff-route.test.js
# tests 5  # pass 5  # fail 0
$ TZ=UTC node --test test/tasks.test.js test/task-drive.test.js
# tests 57  # pass 57  # fail 0
$ node scripts/ds-bot-patches.mjs check
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

Red first, on this base: with only core/src/host/tasks.js put back to its base blob (main 125686bc, the file the PR changes), the new case fails not ok 5 ... the card held for rework is not recovered / 1 !== 0. Restored to the tip, it is green again.

What I checked:

Not run: the smoke bench, the package job's pack/typecheck/actionlint, and the CI-only jobs; tests-ok at this tip covers the full suites.

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 30, 2026
@github-actions github-actions Bot added the pass label Oct 30, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 30, 2026
Merged via the queue into ThinkFlowLab:main with commit f003a50 Oct 30, 2026
17 checks passed
@Birfy
Birfy deleted the dev/3-handoff-claim-queue-role branch October 30, 2026 09:02
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 30, 2026
HugoXK added a commit to HugoXK/FlyBot that referenced this pull request Oct 30, 2026
…t accepts (ThinkFlowLab#1823)

Fixes ThinkFlowLab#1820

## What was wrong

The Project settings panel's **Save memory** button never worked, for
anyone including the Project owner. The panel wrote `kind: 'note'`
(`core/src/client/project-board.js`), and the host accepts only
`decision` / `question` (`core/src/host/store.js:502`,
`project-assistant.js:185`, `MEMORY_KINDS` at `:28`), so every press
ended in `bot/invalid: Unknown Project memory kind note`. The success
line was unreachable, and a zh UI showed the host's English refusal
verbatim. The old test pinned that payload, so green CI never caught it.

The base is `2ea55d8a4d67cd1e4fe489ab2b14b1946cea4dd6` (live `main` when
this was pushed); the issue's `adbaf320fed0` is stale (`ThinkFlowLab#1815`, `ThinkFlowLab#1817`,
`ThinkFlowLab#1818` and `ThinkFlowLab#1822` have landed since).

## What changed

- `core/src/client/project-board.js`: the memory box sends `kind:
'decision'` — the host's own default when a writer omits `kind`
(`project-assistant.js:184`), and the only accepted kind that is not an
open question. The status line goes through `hostText`, so a refusal is
quoted in the UI language. The other two write paths are unchanged.
- `core/src/client/i18n.js`: three `HOST_ERRORS` entries, only for
refusals this panel's three write paths can actually surface — the owner
gate on all three (`Only the owner of this Project may change it`), an
empty member id on Bind/Unbind (`A binding needs the member id`), and a
stale Project id (`Unknown Project (\S+)`). The id rule is last and
takes one token, so it cannot swallow a longer `Unknown Project …`
sentence; the test keeps `Unknown Project status active` unchanged so
that stays proven. No other entry was added.
- `core/test/project-board-panels.test.js`: the stub that pinned `kind:
'note'` is replaced by a real round-trip through `rt.projectEndpoint`
(`call: (event, payload) => rt.projectEndpoint(event, payload)`), with a
control that a `decision` write lands in the memory, a control that
`note` is refused and writes nothing, and a zh assertion on the refusal
and on `记忆已保存。`.
- `docs/subsystems/core-client.md`: one paragraph on the panel's three
write paths and the memory-kind mapping.
- `changelog.d/1820-project-memory-save.fixed.md`.
- Generated surface: `core/client.js`,
`patches/ds-bot/0001-core-fixes.patch`,
`patches/ds-bot/0008-client-bundle.patch`, and the two
`patches/ds-bot/files/...` mirrors. The bulk of the diff is
`patches/ds-bot/0008-client-bundle.patch` (+5,023/−2,573 of the
+5,342/−2,598): that patch is machine-generated whenever the client
bundle is rebuilt and is never hand-edited — `npm run build-client`
regenerates it, and `node scripts/ds-bot-patches.mjs check` printed the
same three green lines on two consecutive runs. `0012` / `0014` were
reverted when the generator only changed hunk headers.

### No display surface today

This fix adds no display surface and no new wording for one. The only
reader of a `decision`-kind row is the `project-memory` endpoint, and no
client code calls it; the approved P-228 read-back panel is the one
future surface, and it labels these rows **memory entries** (记忆条目),
never "decisions". The audit below is why `decision` is the correct side
of the host's two-kind contract today.

## Memory kind mapping

The panel has no type selector: every Save memory press writes one
free-text row. `note` is not a kind the host accepts, so the panel sends
`decision`. This is a client-side mapping only — `MEMORY_KINDS`
(`project-assistant.js:28`) and the host's kind checks (`store.js:502`,
`project-assistant.js:185`) are unchanged.

**Every reader of a `decision`-kind row (base `2ea55d8a`):**

1. **`project-memory` endpoint** (`store.js:606`, routed by
`channel.js:676`, owner-gated at `store.js:605`) — returns `{project,
memory: {decisions, questions, digest}}`. Its only caller is a test
today: `core/src/client/` contains no `project-memory` read at all (the
bundle has `project-memory-write` and the `project-memory-save` action,
never the read endpoint), and no page renders the response. This is the
endpoint the approved P-228 read-back panel will use; today the row has
no display surface.
2. **The digest** (`project-assistant.js:159` `memoryOf` →
`projectMemoryFor`) — `digestForOwner` (`:432`) reads only
`memory.digest` (`:447`, `:450`); `buildDigest` (`:61`) takes
`project/rows/files/actionable` and no `decisions`/`questions`, so a
memory row never appears in the digest text, and the word "decision" is
never printed from it.
3. **The board's Decisions section** (`store.js:656`) — comes from
`project-board.js:150` `decisionsOf(project)`, which lists
`workDir/decisions/*.md` on disk. Different source from
`state.projectMemory`; a memory row never shows there and is never
labelled a decision.
4. **`rt.projectAssistant.read` / `write`** (`:176`, `:182`, exposed at
`:508`) — the internal API returns the owner's `{decisions, questions,
digest}`, but nothing calls `read` (the assistant Session registers only
`project_digest`, `:513`) and no `project_memory_write` tool is
registered (the instructions at `:37` name one that does not exist). No
assistant-side display path.
5. **The write response** — `project-memory-write` returns the updated
`memory` (`store.js:612` → `ownerView`), and the panel accepts it, but
`run()` uses it only for the success/refusal status line; `onSaved` is
not passed by any caller, so no entry is rendered.

Measured on `2ea55d8a` with the real endpoints (throwaway probe, not
committed):

```text
READBACK decisions => ["memory row: ship Friday"]
BOARD decisions => {"dir":"...","files":[{"path":"decisions/2026-10-30-ship.md",...}],"reason":""}
DIGEST => "Project Assistant (Ops) — 2026-10-30: no change since the previous digest."
```

**Why `decision` and not `question`.** `question` is the host's list for
open questions; the free text a person types on Save memory is a
statement or a settled call, so it belongs on the "recorded, no longer
open" side. `decision` is also the host's own default when a writer
omits `kind` (`project-assistant.js:184`), so the panel sends explicitly
what the host would have assumed.

**The trade-off and its condition.** Mapping to `decision` is deliberate
and holds only while no surface renders such a row under the word
"decision": today none does (readers 1–5 above), and P-228 renders them
as memory entries. It does not hold by itself. The approved P-228
read-back panel will list both arrays, and if it labels a `decisions`
row as a decision, a person's typed memory would read as a decision they
never made. For this mapping to stay honest, P-228 must render these
rows as **memory entries** (记忆条目) — or separate "recorded memory" from
"decisions" — and that requirement is P-228's acceptance, not this PR's.
The alternative here would be a third host kind (a new contract with no
reader, which the issue excludes) or a type selector in the panel (also
out of scope).

## Red first

Re-taken on the final base `2ea55d8a4d67cd1e4fe489ab2b14b1946cea4dd6`,
with only `core/src/client/project-board.js` reverted to that base and
the new tests kept:

```text
not ok 3 - the settings panel memory save lands in the Project memory through the real endpoint
  error: |-
    the panel reports the state the host answered
    + actual - expected

    + 'Unknown Project memory kind note'
    - 'Memory saved.'

not ok 4 - a memory kind the host does not know writes nothing, and the panel quotes a refusal in Chinese
  error: |-
    Expected values to be strictly equal:
    + actual - expected

    + 'Only the owner of this Project may change it'
    - '只有这个项目的主人可以修改'

# pass 4
# fail 2
```

The two failures are the two contracts this PR fixes: the panel sends a
kind the host refuses, and the panel quotes the host's English refusal
instead of the localized one. With the fix, the same file is 6/6.

## Verification at the tip

At the pushed tip `7c374bb32f38241d4d0f937f4f4ddcd11f45057c` (tree
`b1f5352cd2e2907437b75addc5dd2aba6291aca3`, the tree `npm run verify`
ran on locally as `06ac78a59af1738ff420f4e18592a6067394e044`):

```text
TZ=UTC ARCH_DOCS_BASE=2ea55d8a4d67cd1e4fe489ab2b14b1946cea4dd6 npm run verify
```

- core suite: **2182/2182** pass, 0 fail.
- core lint: **71 warnings, 0 errors**.
- `arch-docs`: **OK (10 changed file(s))** with
`ARCH_DOCS_BASE=2ea55d8a4d67cd1e4fe489ab2b14b1946cea4dd6`.
- `build-client --check`: green.
- `ds-bot-patches check` run twice, identical three lines:

```text
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 repo changelog test (`ok 54`) is green;
`changelog.d/1820-project-memory-save.fixed.md` is the new fragment.
- the remaining plugin / script suites are green (541, 127, 135, 294 and
549 tests), and `git status` is clean apart from the untracked
`node_modules` symlinks the bench needs.

10 files changed, +5342/−2598:
`changelog.d/1820-project-memory-save.fixed.md` +1/−0, `core/client.js`
+12/−3, `core/src/client/i18n.js` +7/−0,
`core/src/client/project-board.js` +7/−4,
`core/test/project-board-panels.test.js` +132/−7,
`docs/subsystems/core-client.md` +7/−0,
`patches/ds-bot/0001-core-fixes.patch` +14/−0,
`patches/ds-bot/0008-client-bundle.patch` +5023/−2573,
`patches/ds-bot/files/src/client/project-board.js` +7/−4,
`patches/ds-bot/files/test/project-board-panels.test.js` +132/−7.

Not run: the smoke bench (no `index.js` or `core/src/host/` change), the
pack check, and actionlint — `npm run verify` does not include them.

Owner: Dev-3
Reviewer: Birfy

<!-- flybot:checkpoint -->
```json
{
  "pr": 1823,
  "claim": "Dev-3",
  "issue": 1820,
  "summary": "The Project settings memory box now writes kind decision, the host's own default, so Save memory stores the text and shows the localized success line. Refusals are quoted through hostText, with dictionary entries only for the three refusals this panel's write paths can surface. The stub that pinned kind note became a real projectEndpoint round-trip, with controls that decision lands and note writes nothing. No display surface is added; the future P-228 panel labels these rows memory entries.",
  "files": ["core/src/client/project-board.js", "core/src/client/i18n.js", "core/test/project-board-panels.test.js", "docs/subsystems/core-client.md", "changelog.d/1820-project-memory-save.fixed.md", "core/client.js", "patches/ds-bot/0001-core-fixes.patch", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/src/client/project-board.js", "patches/ds-bot/files/test/project-board-panels.test.js"],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=2ea55d8a4d67cd1e4fe489ab2b14b1946cea4dd6 npm run verify", "result": "green", "at": "7c374bb32f38241d4d0f937f4f4ddcd11f45057c" },
  "next": "none — ready for review"
}
```
<!-- /flybot:checkpoint -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

handoff claim reads handoff.to while the wake reads queueRole: a reworked card is re-offered to the reworker and still claimable by the Lead

2 participants