feat: let a background task ask the chat that started it, not just the Inbox
Nightly Build / build (push) Successful in 7m42s
Nightly Build / build (push) Successful in 7m42s
An async sub-agent runs in a session of its own, so the rich per-session events
that draw the inline approval card never reach the chat's socket — only the
id-only inbox lifecycle ones do. A task blocked on an approval was therefore
invisible in the conversation that started it, and the only way to unblock it
was to notice the sidebar badge and go to the Inbox.
The chat already shows what it handed off. This asks the same question of the
pending items: `GET /{source}/inbox` joins them against the sessions of this
conversation's running async jobs, so "whose is this" has one answer, in the
same place `/{source}/tasks` answers it for a task. The client is left with a
list to render, not a correlation to guess. The live path adds no event — the
existing `approval_requested` / `clarification_*` broadcasts already reach every
socket of the user, and re-reading the endpoint turns a nudge into something
renderable and survives a reload for free.
The card sits above the task strip rather than in the transcript: the task that
is asking may have been started twenty messages ago, and a card that scrolls
away is a card that gets missed. One at a time, with a count of what is behind
it — a blocked task stays blocked whether or not its card is on screen, so
stacking them would trade a readable chat for a queue nobody asked to see. And
it closes: the ✕ hides the card without resolving anything, leaving the item in
the Inbox, because a panel that cannot be moved takes the chat hostage.
`InboxCardsMixin` is the cards and their resolve calls, split out of
`InboxMixin` so the chat and the Inbox render the same approval rather than two
drifting copies of it; `_afterInboxResolve` is the only thing they disagree on.
Elicitations are left out: `PendingElicitationInfo` carries no `session_id`, so
there is nothing to attribute one to a task with.
Also: an async task's context label said "CronJob:", which sends whoever reads
the approval looking on the wrong page — and now says so next to the task's
real name.
This commit is contained in:
@@ -384,7 +384,14 @@ async fn run_job(
|
||||
}
|
||||
|
||||
let handler = session.get_or_create_handler(session_id).await?;
|
||||
handler.set_context_label(format!("CronJob: {}", job.title));
|
||||
// The label rides every pending item this run raises, so it is what a human
|
||||
// reads when asked to approve something. An async task is not on a schedule
|
||||
// and calling it a cron job sends them looking on the wrong page — which
|
||||
// now shows next to the task's real name in the chat's own card.
|
||||
handler.set_context_label(match job.kind.as_str() {
|
||||
"async" => format!("Task: {}", job.title),
|
||||
_ => format!("CronJob: {}", job.title),
|
||||
});
|
||||
if job.kind == "async" {
|
||||
if let Some(parent_id) = job.parent_session_id {
|
||||
handler.set_scratchpad_session_id(parent_id);
|
||||
|
||||
@@ -148,6 +148,48 @@ pub async fn list_for_parent_session(
|
||||
.collect())
|
||||
}
|
||||
|
||||
/// One live background task, reduced to what identifies its session.
|
||||
///
|
||||
/// The pairing an approval needs: a pending item names the session it was
|
||||
/// raised in, and this is what turns that id back into "the task «X» your
|
||||
/// conversation started".
|
||||
#[derive(Debug, Clone, sqlx::FromRow)]
|
||||
pub struct RunningChildSession {
|
||||
pub job_id: i64,
|
||||
pub title: String,
|
||||
pub session_id: i64,
|
||||
}
|
||||
|
||||
/// The sessions of the async tasks this conversation has running *right now*.
|
||||
///
|
||||
/// Deliberately narrower than [`list_for_parent_session`]: that one also
|
||||
/// reports recent failures, because a failure is still worth showing. A task
|
||||
/// that is no longer running cannot be waiting on a human, so including one
|
||||
/// here could only match a stale pending item against the wrong job.
|
||||
///
|
||||
/// `running_session_id` is written before the task's handler is built (see
|
||||
/// `cron::run_job`), so a task can never raise an approval before this query
|
||||
/// can attribute it.
|
||||
pub async fn running_child_sessions(
|
||||
pool: &SqlitePool,
|
||||
parent_session_id: i64,
|
||||
) -> Result<Vec<RunningChildSession>> {
|
||||
let rows = sqlx::query_as::<_, RunningChildSession>(
|
||||
"SELECT id AS job_id,
|
||||
title AS title,
|
||||
running_session_id AS session_id
|
||||
FROM scheduled_jobs
|
||||
WHERE kind = 'async'
|
||||
AND parent_session_id = ?
|
||||
AND running_session_id IS NOT NULL
|
||||
ORDER BY id",
|
||||
)
|
||||
.bind(parent_session_id)
|
||||
.fetch_all(pool)
|
||||
.await?;
|
||||
Ok(rows)
|
||||
}
|
||||
|
||||
/// The two timestamp shapes this table mixes, as one RFC 3339 string:
|
||||
/// `running_since` is written by SQLite's `datetime('now')` (`Y-m-d H:M:S`,
|
||||
/// UTC, no offset) while `job_runs.started_at` is already RFC 3339. A client
|
||||
@@ -357,6 +399,27 @@ mod tests {
|
||||
assert_eq!(tasks[1].error.as_deref(), Some("boom"));
|
||||
}
|
||||
|
||||
/// Attributing a pending approval to a task means matching its session, so
|
||||
/// this query has to be narrower than the strip's: only what is running, and
|
||||
/// only for this conversation. A finished task cannot be waiting on a human,
|
||||
/// so including one could only pair a stale item with the wrong job.
|
||||
#[tokio::test]
|
||||
async fn only_this_conversations_live_task_sessions_are_attributable() {
|
||||
let pool = seeded().await;
|
||||
let children = running_child_sessions(&pool, 1).await.unwrap();
|
||||
|
||||
let seen: Vec<_> = children.iter().map(|c| (c.job_id, c.session_id)).collect();
|
||||
// Job 1 only: 2/3/4 have ended (no `running_session_id`), 5 belongs to
|
||||
// the other conversation, and 6 is a cron job — nobody's chat.
|
||||
assert_eq!(seen, vec![(1, 11)]);
|
||||
assert_eq!(children[0].title, "still going");
|
||||
|
||||
// A conversation whose tasks are all someone else's gets nothing, and a
|
||||
// conversation that never started one gets nothing — not an error.
|
||||
assert_eq!(running_child_sessions(&pool, 2).await.unwrap().len(), 1);
|
||||
assert!(running_child_sessions(&pool, 999).await.unwrap().is_empty());
|
||||
}
|
||||
|
||||
/// `running_since` is SQLite-shaped and `job_runs.started_at` is RFC 3339;
|
||||
/// both leave here as RFC 3339, or a browser reads one of them in the wrong
|
||||
/// timezone and shows an elapsed counter hours off.
|
||||
|
||||
Reference in New Issue
Block a user