Skip to content
Merged
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
1 change: 1 addition & 0 deletions changelog.d/2534-inbox-stale-failed-card.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A workflow task that failed and was then run again to a green finish no longer keeps its **Failed** card in the Inbox, or its place in the sidebar's *Needs you* count, for the whole seven-day window: the success retires the card ([#2534](https://github.com/ThinkFlowLab/FlyBot/issues/2534)).
2 changes: 2 additions & 0 deletions core/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -9037,6 +9037,8 @@ function inboxModel({ roster, work = {}, tasks = [], sessions, workflows = [], n
}
if (latest === void 0) continue;
const at = endedAt(latest);
const taskId = latest.task?.id;
if (taskId !== void 0 && [...graph.runs ?? [], ...graph.history ?? []].some((run) => run?.status === "done" && run.task?.id === taskId && endedAt(run) > at)) continue;
const name = typeof graph.name === "string" && graph.name.trim() !== "" ? graph.name.trim() : String(graph.id ?? t("A workflow"));
const nodeName = typeof latest.failure?.nodeName === "string" ? latest.failure.nodeName.trim() : "";
const reason = typeof latest.failure?.text === "string" && latest.failure.text !== "" ? latest.failure.text : typeof latest.failure?.code === "string" ? latest.failure.code : "";
Expand Down
4 changes: 4 additions & 0 deletions core/src/client/inbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ export function inboxModel({ roster, work = {}, tasks = [], sessions, workflows
}
if (latest === undefined) continue
const at = endedAt(latest)
// A task re-run that ended green answers its failure (#P-599): the success is in the same
// per-graph history under the same `task.id`, so no Failed card outlives the work.
const taskId = latest.task?.id
if (taskId !== undefined && [...(graph.runs ?? []), ...(graph.history ?? [])].some(run => run?.status === 'done' && run.task?.id === taskId && endedAt(run) > at)) continue
const name = typeof graph.name === 'string' && graph.name.trim() !== '' ? graph.name.trim() : String(graph.id ?? t('A workflow'))
const nodeName = typeof latest.failure?.nodeName === 'string' ? latest.failure.nodeName.trim() : ''
const reason = typeof latest.failure?.text === 'string' && latest.failure.text !== ''
Expand Down
21 changes: 21 additions & 0 deletions core/test/inbox-workflow-failure.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,24 @@ test('#1706: failed workflow rows share the one Needs you list', async () => {
assert.deepEqual(plain(heads), plain(GROUPS.map(group => group.title)))
assert.equal(all(tree).filter(node => node.type === 'section').length, GROUPS.length)
})

// P-599: a run that failed and was then re-run to a green finish kept its Failed card for the
// whole 7-day window, and the sidebar's Need-you count with it, because the card was chosen
// from the failed runs alone. The success is in the same per-graph history under the same
// `task.id`, and later than the failure.
test('P-599: a later successful run of the same task retires its failed card', () => {
const task = { id: 'issue-2447-run', title: 'issue #2447: a room-share envelope', callerBotId: 'b1' }
const failed = { ...failedRun, id: 'run-failed', task, finishedAt: NOW - 2 * 60 * MIN, updatedAt: NOW - 2 * 60 * MIN }
const done = { ...failedRun, id: 'run-done', task, status: 'done', failure: undefined, finishedAt: NOW - 30 * MIN, updatedAt: NOW - 30 * MIN }
const graph = { id: 'delivery', name: 'Delivery pipeline', runs: [], history: [{ ...failed, at: failed.finishedAt }, { ...done, at: done.finishedAt }] }
const model = inboxModel({ roster, work, tasks: [], workflows: [graph], now: NOW })
const listed = [...model.needs, ...model.progress, ...model.done].filter(card => card.kind === 'workflow')
assert.deepEqual(listed.map(card => card.run.id), [], 'the run that finished green answers the failure')
assert.equal(needsCount(roster, work, [], NOW, [graph]), model.needs.length, 'the badge counts the same cards')
// Control: another task's failure on the same graph still lists, so the graph itself did not
// simply stop reporting failures.
const other = { ...failed, id: 'run-other', task: { ...task, id: 'issue-2400-run' } }
const still = inboxModel({ roster, work, tasks: [], workflows: [{ ...graph, history: [{ ...other, at: other.finishedAt }, { ...done, at: done.finishedAt }] }], now: NOW })
const cardsStill = [...still.needs, ...still.progress, ...still.done].filter(card => card.kind === 'workflow')
assert.deepEqual(cardsStill.map(card => card.run.id), ['run-other'])
})
2 changes: 1 addition & 1 deletion docs/subsystems/core-client.md

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion patches/ds-bot/0008-client-bundle.patch
Original file line number Diff line number Diff line change
Expand Up @@ -6359,7 +6359,7 @@ diff --git a/client.js b/client.js
@keyframes bt-typing-in{from{opacity:0;transform:scale(.9)}}
@keyframes bt-typing-out{from{opacity:1;transform:scale(1)}to{opacity:0;transform:scale(.92);filter:blur(3px)}}
@keyframes bt-typing-label{from{opacity:0;filter:blur(5px);transform:translateY(3px)}}
@@ -2676,2833 +2676,6532 @@ header[class*="_header"] .bt-head{flex:1 1 auto;width:100%}
@@ -2676,2833 +2676,6534 @@ header[class*="_header"] .bt-head{flex:1 1 auto;width:100%}
[data-bt-enter="new"]{animation:none}
[data-bt-enter="after-collapse"]{animation:bt-fade-in .12s ease-out}
}
Expand Down Expand Up @@ -10540,6 +10540,8 @@ diff --git a/client.js b/client.js
+ }
+ if (latest === void 0) continue;
+ const at = endedAt(latest);
+ const taskId = latest.task?.id;
+ if (taskId !== void 0 && [...graph.runs ?? [], ...graph.history ?? []].some((run) => run?.status === "done" && run.task?.id === taskId && endedAt(run) > at)) continue;
+ const name = typeof graph.name === "string" && graph.name.trim() !== "" ? graph.name.trim() : String(graph.id ?? t("A workflow"));
+ const nodeName = typeof latest.failure?.nodeName === "string" ? latest.failure.nodeName.trim() : "";
+ const reason = typeof latest.failure?.text === "string" && latest.failure.text !== "" ? latest.failure.text : typeof latest.failure?.code === "string" ? latest.failure.code : "";
Expand Down
4 changes: 4 additions & 0 deletions patches/ds-bot/files/src/client/inbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ export function inboxModel({ roster, work = {}, tasks = [], sessions, workflows
}
if (latest === undefined) continue
const at = endedAt(latest)
// A task re-run that ended green answers its failure (#P-599): the success is in the same
// per-graph history under the same `task.id`, so no Failed card outlives the work.
const taskId = latest.task?.id
if (taskId !== undefined && [...(graph.runs ?? []), ...(graph.history ?? [])].some(run => run?.status === 'done' && run.task?.id === taskId && endedAt(run) > at)) continue
const name = typeof graph.name === 'string' && graph.name.trim() !== '' ? graph.name.trim() : String(graph.id ?? t('A workflow'))
const nodeName = typeof latest.failure?.nodeName === 'string' ? latest.failure.nodeName.trim() : ''
const reason = typeof latest.failure?.text === 'string' && latest.failure.text !== ''
Expand Down
21 changes: 21 additions & 0 deletions patches/ds-bot/files/test/inbox-workflow-failure.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,24 @@ test('#1706: failed workflow rows share the one Needs you list', async () => {
assert.deepEqual(plain(heads), plain(GROUPS.map(group => group.title)))
assert.equal(all(tree).filter(node => node.type === 'section').length, GROUPS.length)
})

// P-599: a run that failed and was then re-run to a green finish kept its Failed card for the
// whole 7-day window, and the sidebar's Need-you count with it, because the card was chosen
// from the failed runs alone. The success is in the same per-graph history under the same
// `task.id`, and later than the failure.
test('P-599: a later successful run of the same task retires its failed card', () => {
const task = { id: 'issue-2447-run', title: 'issue #2447: a room-share envelope', callerBotId: 'b1' }
const failed = { ...failedRun, id: 'run-failed', task, finishedAt: NOW - 2 * 60 * MIN, updatedAt: NOW - 2 * 60 * MIN }
const done = { ...failedRun, id: 'run-done', task, status: 'done', failure: undefined, finishedAt: NOW - 30 * MIN, updatedAt: NOW - 30 * MIN }
const graph = { id: 'delivery', name: 'Delivery pipeline', runs: [], history: [{ ...failed, at: failed.finishedAt }, { ...done, at: done.finishedAt }] }
const model = inboxModel({ roster, work, tasks: [], workflows: [graph], now: NOW })
const listed = [...model.needs, ...model.progress, ...model.done].filter(card => card.kind === 'workflow')
assert.deepEqual(listed.map(card => card.run.id), [], 'the run that finished green answers the failure')
assert.equal(needsCount(roster, work, [], NOW, [graph]), model.needs.length, 'the badge counts the same cards')
// Control: another task's failure on the same graph still lists, so the graph itself did not
// simply stop reporting failures.
const other = { ...failed, id: 'run-other', task: { ...task, id: 'issue-2400-run' } }
const still = inboxModel({ roster, work, tasks: [], workflows: [{ ...graph, history: [{ ...other, at: other.finishedAt }, { ...done, at: done.finishedAt }] }], now: NOW })
const cardsStill = [...still.needs, ...still.progress, ...still.done].filter(card => card.kind === 'workflow')
assert.deepEqual(cardsStill.map(card => card.run.id), ['run-other'])
})
Loading