Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions coworker/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -1837,7 +1837,17 @@ async def _handle_ask_user(self, tool_call: ToolCall) -> AsyncIterator[Event]:
"error": "no response",
}

status = "ok" if (result.get("answer") or result.get("answers")) else "denied"
# A skipped question answers None (OPE-153), so an all-skipped card is "denied" like an
# unanswered one — `answers` being non-empty isn't enough on its own.
replies = result.get("answers")
status = (
"ok"
if (
result.get("answer")
or (isinstance(replies, dict) and any(v is not None for v in replies.values()))
)
else "denied"
)
if status == "ok":
self._note_ask_replies(result, question)
self.messages.append(_tool_result_message(tool_call, result))
Expand Down Expand Up @@ -1872,8 +1882,10 @@ def _note_ask_replies(
self._reviewer_denials = 0
anchor = sum(1 for m in self.messages if m.get("role") == "user")
answers = result.get("answers")
# Skipped questions answer None (OPE-153) — dropped here, else str(None) files "None"
# as something the user actually said.
values = (
[str(v) for v in answers.values()]
[str(v) for v in answers.values() if v is not None]
if isinstance(answers, dict)
else [str(result.get("answer") or "")]
)
Expand Down
11 changes: 9 additions & 2 deletions coworker/interactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from typing import Optional

from .inbox import KIND_APPROVAL, KIND_QUESTION
from .tools.ask import option_label
from .tools.ask import SKIP_SENTINEL, option_label


@dataclass
Expand Down Expand Up @@ -56,8 +56,15 @@ def buttons_for(item) -> list[Button]:
if item.kind == KIND_QUESTION and getattr(item, "options", None):
# One button per option; the resolution IS the chosen option's label (what the agent
# gets). Rich {label, description, …} options button as their label.
#
# Skip trails the options (OPE-153) so the channel has the same way out the card has:
# without it, a reader whose answer isn't on the list can only pick a wrong option or
# leave the agent suspended. It grants nobody new authority — anyone who can click an
# option here could already decide the question — it only adds declining to what they
# can already do. Grouped questions stay button-less on purpose: they fall back to the
# "(Open the app to respond.)" text, and the app is where their per-question skip lives.
return [
Button(option_label(opt), encode(item.id, option_label(opt)))
for opt in item.options
]
] + [Button("Skip", encode(item.id, SKIP_SENTINEL))]
return []
68 changes: 63 additions & 5 deletions coworker/tools/ask.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,24 @@
# How many questions one grouped call may carry (stepper chips get unreadable past this).
MAX_GROUPED_QUESTIONS = 4

# OPE-153 — the resolution string a card sends when the user declines to answer. The resolution
# field otherwise carries the answer verbatim, so "skipped" needs a value no answer can collide
# with; the GUI writes the same constant (see SKIP in InboxItemCard.tsx — keep the two in step).
# A grouped card marks a per-question skip by using it as that question's value in the answer map,
# and a whole-card skip by filling every unanswered question with it.
SKIP_SENTINEL = "__ocw_skip__"

# Told to the agent alongside a skipped answer. Only the no-re-ask half is absolute — it's the
# anti-loop guarantee the skip exists for. "Pick a default" is deliberately conditional: the same
# note fires for "you choose the chart colour" and for "Staging or Production?", and a model that
# follows instructions literally would deploy off a shrug. Proceeding without deciding is offered
# as a third path so that judgment is asked for, not left to whichever model happens to be driving.
SKIP_NOTE = (
"The user chose not to answer. Do not put this question back to them. If you can proceed "
"safely, pick a sensible default and say which way you went. If the choice is consequential "
"or hard to undo, don't guess — say what you'd need and let them come back to it."
)

