fix: barrier before the CLI count, forward the observed job, surface query errors

P1 — the CLI's active-run count was a bare time-of-check/time-of-use read I added
an hour ago: a fire could be claimed and accepted between the zero result and the
deletes, and this script can neither abort nor drain it. The durable deletion
barrier is now raised BEFORE the count, so a live server refuses new fires at the
dispatch boundary from that point and anything the count misses cannot have
started after it.

expireApproval read the job and then called expireWithIdentity without forwarding
it, so the expiry re-read the store. Not merely a wasted round trip: the preserve
decision keys on scheduleId, so a second read returning null or a replacement
would drop a scheduled run's retained evidence — the opposite of what the first
read established. I added that parameter and missed its only caller.

The schedules panel rendered a failed query as the empty state, telling users
their schedules were gone when the request had merely failed, and left the create
button enabled against an unknown limit. It now shows an error with a retry.

NOT fixed, deliberately: nextRunAt is still computed from the process clock while
claiming and misfire detection compare against MongoDB's $$NOW, so a skewed host
can skip or duplicate an occurrence near a cadence boundary. That is a pre-existing
design seam rather than a regression, and moving occurrence math onto the database
clock is a larger change than this series should absorb.
This commit is contained in:
Danny Avila 2026-07-27 12:46:39 -04:00
parent 6299358633
commit 7dee15de7c
4 changed files with 35 additions and 1 deletions

View file

@ -9,7 +9,7 @@ import ScheduleCard from './ScheduleCard';
export default function SchedulePanel() {
const localize = useLocalize();
const { data, isLoading } = useSchedulesQuery();
const { data, isLoading, isError, refetch } = useSchedulesQuery();
const [createOpen, setCreateOpen] = useState(false);
const hasCreateAccess = useHasAccess({
@ -25,6 +25,23 @@ export default function SchedulePanel() {
);
}
// A failed query leaves `data` undefined, which the empty-state branch below would
// render as "no scheduled chats yet" — telling the user their schedules are gone when
// the request merely failed. `maxPerUser` is unknown too, so the create button would
// stay enabled and any create would 4xx against a limit we cannot see.
if (isError) {
return (
<div className="flex h-full w-full flex-col items-center justify-center gap-2 p-4">
<p className="text-center text-sm text-text-secondary">
{localize('com_ui_schedules_error')}
</p>
<Button variant="outline" size="sm" onClick={() => refetch()}>
{localize('com_ui_retry')}
</Button>
</div>
);
}
const schedules = data?.schedules ?? [];
const maxPerUser = data?.limits.maxPerUser;
const atLimit = maxPerUser !== undefined && schedules.length >= maxPerUser;

View file

@ -1707,6 +1707,7 @@
"com_ui_schedule_weekdays": "Weekdays",
"com_ui_schedule_weekly": "Weekly",
"com_ui_schedules": "Scheduled chats",
"com_ui_schedules_error": "Couldn't load your scheduled chats",
"com_ui_schedules_empty": "No scheduled chats yet",
"com_ui_schedules_used": "{{used}} of {{max}} schedules used",
"com_ui_schema": "Schema",

View file

@ -102,6 +102,17 @@ async function gracefulExit(code = 0) {
AclEntry.deleteMany({ principalId: user._id }),
];
// Raise the durable deletion barrier BEFORE counting. A bare count is a
// time-of-check/time-of-use read: a fire can be claimed and accepted between the zero
// result and the deletes below, and this script cannot abort or drain it. The barrier
// is what makes the count meaningful — a live server refuses new fires at the dispatch
// boundary (fireSchedule's isOwnerDeleting probe) from this point on, so anything the
// count then misses cannot have started after it.
await User.updateOne(
{ _id: uid, deletionRequestedAt: { $exists: false } },
{ $set: { deletionRequestedAt: new Date() } },
);
// REFUSE rather than warn when a scheduled run is in flight. This script talks to the
// database directly, so unlike the HTTP deletion paths it cannot abort a live loopback
// generation or wait for it to drain. That generation can already have passed its

View file

@ -3198,10 +3198,15 @@ class GenerationJobManagerClass {
} catch (err) {
logger.warn(`[GenerationJobManager] Failed to read approval before expiry ${streamId}`, err);
}
// Forward the job we ALREADY read. Without it the expiry re-reads the store, which
// is not just a wasted round trip: the preserve decision keys on `scheduleId`, so a
// second read that returns null (or a replacement) would drop a scheduled run's
// retained evidence — the opposite of what the first read established.
const expiredCreatedAt = await this._approvals.expireWithIdentity(
streamId,
actionId,
observedJob?.createdAt,
observedJob,
);
if (expiredCreatedAt == null) {
return false;