mirror of
https://github.com/mvschwarz/openrig.git
synced 2026-11-07 05:56:21 -10:00
fix(slack): a click on a re-parked ask records a new answer for that park and wakes its asker (#2421)
A click on a re-parked Slack ask records the person's new choice for that park and wakes the agent, instead of silently resuming it with the person's earlier answer. The click branch looks up the current park's answer, as typed replies already do; a direct answer to a blocked ask, typed or clicked, carries the park's episode key, so a re-park's click gets its own answer row, which is the owner's wake; and a complete stored set is final only for a pending ask, or for a park this person has already answered, with other questions' answers carried forward as on a typed re-park. The unused typedReply input flag is removed. The committed-answer guard for another person's answer to the current park and the winner check are unchanged, and a pending ask's clicks behave as before. Source +11/-7 across three files; the pinned re-park click test now expects the new answer, plus one cell for a re-park over a complete set.
This commit is contained in:
1 parent
fa257fa12c
commit
d7fc1fce0c
5 files changed
+34
-13
No files matched your search
@@ -419,7 +419,7 @@ export class InboundRouter {
|
||||
summary,
|
||||
body,
|
||||
...(route.correlationQitemId ? { inReplyTo: route.correlationQitemId } : {}),
|
||||
...(askOpen && answerable ? { humanAnswer: true, typedReply: true } : {}),
|
||||
...(askOpen && answerable ? { humanAnswer: true } : {}),
|
||||
});
|
||||
} catch (e) {
|
||||
this.deps.log?.(`qitem create failed ts=${ts}: ${(e as Error).message}`);
|
||||
@@ -563,7 +563,10 @@ export class InboundRouter {
|
||||
if (live) await acknowledge(CLICK_REPLIES.unmapped);
|
||||
return { status: "ignored", reason: "unmapped-message" };
|
||||
}
|
||||
const existing = this.deps.queue.getHumanAnswer?.(qitemId, who.source);
|
||||
// A decision parked on this person takes the current question's answer, as a typed reply does: a re-park gets its own.
|
||||
const parkOpen = this.deps.queue.getQitem?.(qitemId)?.state === "blocked"
|
||||
&& this.deps.queue.answersPark?.(qitemId, who.source, action?.action_ts) === true;
|
||||
const existing = parkOpen ? this.deps.queue.getEpisodeAnswer?.(qitemId, who.source) : this.deps.queue.getHumanAnswer?.(qitemId, who.source);
|
||||
if (existing) {
|
||||
try {
|
||||
const resolution = await this.deps.resolveHumanReply?.({ qitemId, actorSession: who.source, decision: humanAnswerText(existing.body), messageTs: action?.action_ts });
|
||||
|
||||
@@ -66,8 +66,6 @@ export interface CreateQitemInput {
|
||||
inReplyTo?: string;
|
||||
/** A current typed/button answer, never a reaction or a recovered late message. */
|
||||
humanAnswer?: boolean;
|
||||
/** The answer is a typed thread reply: on a decision parked on its person, its row belongs to the current question. */
|
||||
typedReply?: boolean;
|
||||
}
|
||||
|
||||
/** What the inbound router needs: land a durable qitem, get its id (or throw). */
|
||||
@@ -268,8 +266,8 @@ export function makeQueuePorts(
|
||||
};
|
||||
let created = directAnswer && input.inReplyTo
|
||||
? await queueRepo.createHumanAnswer({ ...createInput, inboundReplyTo: input.inReplyTo,
|
||||
// A parked decision's answer belongs to its current question, so a re-park gets its own.
|
||||
...(input.typedReply && ask?.state === "blocked" ? { episodeKey: queueRepo.currentHumanRequiredEpisode(input.inReplyTo)?.key } : {}) })
|
||||
// A parked decision's answer, typed or clicked, belongs to its current question, so a re-park gets its own.
|
||||
...(ask?.state === "blocked" ? { episodeKey: queueRepo.currentHumanRequiredEpisode(input.inReplyTo)?.key } : {}) })
|
||||
: await queueRepo.create(createInput);
|
||||
// Another registered person's answer already settled the ask: these words still reach the agent, as correspondence.
|
||||
if (directAnswer && created.qitemId !== input.inReplyTo && created.sourceSession !== input.source) {
|
||||
|
||||
@@ -3911,7 +3911,10 @@ export class QueueRepository {
|
||||
const question = item.humanQuestions.find((q) => q.id === input.questionId);
|
||||
if (!question?.options.some((o) => o.id === input.optionId)) return { status: "not-applicable" as const, reason: "unknown-option" };
|
||||
const recorded = item.humanAnswers ?? {};
|
||||
if (unansweredQuestions(item.humanQuestions, recorded).length === 0) {
|
||||
// A park's set is final once this park has its answer: until then a complete set is an earlier park's, and a click
|
||||
// on the re-park records over it.
|
||||
if (unansweredQuestions(item.humanQuestions, recorded).length === 0
|
||||
&& (item.state === "pending" || this.currentEpisodeAnswer(item.qitemId, input.actorSession))) {
|
||||
return { status: "recorded" as const, answers: recorded, complete: true, questions: item.humanQuestions };
|
||||
}
|
||||
const answers: HumanAnswers = { ...(item.humanAnswers ?? {}), [input.questionId]: input.optionId };
|
||||
|
||||
@@ -120,7 +120,7 @@ describe("shared-workspace people", () => {
|
||||
// One answer per ask on every surface, including a process stopped between a committed answer and the close.
|
||||
const ports = () => makeQueuePorts(repo, { loadHumanRegistry: () => loadHumanRegistry(home), sharedChannels: () => [] });
|
||||
const slackAnswer = (askId: string, who: string, words: string) => ports().createQitem({ qitemId: `qitem-slack-inbound-${who.split("@")[0]}-${askId}`,
|
||||
source: who, destination: ASKER, summary: words, body: words, inReplyTo: askId, humanAnswer: true, typedReply: true });
|
||||
source: who, destination: ASKER, summary: words, body: words, inReplyTo: askId, humanAnswer: true });
|
||||
|
||||
it("Bob's Slack answer committed before Sarah answers in the Feed: Sarah is told Bob answered, and it settles once, as Bob's", async () => {
|
||||
register("sarah", "bob");
|
||||
|
||||
@@ -346,7 +346,6 @@ describe("update --reply-to an earlier item's thread (#96)", () => {
|
||||
actions: [{ type: "button", block_id: `or-q:${questionId}`, action_id: `or-opt:${optionId}`, value: optionId, action_ts: actionTs }],
|
||||
},
|
||||
}) });
|
||||
const answerRowId = (qitemId: string) => `qitem-human-answer-${createHash("sha256").update(qitemId).digest("hex").slice(0, 20)}`;
|
||||
const resolvedCount = (qitemId: string) => repo.transitionLog.listForQitem(qitemId)
|
||||
.filter((t) => t.ownerNotificationKind === "human-decision-resolved").length;
|
||||
// The ask posts as root 1.1; its agent then parks it on the person it asked, and that question posts too.
|
||||
@@ -368,7 +367,7 @@ describe("update --reply-to an earlier item's thread (#96)", () => {
|
||||
click("q2", "no", "991.1");
|
||||
await vi.waitFor(() => expect(repo.getById(ask.qitemId)?.state).toBe("in-progress"));
|
||||
expect(repo.getById(ask.qitemId)?.humanAnswers).toEqual({ q1: "yes", q2: "no" });
|
||||
expect(repo.getById(answerRowId(ask.qitemId))).toMatchObject({ destinationSession: "author@rig" });
|
||||
expect(humanAnswerRows(ask.qitemId)).toHaveLength(1);
|
||||
expect(resolvedCount(ask.qitemId)).toBe(1);
|
||||
click("q2", "no", "991.1");
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
@@ -485,8 +484,7 @@ describe("update --reply-to an earlier item's thread (#96)", () => {
|
||||
} finally { spy.mockRestore(); }
|
||||
});
|
||||
|
||||
// The click path keeps its own rules until its follow-up: only a typed reply stores its words as the answer.
|
||||
it("stores no earlier typed answer under a re-park's question when the person clicks", async () => {
|
||||
it("answers a re-park with the person's click, not their typed answer to the earlier question", async () => {
|
||||
const ask = await parkedAsk();
|
||||
typed("large", "990.1");
|
||||
await vi.waitFor(() => expect(repo.getById(ask.qitemId)?.state).toBe("in-progress"));
|
||||
@@ -495,7 +493,26 @@ describe("update --reply-to an earlier item's thread (#96)", () => {
|
||||
await postPendingNotice(ask.qitemId);
|
||||
click("q2", "yes", "996.1");
|
||||
await vi.waitFor(() => expect(resolvedCount(ask.qitemId)).toBe(2));
|
||||
expect(repo.getById(ask.qitemId)?.humanAnswers?.q2).toBeUndefined();
|
||||
expect(repo.getById(ask.qitemId)?.humanAnswers).toEqual({ q1: typedAnswer("large", 1), q2: "yes" });
|
||||
expect(humanAnswerRows(ask.qitemId)).toHaveLength(2);
|
||||
expect(posts.some((post) => String(post.text ?? "").startsWith("Already answered"))).toBe(false);
|
||||
});
|
||||
|
||||
it("answers a re-park's click with a new answer to the asker, over the earlier park's complete set", async () => {
|
||||
const ask = await parkedAsk();
|
||||
click("q1", "yes", "990.1");
|
||||
await vi.waitFor(() => expect(repo.getById(ask.qitemId)?.humanAnswers).toEqual({ q1: "yes" }));
|
||||
click("q2", "yes", "991.1");
|
||||
await vi.waitFor(() => expect(repo.getById(ask.qitemId)?.state).toBe("in-progress"));
|
||||
repo.update({ qitemId: ask.qitemId, actorSession: "author@rig", state: "blocked", blockedOn: human, summary: "Tell the users after all?",
|
||||
evidenceRef: "/private/proof.md", transitionNote: "parked again on the second question" });
|
||||
await postPendingNotice(ask.qitemId);
|
||||
click("q2", "no", "996.1");
|
||||
await vi.waitFor(() => expect(resolvedCount(ask.qitemId)).toBe(2));
|
||||
expect(repo.getById(ask.qitemId)?.humanAnswers).toEqual({ q1: "yes", q2: "no" });
|
||||
expect(humanAnswerRows(ask.qitemId)).toHaveLength(2);
|
||||
expect(humanAnswerRows(ask.qitemId).filter((row) => row.body.includes(`"q2":"no"`))).toHaveLength(1);
|
||||
expect(posts.some((post) => String(post.text ?? "").startsWith("Already answered"))).toBe(false);
|
||||
});
|
||||
|
||||
it("returns nothing again when a re-park's interrupted answer is replayed after the ask closes", async () => {
|
||||
|
||||
Reference in new issue
Block a user