From 84f535539f03e7bc2e59355461824de0ab430577 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Sun, 15 Nov 2026 12:27:04 +0800 Subject: [PATCH] fix: a disabled repository no longer names a next run (#723) The helper answers `enabled` before a saved schedule row, so the page, the dashboard and the digest all stop naming a run the worker will never do (listAutoRepos is enabled=1 AND auto_iterate=1, and a manual start refuses REPO_DISABLED). --- changelog.d/723-disabled-next-run.fixed.md | 1 + docs/architecture.md | 11 +- ...17-one-next-run-decision-three-surfaces.md | 12 ++ src/app.ts | 10 +- src/lib/nextRunFor.ts | 24 ++-- src/views/pages.ts | 5 +- test/jobs.test.ts | 6 +- test/nextScheduledRun.test.ts | 110 +++++++++++++++++- 8 files changed, 161 insertions(+), 18 deletions(-) create mode 100644 changelog.d/723-disabled-next-run.fixed.md diff --git a/changelog.d/723-disabled-next-run.fixed.md b/changelog.d/723-disabled-next-run.fixed.md new file mode 100644 index 00000000..b1acf9fe --- /dev/null +++ b/changelog.d/723-disabled-next-run.fixed.md @@ -0,0 +1 @@ +A paused repository no longer promises a next run: the repository page says it is disabled instead of printing a schedule row's planner instant, and the dashboard never names a paused repository as the next automatic run while an enabled one has one, or prints an instant when every repository with auto-iteration is paused. The daily digest gives the same answer: a paused repository says it is disabled instead of printing its row's instant. diff --git a/docs/architecture.md b/docs/architecture.md index d899f46b..8b8b300d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -171,9 +171,16 @@ defect. calls `nextRunFor` (`src/lib/nextRunFor.ts`), which makes the same row-vs-interval choice and delegates the instant to `nextTime`/`dueAfterMs`, so the email prints the same state and time as the page. The digest owns only its Chinese wording; it - does not import `src/views/**`. The dashboard route passes open work, the + does not import `src/views/**`. A disabled repository is answered by `enabled` + before either rule: the worker only considers `enabled = 1 AND + auto_iterate = 1` (`src/db.ts`, `listAutoRepos`) and a manual start on a paused + repository is refused with `REPO_DISABLED`, so the page route withholds a saved + row's answer and `nextRunFor` answers `disabled` before the row — no surface + names a run for a paused repository. The page keeps the `disabled` sentence with + or without a schedule row, and the dashboard never names a paused repository + while an enabled one has a next run. The dashboard route passes open work, the active job and instance readiness per repository because those live outside - `RepoRow`; the two surfaces therefore print one string, and + `RepoRow`; the surfaces therefore print one string, and the wording says "next run", never an estimate. The newest job is read on its own, not from the job-table slice: the table is paged and filtered, and `?jobsPage=2` or `?status=` must not change the sentence the page opens with. The first-run diff --git a/docs/decisions/0017-one-next-run-decision-three-surfaces.md b/docs/decisions/0017-one-next-run-decision-three-surfaces.md index 2ba94e91..18d14981 100644 --- a/docs/decisions/0017-one-next-run-decision-three-surfaces.md +++ b/docs/decisions/0017-one-next-run-decision-three-surfaces.md @@ -64,3 +64,15 @@ dashboard use, so a deployment with no FlyBot image never prints "not ready". planner instant for a scheduled repository — including a disabled repository that has a saved row, where the row decides the cadence on both surfaces. - `src/jobs/` remains a caller of the rule, never its owner. + +## Amendment — #723 (2026-11-15) + +`enabled` answers before the saved row does, here and on every surface. The +worker only considers `enabled = 1 AND auto_iterate = 1` (`src/db.ts`, +`listAutoRepos`) and a manual start on a paused repository is refused with +`REPO_DISABLED` (`src/jobs/service.ts`), so a paused repository must not be +promised a run even when it keeps its schedule row. The Consequences sentence +that let a disabled repository with a saved row follow the row's cadence is +therefore superseded: the page route withholds the row's answer for a paused +repository and `nextRunFor` returns `disabled` before it reads the row, so the +page, the dashboard and the digest keep one sentence. diff --git a/src/app.ts b/src/app.ts index 79699963..c5cc4b0f 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1277,7 +1277,10 @@ export async function buildApp(options: BuildOptions): Promise { openWork: store.listOpenWorkItems(repo.id).length > 0, activeJob: store.activeJobForRepo(repo.id) !== undefined, instanceReady: store.getRepoInstance(repo.id)?.status === 'ready', - ...(scheduleRow === undefined ? {} : { + // #723: the worker only considers an enabled repository (`listAutoRepos` + // is `WHERE enabled = 1 AND auto_iterate = 1`), so a paused repository + // must not travel with a planner answer the card would treat as runnable. + ...(scheduleRow === undefined || repo.enabled !== 1 ? {} : { scheduledNext: schedule === null ? { kind: 'unreadable' as const, at: null } : { kind: schedule.kind, at: nextTime(schedule, Date.now()) }, @@ -1909,7 +1912,10 @@ export async function buildApp(options: BuildOptions): Promise { reviewTrend, // #544: a repository with a schedule row is answered by the planner, not // by the legacy interval rule the page computed for every repository. - ...(scheduleRow === undefined ? {} : { + // #723: `enabled` answers before the row does, so a paused repository + // keeps the `disabled` sentence the page already has instead of a + // planner instant the worker will never run. + ...(scheduleRow === undefined || repo.enabled !== 1 ? {} : { scheduledNext: schedule === null ? { kind: 'unreadable' as const, at: null } : { kind: schedule.kind, at: nextTime(schedule, Date.now()) }, }), workflowFailures: store.listWorkflowFailuresForRepo(repo.id, 10), diff --git a/src/lib/nextRunFor.ts b/src/lib/nextRunFor.ts index 6d15f3f0..cdd7ee7b 100644 --- a/src/lib/nextRunFor.ts +++ b/src/lib/nextRunFor.ts @@ -39,14 +39,7 @@ export interface NextRunFor { export function nextRunFor(input: NextRunForInput): NextRunFor { const now = input.now ?? Date.now() - if (input.row !== undefined) { - const schedule = scheduleFromRow(input.row) - if (schedule === null) return { at: null, state: 'unknown-unreadable' } - const at = nextTime(schedule, now) - if (at === null) return { at: null, state: 'unknown-closed' } - return { at, state: at <= now ? 'due' : 'scheduled' } - } - return nextRun({ + const intervalAnswer = (): NextRunFor => nextRun({ lastRunAt: input.repo.last_run_at, intervalMinutes: input.repo.interval_minutes, workCooldownMinutes: input.workCooldownMinutes, @@ -57,4 +50,19 @@ export function nextRunFor(input: NextRunForInput): NextRunFor { instanceReady: input.instanceReady, now, }) + // #723: `enabled` answers before the row does, exactly as the worker does. + // `listAutoRepos()` is `WHERE enabled = 1 AND auto_iterate = 1` and a manual + // start on a paused repository is refused with `REPO_DISABLED`, so a saved + // schedule row must not buy a paused repository a next run on any surface. + // The interval answer already carries the `disabled` state, so the page, + // dashboard and digest keep one wording. + if (input.repo.enabled !== 1) return intervalAnswer() + if (input.row !== undefined) { + const schedule = scheduleFromRow(input.row) + if (schedule === null) return { at: null, state: 'unknown-unreadable' } + const at = nextTime(schedule, now) + if (at === null) return { at: null, state: 'unknown-closed' } + return { at, state: at <= now ? 'due' : 'scheduled' } + } + return intervalAnswer() } diff --git a/src/views/pages.ts b/src/views/pages.ts index 90eccf69..8edc065c 100644 --- a/src/views/pages.ts +++ b/src/views/pages.ts @@ -780,7 +780,10 @@ function nextScheduledRun(input: { repos: RepoRow[]; facts: NextRunFact[]; confi repo, line: plannedNextLine(scheduled, t), at: scheduled.at, - runnable: scheduled.kind !== 'unreadable' && scheduled.at !== null, + // #723: a paused repository keeps its row but is never a candidate; the + // route already withholds the row's answer for it, so this is the same + // condition stated where the choice is made. + runnable: repo.enabled === 1 && scheduled.kind !== 'unreadable' && scheduled.at !== null, } } const state = nextRun({ diff --git a/test/jobs.test.ts b/test/jobs.test.ts index ad2fb116..694c3463 100644 --- a/test/jobs.test.ts +++ b/test/jobs.test.ts @@ -488,7 +488,7 @@ test('the digest says a disabled repository will not run when it has no schedule } }) -test('the digest keeps the planner instant for a disabled repository with a schedule row', async () => { +test('the digest says a disabled repository will not run even with a schedule row', async () => { const config = testConfig({ digest: { hour: 0, timeZone: 'Asia/Shanghai' } }) const store = new Store(':memory:') const mailer = new MemoryMailer() @@ -505,8 +505,8 @@ test('the digest keeps the planner instant for a disabled repository with a sche await jobs.tick() const digest = mailer.messages.find((message) => /今日摘要/.test(message.subject)) assert.ok(digest, 'the digest was sent') - assert.match(digest.text, new RegExp(`下次自动迭代：o/disabled-scheduled · ${escapeRegExp(expected)}`)) - assert.doesNotMatch(digest.text, /仓库已停用，自动运行不会开始/, 'a saved row answers before enabled, exactly as on the page') + assert.match(digest.text, /下次自动迭代：o\/disabled-scheduled · 仓库已停用，自动运行不会开始/, 'enabled answers before the saved row') + assert.doesNotMatch(digest.text, new RegExp(escapeRegExp(expected)), 'a paused repository is not promised a run') } finally { store.close() } diff --git a/test/nextScheduledRun.test.ts b/test/nextScheduledRun.test.ts index 31656207..e0de3f05 100644 --- a/test/nextScheduledRun.test.ts +++ b/test/nextScheduledRun.test.ts @@ -126,8 +126,9 @@ test('the shared next-run helper chooses the planner for a saved row', () => { assert.equal(next.at, expected) store.updateRepo(repo.id, { enabled: 0 }) const disabled = nextRunFor({ repo: store.getRepo(repo.id)!, row, openWork: true, activeJob: false, instanceReady: true, workCooldownMinutes: COOLDOWN_MINUTES, now: NOW }) - assert.equal(disabled.state, 'scheduled', 'a saved row answers before enabled, exactly as on the page') - assert.equal(disabled.at, expected) + assert.equal(disabled.state, 'disabled', 'enabled answers before the saved row, exactly as the worker does') + assert.equal(disabled.at, null) + store.updateRepo(repo.id, { enabled: 1 }) const unreadable = store.saveSchedule({ id: 'sch_for_bad', repoId: repo.id, kind: 'cron', expression: null, timezone: 'Asia/Shanghai' }) assert.deepEqual( @@ -298,3 +299,108 @@ test('the dashboard answers with the schedule row, not the interval, when one ex store.close() } }) + +// #723: the worker only ever considers an enabled repository (`listAutoRepos` is +// `WHERE enabled = 1 AND auto_iterate = 1`) and a manual start on a paused one is +// refused with REPO_DISABLED, so no surface may name a run for it. These cases +// cover the two surfaces the card measured. +function seedAllClear(store: Store, user: ReturnType, repoId: string): void { + const at = new Date().toISOString() + store.saveRepoInstance({ + repo_id: repoId, user_id: user.id, container: 'c', network: 'n', data_dir: '/tmp/disabled', bridge_script: '/tmp/bridge', + dsh_port: 1, bridge_port: 2, model_port: 3, model_bind: '127.0.0.1', image: 'image', status: 'ready', + chief_session: 'c', team_json: null, bridge_token_enc: null, bridge_pid: null, log_pid: null, + last_error: null, created_at: at, updated_at: at, last_health_at: null, + }) + store.createModelProvider({ id: `prov_${repoId}`, userId: user.id, label: 'p', baseUrl: 'https://example.test', keyEnc: 'x', allowPrivate: false }) + store.createProviderModel({ id: `pm_${repoId}`, providerId: `prov_${repoId}`, model: 'm', label: 'm' }) + // #732: a provider row is not an own model until a repository selects it, and + // this deployment configures no platform model, so the selection is what makes + // the first-run check complete. + store.setRepoModelSelection(repoId, [`pm_${repoId}`], `pm_${repoId}`) + store.createJob({ id: `job_${repoId}`, repoId, userId: user.id, trigger: 'manual', goal: 'g', callbackHash: `cb_${repoId}`, platformTokenHash: '' }) + store.updateJob(`job_${repoId}`, { status: 'succeeded' }) +} + +function nextRunRepo(store: Store, user: ReturnType, id: string, name: string) { + return store.createRepo({ + id, userId: user.id, owner: 'acme', name, defaultBranch: 'main', tokenEnc: 'x', permission: 'admin', + repoRole: 'maintainer', goal: '', autoIterate: true, intervalMinutes: INTERVAL_MINUTES, + }) +} + +test('a disabled repository with a schedule row says so on the page, not a planner instant', async () => { + const config = testConfig() + const store = new Store(':memory:') + const built = await buildApp({ config, store, mailer: new MemoryMailer(), runner: new MockRunner(), startScheduler: false }) + try { + const user = makeUser(store, { login: 'next-run-disabled-page', status: 'approved' }) + const session = sessionFor(built, user) + const repo = nextRunRepo(store, user, 'repo_disabled_row', 'paused') + store.updateRepo(repo.id, { last_run_at: LAST_RUN_RECENT }) + store.createWorkItem({ id: 'wi_disabled_row', repoId: repo.id, userId: user.id, title: 'open work', detail: '' }) + store.saveSchedule({ id: 'sch_disabled_row', repoId: repo.id, kind: 'cron', expression: '0 9 * * 1-5', timezone: 'Asia/Shanghai' }) + store.updateRepo(repo.id, { enabled: 0 }) + + const page = await built.app.inject({ method: 'GET', url: `/repos/${repo.id}`, headers: { cookie: session.cookie } }) + assert.equal(page.statusCode, 200) + assert.equal(repoLine(page.body), '仓库已停用，自动运行不会开始', 'a paused repository is not given a next run') + const source = /data-next-source="([^"]*)"/.exec(page.body)?.[1] + assert.equal(source, 'interval', 'the line is not tagged as the planner answering for a paused repository') + } finally { + await built.app.close() + store.close() + } +}) + +test('the dashboard never names a paused repository while an enabled one has a next run', async () => { + const config = testConfig() + const store = new Store(':memory:') + const built = await buildApp({ config, store, mailer: new MemoryMailer(), runner: new MockRunner(), startScheduler: false }) + try { + const user = makeUser(store, { login: 'next-run-disabled-dash', status: 'approved' }) + const session = sessionFor(built, user) + const paused = nextRunRepo(store, user, 'repo_dash_paused', 'paused') + const enabled = nextRunRepo(store, user, 'repo_dash_enabled', 'enabled') + seedAllClear(store, user, enabled.id) + // The paused repository's cron is earlier, so before the fix it is both + // runnable and the earliest candidate, and the card names it. + store.saveSchedule({ id: 'sch_dash_paused', repoId: paused.id, kind: 'cron', expression: '0 * * * *', timezone: 'Asia/Shanghai' }) + store.saveSchedule({ id: 'sch_dash_enabled', repoId: enabled.id, kind: 'cron', expression: '0 9 * * 1-5', timezone: 'Asia/Shanghai' }) + store.updateRepo(paused.id, { enabled: 0 }) + + const dashboard = await built.app.inject({ method: 'GET', url: '/dashboard', headers: { cookie: session.cookie } }) + assert.equal(dashboard.statusCode, 200) + const section = /
/.exec(dashboard.body)?.[0] ?? '' + assert.match(section, /data-next-action="clear"/, 'the fixture reaches the all-clear card, where the next run is named') + assert.match(section, /下次自动运行：acme\/enabled/, 'the enabled repository is the next automatic run') + assert.doesNotMatch(section, /acme\/paused/, 'a paused repository is never named as the next automatic run') + } finally { + await built.app.close() + store.close() + } +}) + +test('with only paused repositories the dashboard says runs are paused and names no instant', async () => { + const config = testConfig() + const store = new Store(':memory:') + const built = await buildApp({ config, store, mailer: new MemoryMailer(), runner: new MockRunner(), startScheduler: false }) + try { + const user = makeUser(store, { login: 'next-run-disabled-only', status: 'approved' }) + const session = sessionFor(built, user) + const paused = nextRunRepo(store, user, 'repo_dash_only_paused', 'paused') + seedAllClear(store, user, paused.id) + store.saveSchedule({ id: 'sch_dash_only_paused', repoId: paused.id, kind: 'cron', expression: '0 9 * * 1-5', timezone: 'Asia/Shanghai' }) + store.updateRepo(paused.id, { enabled: 0 }) + + const dashboard = await built.app.inject({ method: 'GET', url: '/dashboard', headers: { cookie: session.cookie } }) + assert.equal(dashboard.statusCode, 200) + const section = /
/.exec(dashboard.body)?.[0] ?? '' + assert.match(section, /data-next-action="clear"/, 'the fixture reaches the all-clear card') + assert.match(section, /自动运行暂停：acme\/paused · 仓库已停用，自动运行不会开始/, 'the card says why nothing runs') + assert.doesNotMatch(section, /\d{2}:\d{2}/, 'no instant is printed for a paused repository') + } finally { + await built.app.close() + store.close() + } +})