diff --git a/docs/design.md b/docs/design.md index c43416a..d63c0cc 100644 --- a/docs/design.md +++ b/docs/design.md @@ -156,7 +156,7 @@ Every artifact moves through the same four moments. None of them is a state the **Fulfillment.** One turn produces exactly one artifact answering the request. Implementation-type artifacts link branch, commit, and PR (§10). -**Review.** Every artifact card has a Review button — it writes an `artifactRequest` with `type: "review"` and the artifact as `subject`; the fulfilling turn's terminal record is a `review` verdict (referencing the request) rather than an artifact. The **auto-review toggle** (per artifact type per project, overridable per request) makes the daemon write an **open** review request automatically the moment a matching artifact lands. One eligible local agent identity authors the record (§3), but it is not assigned to that identity: ordinary claims arbitrate execution among every capable agent in the space, including agents run by other operators (§9). The reviewer needs no distinct identity — the same agent DID may still win the claim and review its own artifact: what buys a review its value is a fresh context (every turn starts from nothing, §9) plus, when the operator configures it, a different model routed to the `review` type (§11). Auto-review is deliberately terminal: a verdict never triggers further generation. It is the only automation in v1, and it is exactly one hop. +**Review.** Every artifact card has a Review button — it writes an `artifactRequest` with `type: "review"` and the artifact as `subject`; the fulfilling turn's terminal record is a `review` verdict (referencing the request) rather than an artifact. The **auto-review toggle** (per artifact type per project, overridable per request) makes the daemon write an **open** review request automatically the moment a matching artifact lands. One eligible local agent identity authors the record (§3), but it is not assigned to that identity: ordinary claims arbitrate execution among every capable agent in the space, including agents run by other operators (§9). That bounds duplicate execution per request, not authorship: two operators can still author separate requests for the same artifact before either observes the other's record. The reviewer needs no distinct identity — the same agent DID may still win the claim and review its own artifact: what buys a review its value is a fresh context (every turn starts from nothing, §9) plus, when the operator configures it, a different model routed to the `review` type (§11). Auto-review is deliberately terminal: a verdict never triggers further generation. It is the only automation in v1, and it is exactly one hop. **Supersession.** A `request_changes` verdict blocks nothing mechanically — it is information on the card. The human reads the findings and decides whether to request a v2 (`prev`-linked, findings included in the new turn's bundle) or move on. Radial is **review-annotated, not review-gated**: a team that wants "no implementation without an approved plan" gets it as soft UI policy (the button warns when the basis plan lacks an approving verdict), not a protocol guarantee. That's acceptable precisely because the thing gates protected against — agents autonomously spending tokens past an unapproved plan — cannot happen when every generation has a human click behind it. diff --git a/packages/core/src/materializer.ts b/packages/core/src/materializer.ts index e775f12..33bcd28 100644 --- a/packages/core/src/materializer.ts +++ b/packages/core/src/materializer.ts @@ -523,14 +523,18 @@ interface ViewInput { // not just this target's `messages` (which drive the displayed thread). Matching is by exact // request/question strongref, so the wider scan never mis-attributes across targets. allMessages: Array> + // Active agent membership distinguishes a turn's question from an ordinary member comment. + // This is convergent fold state, not a claim or observer-local dispatch decision, so open + // requests remain blocked for every operator after the agent holding a claim asks for input. + agentDids: Set // refKeys (uri#cid) of requests a trusted retract tombstone has withdrawn. Global set, keyed by // strongref; buildView only sees this target's requests, so cross-target keys never collide. retractedRefs: Set asOf: string } -// A request is awaitingInput iff its assignee has posted an unanswered trusted -// question against it: a message `q` with `q.did === request.value.assignee`, +// A request is awaitingInput iff an active agent member has posted an unanswered trusted +// question against it: a message `q` whose author is in `agentDids`, // `q.value.re` pointing at the request (exact uri#cid match), and `q.value.declines` // not `true`, for which no trusted message `a` (authored by a different member, // i.e. `a.did !== q.did`) has `a.value.parent` pointing back at `q`. This is purely @@ -541,14 +545,14 @@ interface ViewInput { function computeAwaitingInput( requests: Array>, messages: Array>, + agentDids: Set, ): string[] { const awaiting = new Set() for (const request of requests) { - if (!request.value.assignee) continue const requestKey = recordRefKey(request) const questions = messages.filter( (message) => - message.did === request.value.assignee && + agentDids.has(message.did) && message.value.re && refKey(message.value.re) === requestKey && message.value.declines !== true && @@ -651,7 +655,7 @@ function buildView(input: ViewInput): T requests: input.requests.sort(compareRecord), openRequests: openRequests.sort(compareRecord), retracted: retracted.sort(compareRecord), - awaitingInput: computeAwaitingInput(input.requests, input.allMessages), + awaitingInput: computeAwaitingInput(input.requests, input.allMessages, input.agentDids), artifacts: input.artifacts.sort(compareRecord), artifactChains: buildChains(input.artifacts), reviews: input.reviews @@ -1106,6 +1110,7 @@ export function materialize(store: RecordStore, options: MaterializeOptions): Ma messages: messages.filter((message) => targetUriForMessage(message) === target.uri), claims: claims.filter((claim) => targetUriForClaim(claim) === target.uri), allMessages: messages, + agentDids, retractedRefs, asOf, }) diff --git a/packages/core/test/awaiting-input.test.mjs b/packages/core/test/awaiting-input.test.mjs index 14fa8b6..5ce9b77 100644 --- a/packages/core/test/awaiting-input.test.mjs +++ b/packages/core/test/awaiting-input.test.mjs @@ -136,7 +136,8 @@ function awaitingScenario() { }), ) - // Case 2b: a `re` message from someone other than the assignee -> not awaitingInput. + // Case 2b: a `re` message from another active agent member is also a question. This matters for + // open requests, whose claimant is not represented by an assignee in the convergent fold. const requestUnrelatedWrongAuthor = add( make( HUMAN, @@ -157,7 +158,7 @@ function awaitingScenario() { make(AGENT_D, COLLECTIONS.message, 'unrelated-wrong-author', 'cid-unrelated-wrong-author', { $type: COLLECTIONS.message, goal: ref(goal), - body: 'Bystander comment.', + body: 'Question from another active agent.', mentions: [], re: ref(requestUnrelatedWrongAuthor), createdAt: '2026-01-01T00:03:21Z', @@ -347,12 +348,54 @@ describe('TargetView.awaitingInput', () => { assert.ok(goal.awaitingInput.includes(scenario.requestQuestionOnlyUri)) }) - it('ignores a non-question message and a `re` from a non-assignee', () => { + it('ignores a non-question message but accepts a question from any active agent', () => { const scenario = awaitingScenario() const index = build(scenario.records, scenario.spaceUri) const goal = index.goals.find((view) => view.target.uri === scenario.goalUri) assert.equal(goal.awaitingInput.includes(scenario.requestUnrelatedNoReUri), false) - assert.equal(goal.awaitingInput.includes(scenario.requestUnrelatedWrongAuthorUri), false) + assert.ok(goal.awaitingInput.includes(scenario.requestUnrelatedWrongAuthorUri)) + }) + + it('flags an open request after an active agent asks a question', () => { + const scenario = awaitingScenario() + const request = scenario.records.find((record) => record.uri === scenario.requestQuestionOnlyUri) + delete request.value.assignee + const index = build(scenario.records, scenario.spaceUri) + const goal = index.goals.find((view) => view.target.uri === scenario.goalUri) + assert.ok(goal.awaitingInput.includes(scenario.requestQuestionOnlyUri)) + }) + + it('does not treat a human comment on an open request as an agent question', () => { + const scenario = awaitingScenario() + const request = scenario.records.find((record) => record.uri === scenario.requestQuestionOnlyUri) + delete request.value.assignee + const question = scenario.records.find((record) => record === scenario.questionOnlyQ) + question.did = HUMAN + question.uri = `at://${HUMAN}/${question.collection}/${question.rkey}` + const index = build(scenario.records, scenario.spaceUri) + const goal = index.goals.find((view) => view.target.uri === scenario.goalUri) + assert.equal(goal.awaitingInput.includes(scenario.requestQuestionOnlyUri), false) + }) + + it('does not treat a removed agent\'s message as an active turn question', () => { + const scenario = awaitingScenario() + const request = scenario.records.find((record) => record.uri === scenario.requestQuestionOnlyUri) + const member = scenario.records.find( + (record) => record.collection === COLLECTIONS.addMember && record.value.did === AGENT_A, + ) + delete request.value.assignee + scenario.records.push( + make(ROOT, COLLECTIONS.removeMember, 'remove-agent-a', 'cid-remove-agent-a', { + $type: COLLECTIONS.removeMember, + space: member.value.space, + did: AGENT_A, + membership: ref(member), + createdAt: '2026-01-01T00:05:00Z', + }), + ) + const index = build(scenario.records, scenario.spaceUri) + const goal = index.goals.find((view) => view.target.uri === scenario.goalUri) + assert.equal(goal.awaitingInput.includes(scenario.requestQuestionOnlyUri), false) }) it('clears once a different member answers the question', () => { diff --git a/packages/daemon/README.md b/packages/daemon/README.md index 96dc44f..3c43815 100644 --- a/packages/daemon/README.md +++ b/packages/daemon/README.md @@ -26,7 +26,9 @@ why the default lease is short. Auto-review uses this open-request path. The daemon chooses one eligible local review-capable agent identity to author the single review request, but omits `assignee`; any capable agent in the space, including one operated elsewhere, may claim and execute it. Authorship and execution are therefore -separate, at the intended cost of the normal claim-ingestion confirmation cycle. +separate, at the intended cost of the normal claim-ingestion confirmation cycle. Claims bound +duplicate execution of each request; two operators can still author separate auto-review requests +for the same artifact before either request has been ingested by the other. ## Forges diff --git a/packages/daemon/src/claims.ts b/packages/daemon/src/claims.ts index bec2145..5d5dfe5 100644 --- a/packages/daemon/src/claims.ts +++ b/packages/daemon/src/claims.ts @@ -122,7 +122,7 @@ const rowClaim = (row: ClaimRow): ClaimRecord | undefined => * resolves) — via the shared `resolveRequestType`, not a second copy of that rule; * - authored by an active member; * - producible by a loaded actor of ours that holds an active AGENT grant here; - * - not blocked on an unanswered question from that actor; + * - not blocked on an unanswered question from any active agent in the convergent fold; * - eligible in the turn ledger (a request we have already given up on is not worth claiming), OR * the subject of a turn of ours that is RUNNING — work in flight needs a covering claim, and that * is exactly what a claim retired at its horizon mid-turn has to get back; @@ -161,6 +161,7 @@ export function selectClaimable( const type = request.value.type if (!resolveRequestType(request, typeContext, expectedScope)) continue if (!index.members.some((member) => member.did === request.did && member.active)) continue + if (view.awaitingInput.includes(request.uri)) continue // A claim held by somebody else, still live in the fold, is theirs — do not contest it. (Our // OWN winning claim falls through to the ledger check below, which routes it to renewal.) @@ -182,6 +183,8 @@ export function selectClaimable( // `radiald claim reset` is the way back — it raises the row's own ceiling, so the walk resumes // ABOVE the spent generations instead of re-discovering them. if (row && row.generation >= (row.generationCap ?? MAX_CLAIM_GENERATIONS)) continue + // Retain the actor-specific scan for a stale index/ledger edge; normally the fold-wide gate + // above is the authoritative answer for both assigned and open requests. if (hasUnansweredQuestion(allMessages, request, actor.did)) continue // A turn of OURS already running is the one state where the turn ledger says "not eligible" // and claiming is still right. The gate exists so we do not claim work we would then refuse to diff --git a/packages/daemon/src/dispatch.ts b/packages/daemon/src/dispatch.ts index 811a12a..e617d4f 100644 --- a/packages/daemon/src/dispatch.ts +++ b/packages/daemon/src/dispatch.ts @@ -444,13 +444,12 @@ export function selectDispatchable( } } if (view.awaitingInput.includes(request.uri)) { - onReject?.(request.uri, 'blocked awaiting human input (unanswered assignee question)') + onReject?.(request.uri, 'blocked awaiting human input (unanswered agent question)') return undefined } - // `view.awaitingInput` only tracks ASSIGNED requests (materializer keys on `assignee`), so for an - // unassigned request the daemon must itself detect an unanswered question from the - // (deterministically) selected actor and hold off — otherwise it would redispatch on top of a - // pending question (finding 9). Redundant-but-harmless for assigned requests. + // Retain the selected-actor scan for the stale index/ledger edge documented above. Once the + // message is in the fold, `view.awaitingInput` is the space-global gate for assigned and open + // requests alike, so every operator sees the same block. if (hasUnansweredQuestion(allMessages, request, actor.did)) { onReject?.(request.uri, 'blocked awaiting human input (unanswered question from the selected actor)') return undefined diff --git a/packages/daemon/test/auto-review.test.mjs b/packages/daemon/test/auto-review.test.mjs index 068319d..31bf9c2 100644 --- a/packages/daemon/test/auto-review.test.mjs +++ b/packages/daemon/test/auto-review.test.mjs @@ -804,6 +804,22 @@ it('the emitted open review is claimable across operators and only the confirmed assert.equal(selectDispatchable(claimed, registryB, turnsB, undefined, undefined, new Set([emitted.uri])).length, 0) assert.equal(selectDispatchable(claimed, registryA, turnsA).length, 0, 'the winner still waits for local confirmation') + // The winner asks for input and its claim later expires. The question is space-global fold + // state, so another operator must neither reclaim nor dispatch the still-open review. + const question = stored(AGENT_A, 'review-question', { + $type: COLLECTIONS.message, + goal: emitted.value.goal, + body: 'Which compatibility target matters most?', + mentions: [HUMAN], + re: strongref(emitted), + createdAt: '2026-08-07T12:30:00Z', + }, 103, 'cid-review-question') + store.put(question) + const awaiting = materialize(store, { spaceUri: space.uri, asOf: '2026-08-07T14:00:00Z' }) + assert.deepEqual(awaiting.goals[0].awaitingInput, [emitted.uri]) + assert.equal(selectClaimable(awaiting, registryB, claimsB, turnsB, { budget: 1 }).length, 0) + assert.equal(selectDispatchable(awaiting, registryB, turnsB).length, 0) + const scopedOut = fakeActor(AGENT_B, ['review'], [], { [space.uri]: { artifactTypes: ['plan'] } }) const incapable = registry([scopedOut]) const incapableClaims = new ClaimLedger() diff --git a/packages/ui/src/lib/asks.ts b/packages/ui/src/lib/asks.ts index f12f0ca..26943c6 100644 --- a/packages/ui/src/lib/asks.ts +++ b/packages/ui/src/lib/asks.ts @@ -3,8 +3,8 @@ // A `review` and an `answer` are the two things an agent works on that are never units: neither // produces an artifact, so `timeline()` skips both and a goal's pie does not move when somebody asks // for a judgement or a reply. That was always right about the pie and always wrong about the smart -// lists — the daemon runs real turns for both (`dispatch.ts`), auto-review writes one assigned to its -// own reviewer agent, and the claim manager claims unassigned ones — so the most common shape of "an +// lists — the daemon runs real turns for both (`dispatch.ts`), auto-review writes an open review +// request, and the claim manager assigns its execution — so the most common shape of "an // agent is working right now" was the one shape "With an agent" could not see. // // This module exists so the two pages and the rail read ONE list rather than each unioning two