This commit is contained in:
@@ -607,7 +607,11 @@ export async function runPiece(
|
||||
const cfg = loadConfig();
|
||||
flushAndStageRecording(
|
||||
options.taskId,
|
||||
options.userId,
|
||||
// No-auth mode passes userId=null; fall back to the same 'local'
|
||||
// namespace as the ToolContext below, otherwise
|
||||
// flushAndStageRecording's `if (!ownerId) return` silently discards
|
||||
// every browser recording and self-healing patch in no-auth.
|
||||
options.userId ?? 'local',
|
||||
cfg.userFolderRoot ?? './data/users',
|
||||
);
|
||||
}
|
||||
|
||||
@@ -262,6 +262,81 @@ describe('Scheduler.executeScheduledTask: ownership inheritance', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Scheduler.tick: skip-on-in-progress', () => {
|
||||
// Regression: a scheduled task's previous job ending in `waiting_human` (ASK)
|
||||
// used to be treated as "in progress" and block all future scheduled runs
|
||||
// forever — waiting_human has no auto-recovery, and an unattended schedule has
|
||||
// nobody to answer the ASK, so the schedule silently died. Now waiting_human is
|
||||
// excluded from the skip set: each scheduled run is an independent task, so we
|
||||
// leave the stale waiting_human task as-is and start a fresh run.
|
||||
let tempDir: string;
|
||||
let repo: Repository;
|
||||
let scheduler: Scheduler;
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = mkdtempSync(join(tmpdir(), 'agent-sched-skip-'));
|
||||
repo = new Repository(join(tempDir, 'db.sqlite'));
|
||||
scheduler = new Scheduler(repo, join(tempDir, 'workspaces'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
repo.close();
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// Run a first tick to produce a job, force that job into `status`, then re-arm
|
||||
// the schedule (past next_run_at) and return its id + the first job id.
|
||||
async function armWithLastJobStatus(status: string): Promise<{ schedId: number; firstJobId: string }> {
|
||||
const sched = await repo.createScheduledTask({
|
||||
body: 'recurring thing',
|
||||
pieceName: 'chat',
|
||||
profile: 'auto',
|
||||
outputFormat: 'markdown',
|
||||
cronExpression: '0 9 * * *',
|
||||
nextRunAt: '2000-01-01 09:00:00',
|
||||
});
|
||||
const first = await scheduler.tick();
|
||||
expect(first).toBe(1);
|
||||
const after = await repo.getScheduledTask(sched.id);
|
||||
const firstJobId = after!.lastJobId!;
|
||||
await repo.updateJob(firstJobId, { status: status as any });
|
||||
// Re-arm: getScheduledTasksDue advanced next_run_at to the future.
|
||||
await repo.updateScheduledTask(sched.id, { nextRunAt: '2000-01-01 09:00:00' });
|
||||
return { schedId: sched.id, firstJobId };
|
||||
}
|
||||
|
||||
it('starts a NEW run when the previous job is waiting_human (no longer blocks)', async () => {
|
||||
const { schedId, firstJobId } = await armWithLastJobStatus('waiting_human');
|
||||
|
||||
const executed = await scheduler.tick();
|
||||
expect(executed).toBe(1);
|
||||
|
||||
const after = await repo.getScheduledTask(schedId);
|
||||
expect(after?.lastJobId).toBeTruthy();
|
||||
expect(after?.lastJobId).not.toBe(firstJobId); // a fresh job was created
|
||||
});
|
||||
|
||||
it('still skips when the previous job is running (genuinely in progress)', async () => {
|
||||
const { schedId, firstJobId } = await armWithLastJobStatus('running');
|
||||
|
||||
const executed = await scheduler.tick();
|
||||
expect(executed).toBe(0);
|
||||
|
||||
const after = await repo.getScheduledTask(schedId);
|
||||
expect(after?.lastJobId).toBe(firstJobId); // unchanged — skipped
|
||||
});
|
||||
|
||||
it('still skips when the previous job is waiting_subtasks (auto-recovering transient)', async () => {
|
||||
const { schedId, firstJobId } = await armWithLastJobStatus('waiting_subtasks');
|
||||
|
||||
const executed = await scheduler.tick();
|
||||
expect(executed).toBe(0);
|
||||
|
||||
const after = await repo.getScheduledTask(schedId);
|
||||
expect(after?.lastJobId).toBe(firstJobId); // unchanged — skipped
|
||||
});
|
||||
});
|
||||
|
||||
describe('Scheduler.executeScheduledTask: task_kind="script"', () => {
|
||||
// Scheduler runs a user-authored script directly (no LLM agent loop) when
|
||||
// task_kind='script'. The job row gets created in a pre-completed state.
|
||||
@@ -393,6 +468,46 @@ module.exports = main;
|
||||
await expect(scheduler.executeById(sched.id)).rejects.toThrow(/script_name is null/);
|
||||
});
|
||||
|
||||
it('runs as the "local" user when the schedule has no owner (no-auth mode)', async () => {
|
||||
// Regression: no-auth deployments store scheduled tasks with ownerId=null
|
||||
// (scheduled-tasks-api has no authenticated user). executeScriptScheduledTask
|
||||
// used to throw "requires an owner_id", so no-auth scheduled scripts could
|
||||
// never run. Now it falls back to the 'local' namespace (same as the
|
||||
// RunUserScript tool), resolving the script from data/users/local/scripts/.
|
||||
writeScript('local', 'noauth.js', `async function main() {
|
||||
return { ran: 'no-auth-local' };
|
||||
}
|
||||
module.exports = main;
|
||||
`);
|
||||
|
||||
const sched = await repo.createScheduledTask({
|
||||
body: '',
|
||||
cronExpression: '0 9 * * *',
|
||||
nextRunAt: '2099-01-01 09:00:00',
|
||||
// ownerId intentionally omitted → null, as in no-auth mode
|
||||
taskKind: 'script',
|
||||
scriptName: 'noauth',
|
||||
});
|
||||
|
||||
await scheduler.executeById(sched.id);
|
||||
|
||||
const after = await repo.getScheduledTask(sched.id);
|
||||
expect(after?.lastJobId).toBeTruthy();
|
||||
const job = await repo.getJob(after!.lastJobId!);
|
||||
expect(job?.status).toBe('succeeded');
|
||||
|
||||
const task = await repo.getLocalTask(job!.issueNumber);
|
||||
const output = readFileSync(join(task!.workspacePath!, 'output', 'script-output.txt'), 'utf-8');
|
||||
expect(output).toContain('no-auth-local'); // script's return value was saved
|
||||
|
||||
// The audit row records the resolved 'local' owner, not null.
|
||||
const audits = repo.getDb()
|
||||
.prepare("SELECT detail FROM audit_log WHERE job_id = ? AND action = 'user_script_run'")
|
||||
.all(after!.lastJobId!) as Array<{ detail: string }>;
|
||||
expect(audits).toHaveLength(1);
|
||||
expect((JSON.parse(audits[0].detail) as { userId: string }).userId).toBe('local');
|
||||
});
|
||||
|
||||
it('refuses to run when the user-script gate is disabled', async () => {
|
||||
const gatedScheduler = new Scheduler(repo, join(tempDir, 'workspaces'), {
|
||||
userFolderRoot,
|
||||
|
||||
+19
-9
@@ -7,9 +7,16 @@ import { logger } from './logger.js';
|
||||
import { loadConfig } from './config.js';
|
||||
import { resolveAndRunUserScript } from './user-folder/script-orchestrator.js';
|
||||
|
||||
// 進行中とみなすステータス(これらの場合はスキップ)
|
||||
// 進行中とみなすステータス(これらの場合は次回スケジュール実行をスキップ)。
|
||||
// これらはいずれも「真に実行中」か「自動回復する一時状態」のみ:
|
||||
// queued/dispatching/running … 実行中
|
||||
// waiting_subtasks … 並列サブタスク完了待ち。requeueWaitingSubtasks が自動回復させる
|
||||
// waiting_human (ASK) は意図的に含めない。自動回復もタイムアウトも無いため、
|
||||
// 無人実行のスケジュールタスクが一度 ASK を出すとその後の実行が永久にスキップされ、
|
||||
// スケジュールが事実上死ぬ。各スケジュール実行は独立した新規タスクなので、
|
||||
// 前回の waiting_human タスクは残置し(ユーザーが後から回答 or 放置できる)、新規実行を走らせる。
|
||||
const IN_PROGRESS_STATUSES = new Set([
|
||||
'queued', 'dispatching', 'running', 'waiting_human', 'waiting_subtasks',
|
||||
'queued', 'dispatching', 'running', 'waiting_subtasks',
|
||||
]);
|
||||
|
||||
export interface ScheduleInput {
|
||||
@@ -295,9 +302,12 @@ export class Scheduler {
|
||||
if (!this.userFolderRoot) {
|
||||
throw new Error(`scheduled_task=${item.id}: task_kind='script' but Scheduler.userFolderRoot was not configured`);
|
||||
}
|
||||
if (!item.ownerId) {
|
||||
throw new Error(`scheduled_task=${item.id}: task_kind='script' requires an owner_id (scripts are per-user)`);
|
||||
}
|
||||
// No-auth mode stores scheduled tasks with ownerId=null (scheduled-tasks-api
|
||||
// has no authenticated user). Scripts are per-user (data/users/{id}/scripts/),
|
||||
// so resolve to the same 'local' namespace the RunUserScript tool uses in
|
||||
// no-auth (ctx.userId='local'). In auth mode item.ownerId is always set, so
|
||||
// scriptOwner === item.ownerId and behaviour is unchanged.
|
||||
const scriptOwner = item.ownerId ?? 'local';
|
||||
|
||||
// Same security gates as the LLM-facing RunUserScript tool: global config
|
||||
// toggle + optional per-user allowlist. A scheduled run is automated, so a
|
||||
@@ -308,9 +318,9 @@ export class Scheduler {
|
||||
`scheduled_task=${item.id}: user scripts are disabled (tools.user_scripts_enabled=false)`,
|
||||
);
|
||||
}
|
||||
if (Array.isArray(gate.allowUserids) && gate.allowUserids.length > 0 && !gate.allowUserids.includes(item.ownerId)) {
|
||||
if (Array.isArray(gate.allowUserids) && gate.allowUserids.length > 0 && !gate.allowUserids.includes(scriptOwner)) {
|
||||
throw new Error(
|
||||
`scheduled_task=${item.id}: owner "${item.ownerId}" is not in tools.user_scripts_allow_userids`,
|
||||
`scheduled_task=${item.id}: owner "${scriptOwner}" is not in tools.user_scripts_allow_userids`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -375,7 +385,7 @@ export class Scheduler {
|
||||
try {
|
||||
const runResult = await resolveAndRunUserScript({
|
||||
rootDir: this.userFolderRoot,
|
||||
userId: item.ownerId,
|
||||
userId: scriptOwner,
|
||||
name: item.scriptName,
|
||||
params,
|
||||
sessRepo: this.sessRepo,
|
||||
@@ -417,7 +427,7 @@ export class Scheduler {
|
||||
|
||||
await this.repo.addAuditLog(job.id, 'user_script_run', 'scheduler', {
|
||||
scheduledTaskId: item.id,
|
||||
userId: item.ownerId,
|
||||
userId: scriptOwner,
|
||||
scriptName: item.scriptName,
|
||||
ok: !runFailed,
|
||||
...(runFailed && errorMessage ? { error: errorMessage.slice(0, 500) } : {}),
|
||||
|
||||
@@ -67,15 +67,44 @@ describe('maybeEnqueueReflection', () => {
|
||||
expect(await repo.getJobsByStatus('queued')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('does nothing when ownerId is null (no user to learn for)', async () => {
|
||||
it('falls back to the "local" namespace when ownerId is null (no-auth mode)', async () => {
|
||||
// No-auth mode runs every job with ownerId=null. Reflection must still run,
|
||||
// keyed to the same 'local' namespace the rest of the no-auth path uses.
|
||||
const job = await repo.createJob({
|
||||
repo: 'local/task-1', issueNumber: 1,
|
||||
instruction: 'x', pieceName: 'chat',
|
||||
} as any); // no ownerId
|
||||
} as any); // no ownerId → null in no-auth
|
||||
await repo.updateJob(job.id, { status: 'succeeded' });
|
||||
await maybeEnqueueReflection(repo, job, 'succeeded', {
|
||||
enabled: true, workerRequired: false, perUserDailyBudgetTokens: 0,
|
||||
});
|
||||
const queued = await repo.getJobsByStatus('queued');
|
||||
expect(queued).toHaveLength(1);
|
||||
expect(queued[0].taskKind).toBe('reflection');
|
||||
expect(queued[0].ownerId).toBe('local');
|
||||
expect(JSON.parse(queued[0].payload!).userId).toBe('local');
|
||||
});
|
||||
|
||||
it('no-auth budget check is keyed to "local" (not null)', async () => {
|
||||
// A prior null-owner reflection that spent the whole budget must block the
|
||||
// next null-owner enqueue — proving the budget query uses 'local', not null.
|
||||
const job = await repo.createJob({
|
||||
repo: 'local/task-8', issueNumber: 8,
|
||||
instruction: 'x', pieceName: 'chat',
|
||||
} as any); // no ownerId
|
||||
await repo.updateJob(job.id, { status: 'succeeded' });
|
||||
|
||||
const cap = 1_000_000;
|
||||
const todayStartMs = Date.UTC(
|
||||
new Date().getUTCFullYear(),
|
||||
new Date().getUTCMonth(),
|
||||
new Date().getUTCDate(),
|
||||
);
|
||||
seedMetric('local', 600_000, 500_000, todayStartMs + 1_000); // total = 1 100 000 > cap
|
||||
|
||||
await maybeEnqueueReflection(repo, job, 'succeeded', {
|
||||
enabled: true, workerRequired: false, perUserDailyBudgetTokens: cap,
|
||||
});
|
||||
expect(await repo.getJobsByStatus('queued')).toHaveLength(0);
|
||||
});
|
||||
|
||||
|
||||
+12
-10
@@ -271,10 +271,12 @@ export async function maybeEnqueueReflection(
|
||||
): Promise<void> {
|
||||
if (!cfg.enabled) return;
|
||||
if (job.taskKind === 'reflection') return;
|
||||
if (!job.ownerId) {
|
||||
logger.warn(`[reflection] skip enqueue job=${job.id} reason=no_owner`);
|
||||
return;
|
||||
}
|
||||
// No-auth mode runs every job with ownerId=null. Reflection is per-user
|
||||
// (memory/pieces live under data/users/{userId}/), so fall back to the same
|
||||
// 'local' namespace the rest of the no-auth path uses (ToolContext, pieces,
|
||||
// user-folder). Without this the enqueue gate skipped forever and reflection
|
||||
// silently never ran in no-auth deployments.
|
||||
const reflectionOwner = job.ownerId ?? 'local';
|
||||
|
||||
// worker_required enforcement: when true, at least one worker must have 'reflection' in its roles
|
||||
if (cfg.workerRequired) {
|
||||
@@ -282,7 +284,7 @@ export async function maybeEnqueueReflection(
|
||||
(w) => Array.isArray(w.roles) && w.roles.includes('reflection'),
|
||||
);
|
||||
if (!hasReflectionWorker) {
|
||||
logger.warn(`[reflection] enqueue skipped reason=no_reflection_worker user=${job.ownerId}`);
|
||||
logger.warn(`[reflection] enqueue skipped reason=no_reflection_worker user=${reflectionOwner}`);
|
||||
return;
|
||||
}
|
||||
}
|
||||
@@ -294,19 +296,19 @@ export async function maybeEnqueueReflection(
|
||||
// Compute today's start in UTC (00:00:00.000 UTC).
|
||||
const now = new Date();
|
||||
const todayStartMs = Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate());
|
||||
const metrics = repo.aggregateReflectionMetrics(job.ownerId, todayStartMs);
|
||||
const metrics = repo.aggregateReflectionMetrics(reflectionOwner, todayStartMs);
|
||||
const spent = metrics.tokensIn + metrics.tokensOut;
|
||||
if (spent >= cap) {
|
||||
const spentM = (spent / 1_000_000).toFixed(1);
|
||||
const capM = (cap / 1_000_000).toFixed(1);
|
||||
logger.info(`[reflection] enqueue skipped reason=budget user=${job.ownerId} spent=${spentM}M cap=${capM}M`);
|
||||
logger.info(`[reflection] enqueue skipped reason=budget user=${reflectionOwner} spent=${spentM}M cap=${capM}M`);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const payload = JSON.stringify({
|
||||
originalJobId: job.id,
|
||||
userId: job.ownerId,
|
||||
userId: reflectionOwner,
|
||||
pieceName: job.pieceName,
|
||||
outcome,
|
||||
});
|
||||
@@ -316,12 +318,12 @@ export async function maybeEnqueueReflection(
|
||||
instruction: '',
|
||||
pieceName: 'reflection',
|
||||
role: 'reflection',
|
||||
ownerId: job.ownerId,
|
||||
ownerId: reflectionOwner,
|
||||
visibility: 'private',
|
||||
taskKind: 'reflection',
|
||||
payload,
|
||||
} as any);
|
||||
logger.info(`[reflection] enqueued original=${job.id} owner=${job.ownerId} piece=${job.pieceName} outcome=${outcome}`);
|
||||
logger.info(`[reflection] enqueued original=${job.id} owner=${reflectionOwner} piece=${job.pieceName} outcome=${outcome}`);
|
||||
}
|
||||
|
||||
export class Worker {
|
||||
|
||||
Reference in New Issue
Block a user