# An option is a plain string OR a rich object. `label` is what the user picks (and what comes
# back as the answer); `description` renders under it; `recommended` adds the green tag (put the
# recommended option first); `preview` is monospace text shown in the side pane (code, config,
Expand Down Expand Up @@ -143,6 +161,12 @@ def ask_user(
Grouped form (`questions`) returns `{"answers": {"<header or question>": "..."}}` — one
entry per question. Don't ask what you can reasonably decide yourself; reserve this for
choices that are actually the user's to make.

The user may SKIP any question. A skipped answer comes back as `null` (never ""), with
`skipped` naming what was declined. Treat it as "you decide" where the choice is safe:
pick a sensible default, continue, and say which way you went. Where it isn't — anything
consequential or hard to undo — don't guess; say what you'd need instead. Either way,
never re-ask a skipped question.
"""
# Real handling lives in the engine (it needs the out-of-band Inbox round-trip). This body
# only runs if no question_asker is wired (e.g. a headless surface).
Expand Down Expand Up @@ -242,22 +266,56 @@ def question_item_fields(args: dict) -> dict | None:
}


def question_key(entry) -> str:
"""The answer-map key for one grouped question — its header, else the question text. The
same key the card writes when it resolves, so the two halves agree."""
if not isinstance(entry, dict):
return "answer"
return str(entry.get("header") or entry.get("question") or "answer")


def answer_result(item_questions: list, resolution: str | None) -> dict:
"""Shape the ask_user tool result from an Inbox item's resolution string. Grouped items
resolve with a JSON object string keyed by header-or-question → `{"answers": {...}}`;
everything else returns the plain `{"answer": str}` shape."""
everything else returns the plain `{"answer": str}` shape.

A skipped question (OPE-153) answers `None` rather than "", so the agent can tell "the user
declined" from "the answer came back empty" — the two used to be the same value. Skips also
carry `skipped` (the question keys, or True for a lone question) and `SKIP_NOTE`."""
if item_questions:
# A text-only surface (mirrored channel) can only send the bare sentinel: that skips the
# whole card, so every question answers None.
if resolution == SKIP_SENTINEL:
keys = [question_key(q) for q in item_questions]
return {
"answers": dict.fromkeys(keys),
"skipped": keys,
"note": SKIP_NOTE,
}
try:
parsed = json.loads(resolution or "")
except (ValueError, TypeError):
parsed = None
if isinstance(parsed, dict):
return {"answers": {str(k): str(v) for k, v in parsed.items()}}
answers: dict[str, str | None] = {}
skipped: list[str] = []
for k, v in parsed.items():
key = str(k)
if v is None or str(v) == SKIP_SENTINEL:
answers[key] = None
skipped.append(key)
else:
answers[key] = str(v)
result: dict = {"answers": answers}
if skipped:
result["skipped"] = skipped
result["note"] = SKIP_NOTE
return result
if resolution:
# Answered from a text-only surface (e.g. a mirrored channel): attribute the lone
# answer to the first question rather than losing it.
first = item_questions[0] if isinstance(item_questions[0], dict) else {}
key = str(first.get("header") or first.get("question") or "answer")
return {"answers": {key: str(resolution)}}
return {"answers": {question_key(item_questions[0]): str(resolution)}}
return {"answer": ""}
if resolution == SKIP_SENTINEL:
return {"answer": None, "skipped": True, "note": SKIP_NOTE}
return {"answer": resolution or ""}
163 changes: 163 additions & 0 deletions surfaces/gui/e2e/ask-skip.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
import type { Page } from "@playwright/test";
import { test, expect } from "./fixtures";

// OPE-153 — skipping a question. A question card used to have no exit: the only ways out were
// picking an option or typing an answer, so a user who couldn't answer had to invent one or
// abandon the session. Now every card carries Skip, and grouped cards carry both "Skip question"
// (this one, advance) and "Skip all" (resolve the rest). Seeded via a per-test inbox route
// override so the base fixtures' counts stay untouched.

// Mirrors SKIP in InboxItemCard.tsx / SKIP_SENTINEL in coworker/tools/ask.py.
const SKIP = "__ocw_skip__";

const BASE = {
body: "",
state: "pending",
resolution: null as string | null,
inbox: "default",
created_at: "2026-08-31 08:00:00",
resolved_at: null as string | null,
session_title: "Investigate alerts",
session_agent: "ops",
session_workspace: "",
session_exists: true,
};

const SINGLE_ITEM = {
...BASE,
id: "inb-question-single",
session_id: "ops-1",
kind: "question",
title: "Which environment should I deploy to?",
header: "Environment",
options: ["Staging", "Production"],
allow_text: true,
multi: false,
questions: [],
};

// The card a user could previously get stuck on: exhaustive options, no free-text escape.
const NO_TEXT_ITEM = {
...SINGLE_ITEM,
id: "inb-question-notext",
allow_text: false,
};

const GROUPED_ITEM = {
...BASE,
id: "inb-question-grouped",
session_id: "ops-1",
kind: "question",
title: "Chart style?",
header: "Chart style",
options: ["Bar", "Line"],
allow_text: false,
multi: false,
questions: [
{ question: "Chart style?", header: "Chart style", options: ["Bar", "Line"], allow_text: false, multi: false },
{ question: "Which distribution?", header: "Distribution", options: ["Stacked", "Grouped"], allow_text: false, multi: false },
{ question: "Which palette?", header: "Palette", options: ["Warm", "Cool"], allow_text: false, multi: false },
],
};

/** Replace the Inbox's seeded items for this test (resolve mutates the local copy). */
async function seedInbox(page: Page, items: Record<string, unknown>[]) {
const inbox = items.map((i) => ({ ...i }));
const json = (body: unknown) => ({
status: 200,
contentType: "application/json",
body: JSON.stringify(body),
});
await page.route(/\/v1\/inbox\/[^/]+\/resolve$/, (route) => {
const path = new URL(route.request().url()).pathname;
const id = decodeURIComponent(path.split("/").slice(-2)[0]);
const it = inbox.find((x) => x.id === id);
if (it) {
it.state = "resolved";
it.resolution = route.request().postDataJSON().resolution;
}
return route.fulfill(json({ ok: true }));
});
await page.route(/\/v1\/inbox(\?.*)?$/, (route) =>
route.fulfill(json({ items: inbox.filter((i) => i.state === "pending") })),
);
return inbox;
}

async function openInbox(page: Page, expectTitle: string) {
await page.goto("/");
await page.getByTestId("inbox-chip").click();
await expect(page.getByText(expectTitle)).toBeVisible();
}

test("a single question can be skipped, and resolves as skipped rather than blank", async ({
page,
}) => {
await seedInbox(page, [SINGLE_ITEM]);
await openInbox(page, "Which environment should I deploy to?");

// One control, labelled plainly — no stepper, so nothing to say "question" about.
const skip = page.getByTestId("question-skip");
await expect(skip.getByRole("button", { name: "Skip", exact: true })).toBeVisible();
await expect(skip.getByRole("button", { name: "Skip all" })).toHaveCount(0);

const resolved = page.waitForRequest((r) => r.url().includes("/resolve") && r.method() === "POST");
await skip.getByRole("button", { name: "Skip", exact: true }).click();
expect((await resolved).postDataJSON().resolution).toBe(SKIP);
await expect(page.getByText("Nothing pending.")).toBeVisible();
});

test("Skip is offered even when the card has no free-text escape", async ({ page }) => {
await seedInbox(page, [NO_TEXT_ITEM]);
await openInbox(page, "Which environment should I deploy to?");

// Exhaustive options and no "Or type your own answer…" — Skip is the ONLY way out.
await expect(page.getByPlaceholder("Or type your own answer…")).toHaveCount(0);
const resolved = page.waitForRequest((r) => r.url().includes("/resolve") && r.method() === "POST");
await page.getByTestId("question-skip").getByRole("button", { name: "Skip", exact: true }).click();
expect((await resolved).postDataJSON().resolution).toBe(SKIP);
});

test("one step of a grouped card can be skipped while the others are answered", async ({
page,
}) => {
await seedInbox(page, [GROUPED_ITEM]);
await openInbox(page, "Chart style?");

const stepper = page.getByTestId("question-stepper");
const skip = page.getByTestId("question-skip");

// Skipping step 1 advances exactly as answering would.
await skip.getByRole("button", { name: "Skip question" }).click();
await expect(stepper).toContainText("2 of 3");

await page.getByRole("button", { name: "Stacked", exact: true }).click();
await expect(stepper).toContainText("3 of 3");

// Last step: nothing left to "skip all", so only the per-question control remains.
await expect(skip.getByRole("button", { name: "Skip all" })).toHaveCount(0);

const resolved = page.waitForRequest((r) => r.url().includes("/resolve") && r.method() === "POST");
await page.getByRole("button", { name: "Warm", exact: true }).click();
expect((await resolved).postDataJSON().resolution).toBe(
JSON.stringify({ "Chart style": SKIP, Distribution: "Stacked", Palette: "Warm" }),
);
});

test("Skip all resolves the rest of a grouped card but keeps answers already given", async ({
page,
}) => {
await seedInbox(page, [GROUPED_ITEM]);
await openInbox(page, "Chart style?");

await page.getByRole("button", { name: "Bar", exact: true }).click();
await expect(page.getByTestId("question-stepper")).toContainText("2 of 3");

const resolved = page.waitForRequest((r) => r.url().includes("/resolve") && r.method() === "POST");
await page.getByTestId("question-skip").getByRole("button", { name: "Skip all" }).click();
// Step 1's real answer survives; only the two the user never reached are skipped.
expect((await resolved).postDataJSON().resolution).toBe(
JSON.stringify({ "Chart style": "Bar", Distribution: SKIP, Palette: SKIP }),
);
await expect(page.getByText("Nothing pending.")).toBeVisible();
});
40 changes: 40 additions & 0 deletions surfaces/gui/src/components/InboxItemCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ const BTN_BORDERED =
const BTN_ACCENT =
"px-3 py-1.5 rounded-lg border border-accent text-accent text-[13px] font-semibold hover:bg-accentSoft";
const BTN_QUIET = "px-3 py-1.5 text-[13px] text-faint hover:text-danger";
// Skip (OPE-153) is an escape, not a rejection — quiet like Deny, without the danger red. Uses
// `muted` rather than Deny's `faint`: a borderless text button is already secondary next to the
// option pills, and this one has to be FOUND (faint is only ~3:1 on the dark panel).
const BTN_SKIP = "px-3 py-1.5 text-[13px] text-muted hover:text-ink";
const OPT_BASE =
"inline-flex items-center gap-1.5 px-3 py-1.5 rounded-full border text-[13px] transition-colors";
const OPT_OFF = "border-line bg-paper text-ink hover:border-accent hover:bg-accentSoft/50";
Expand All @@ -39,6 +43,11 @@ const ROW_BASE = "w-full text-left rounded-lg border px-3 py-2 transition-colors
const ROW_OFF = "border-line bg-paper hover:border-accent hover:bg-accentSoft/50";
const ROW_ON = "border-accent bg-accentSoft";

// The resolution a card sends when the user declines to answer (OPE-153). Mirrors
// SKIP_SENTINEL in coworker/tools/ask.py, which turns it back into a null answer —
// keep the two constants in step.
const SKIP = "__ocw_skip__";

// -- question normalization ---------------------------------------------------

interface NormOption {
Expand Down Expand Up @@ -248,6 +257,25 @@ function QuestionCard({
else onResolve(item.id, JSON.stringify(all));
};

// Skipping ONE question is just answering it with the sentinel — the stepper advances (or the
// card resolves on the last step) exactly as a real answer would.
const skipStep = () => submit(SKIP);

// Skipping the WHOLE card resolves it now, marking every question the user never got to.
// Answers already given are kept: "skip all" means "don't ask me the rest", not "discard".
const skipAll = () => {
if (!grouped) {
onResolve(item.id, SKIP);
return;
}
const all = { ...answers };
for (let i = step; i < specs.length; i++) {
const k = keyFor(specs[i]);
if (!(k in all)) all[k] = SKIP;
}
onResolve(item.id, JSON.stringify(all));
};

return (
<>
{/* Stepper chips (grouped): "Chart style · 1 of 2 · Distribution ›" — ‹ steps back. */}
Expand Down Expand Up @@ -286,6 +314,18 @@ function QuestionCard({
{chip}
{/* key={step} resets selection/text/hover state when the stepper advances */}
<QuestionBlock key={step} spec={spec} onAnswer={submit} />
{/* OPE-153: the way out. Always present — a card with exhaustive options and
allow_text:false is exactly where the user would otherwise be stuck. */}
<div className="flex items-center gap-1 mt-1.5" data-testid="question-skip">
<button className={BTN_SKIP} onClick={skipStep}>
{grouped ? t("inbox.skip_question") : t("inbox.skip")}
</button>
{grouped && step + 1 < specs.length && (
<button className={BTN_SKIP} onClick={skipAll}>
{t("inbox.skip_all")}
</button>
)}
</div>
</>
);
}
Expand Down
3 changes: 3 additions & 0 deletions surfaces/gui/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,9 @@
"send_count": "Send ({{count}})",
"or_type_answer": "Or type your own answer…",
"your_answer": "Your answer…",
"skip": "Skip",
"skip_question": "Skip question",
"skip_all": "Skip all",
"grant": "Grant",
"grant_no_folder": "Grant (no folder)",
"no_folder_suggested": "No folder was suggested",
Expand Down
Loading
Loading