From 2044f0a2c4b3a86be9e04687979589320da5403e Mon Sep 17 00:00:00 2001 From: oss-sync Date: Thu, 9 Jul 2026 23:57:26 +0000 Subject: [PATCH] sync: update from private repo (91d8d79c) --- config.yaml.example | 28 ++ src/bridge/a2a/a2a-exec-e2e.test.ts | 18 +- src/bridge/a2a/executor.test.ts | 82 ++--- src/bridge/a2a/executor.ts | 20 +- .../a2a/request-handler-auth-gate.test.ts | 70 ++++ src/bridge/a2a/request-handler.ts | 48 ++- src/bridge/app-harness-api.ts | 3 +- src/bridge/auth.has-local-credential.test.ts | 96 ++++++ src/bridge/auth.ts | 11 + src/bridge/console-ws-api.test.ts | 39 +++ src/bridge/console-ws-api.ts | 35 ++ src/bridge/server-subsystems.test.ts | 6 + src/bridge/space-api.test.ts | 58 ++++ src/bridge/space-api.ts | 24 +- src/bridge/ssh-subsystem.ts | 17 + src/config-manager.extra-body.test.ts | 101 ++++++ src/config-manager.ts | 8 +- src/config-normalize.extra-body.test.ts | 296 ++++++++++++++++ src/config-normalize.ts | 68 ++++ src/config.extra-body.test.ts | 24 ++ src/config.ts | 24 +- src/db/migrate.ts | 21 ++ src/db/repositories/schema.ts | 14 + src/db/repositories/spaces.ts | 69 ++++ src/db/repository.ts | 16 +- src/db/schema.sql | 14 + src/engine/reflection/llm-client.ts | 4 +- src/engine/reflection/reflection-runner.ts | 3 + src/llm/openai-compat.extra-body.test.ts | 119 +++++++ src/llm/openai-compat.ts | 29 ++ src/progress/delegate-runs.test.ts | 32 ++ src/progress/delegate-runs.ts | 10 + src/scripts/migrate-config.test.ts | 18 + src/scripts/migrate-config.ts | 27 +- src/worker-bootstrap.ts | 6 +- src/worker.extra-body.test.ts | 114 +++++++ src/worker.ts | 8 +- ui/e2e/calendar.spec.ts | 11 +- ui/e2e/pieces-editor.spec.ts | 39 ++- ui/e2e/spaces.spec.ts | 194 ++++++----- ui/playwright.config.ts | 26 +- ui/src/App.tsx | 2 + ui/src/api/spaces.ts | 16 + ui/src/components/detail/DetailPanel.tsx | 5 +- .../detail/tabs/DelegateRunsSection.test.tsx | 206 +++++++++-- .../detail/tabs/DelegateRunsSection.tsx | 45 ++- ui/src/components/detail/tabs/OverviewTab.tsx | 5 +- ui/src/components/layout/TopBar.tsx | 40 +-- .../layout/UserAccountMenu.test.tsx | 117 +++++++ ui/src/components/layout/UserAccountMenu.tsx | 103 ++++++ .../ConfigForm.extra-body-validity.test.tsx | 300 ++++++++++++++++ ui/src/components/settings/ConfigForm.tsx | 55 ++- .../LlmWorkersForm.extra-body.test.tsx | 120 +++++++ .../settings/LlmWorkersForm.test.tsx | 6 +- ui/src/components/settings/LlmWorkersForm.tsx | 208 ++++++++++- .../settings/PreferencesForm.test.tsx | 65 ++++ .../components/settings/PreferencesForm.tsx | 36 +- ui/src/components/settings/formUtils.tsx | 5 +- ui/src/components/settings/types.ts | 27 ++ .../components/spaces/CrossSpaceCalendar.tsx | 23 +- ui/src/components/spaces/SpaceCalendar.tsx | 15 +- ui/src/components/spaces/SpaceDetail.tsx | 7 + ui/src/components/spaces/SpaceRail.test.tsx | 64 +++- ui/src/components/spaces/SpaceRail.tsx | 322 ++++++++++++++---- ui/src/content/help/00-changelog.md | 28 ++ ui/src/content/help/01-intro.md | 2 +- ui/src/content/help/17-settings.md | 10 +- ui/src/content/help/21-workspaces.md | 4 + ui/src/content/help/22-calendar.md | 4 +- ui/src/hooks/useSpaces.ts | 15 + ui/src/i18n/locales/en/detail.json | 1 + ui/src/i18n/locales/en/layout.json | 3 + ui/src/i18n/locales/en/settings.json | 17 +- ui/src/i18n/locales/en/spaces.json | 28 +- ui/src/i18n/locales/ja/detail.json | 1 + ui/src/i18n/locales/ja/layout.json | 3 + ui/src/i18n/locales/ja/settings.json | 17 +- ui/src/i18n/locales/ja/spaces.json | 28 +- ui/src/lib/delegateRuns.ts | 1 + ui/src/pages/PiecesPage.tsx | 6 +- vitest.config.ts | 18 + 81 files changed, 3456 insertions(+), 372 deletions(-) create mode 100644 src/bridge/auth.has-local-credential.test.ts create mode 100644 src/config-manager.extra-body.test.ts create mode 100644 src/config-normalize.extra-body.test.ts create mode 100644 src/config.extra-body.test.ts create mode 100644 src/llm/openai-compat.extra-body.test.ts create mode 100644 src/worker.extra-body.test.ts create mode 100644 ui/src/components/layout/UserAccountMenu.test.tsx create mode 100644 ui/src/components/layout/UserAccountMenu.tsx create mode 100644 ui/src/components/settings/ConfigForm.extra-body-validity.test.tsx create mode 100644 ui/src/components/settings/LlmWorkersForm.extra-body.test.tsx create mode 100644 ui/src/components/settings/PreferencesForm.test.tsx diff --git a/config.yaml.example b/config.yaml.example index 8cf7f29..4b1243c 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -90,6 +90,34 @@ llm: # max_concurrency: 1 # enabled: true + # 例: extra_body / reasoning_efforts / reasoning_effort_mode + # (vLLM で reasoning effort を渡す)。 + # + # extra_body: OpenAI 互換リクエストボディへそのまま浅くマージされる任意 JSON。 + # model / messages / stream / stream_options / tools / tool_choice / + # temperature は予約キーとしてクライアント側で無視される。 + # ここに API キー等の秘密情報を書かないこと — /api/config はこの値を + # マスクしない (config.yaml に平文で残る)。 + # reasoning_efforts: このワーカーが対応する effort の宣言リスト (後続フェーズで + # ジョブ単位の effort 指定に使われる)。 + # reasoning_effort_mode: effort をリクエストボディへ注入する形。 + # body (既定) = トップレベル reasoning_effort。vLLM 向け。 + # chat_template_kwargs = chat_template_kwargs.reasoning_effort。 + # llama-server (llama.cpp) 向け — llama-server はトップレベルの + # reasoning_effort を黙って無視するため、llama-server を使う場合は + # chat_template_kwargs を明示すること。 + # - id: vllm-reasoning + # connection_type: direct + # endpoint: http://localhost:8000/v1 + # model: deepseek-r1 + # roles: [quality] + # max_concurrency: 1 + # enabled: true + # extra_body: + # reasoning_effort: max + # reasoning_efforts: [low, medium, high, max] + # reasoning_effort_mode: body + # Prometheus exporter (worker side). default で enabled。 # /metrics が bridge HTTP server (PORT, default 9876) に mount される。 # access control: default では localhost (127.0.0.1 / ::1) のみ通る。 diff --git a/src/bridge/a2a/a2a-exec-e2e.test.ts b/src/bridge/a2a/a2a-exec-e2e.test.ts index 4b22371..85b2660 100644 --- a/src/bridge/a2a/a2a-exec-e2e.test.ts +++ b/src/bridge/a2a/a2a-exec-e2e.test.ts @@ -262,19 +262,23 @@ describe('A2A exec e2e (real token → message/send → job → artifact + negat expect(taskRow.piece_name).toBe('research'); }, 30_000); - it('no bearer → terminal failed Task, no job created', async () => { + it('no bearer → JSON-RPC error at ingress gate, no job and no a2a_task persisted', async () => { const before = jobCountForUser(); + const a2aBefore = ((repo as any).db.prepare('SELECT COUNT(*) AS n FROM a2a_tasks').get() as { n: number }).n; const res = await sendMessage(messageSendEnvelope('do research')); expect(res.status).toBe(200); - const json = await res.json() as { jsonrpc: string; error?: unknown; result?: { kind?: string; status?: { state: string } } }; + const json = await res.json() as { jsonrpc: string; error?: { code: number }; result?: unknown }; expect(json.jsonrpc).toBe('2.0'); - // 未認証 → executor が principal 無しで完全な terminal Task(state=failed) を publish する。 - // （2C-3 Task 4 で SDK ResultManager 互換に修正: 旧挙動は bare status-update が drop され error 化していた） - expect(json.error).toBeUndefined(); - expect(json.result?.kind).toBe('task'); - expect(json.result?.status?.state).toBe('failed'); + // 2C-3 review #2: 未認証は AuthGatedRequestHandler.enforceIngress() が execute() 前に弾く。 + // クリーンな JSON-RPC エラー(-32600)を返し、タスク行も監査行も一切書かない（書き込み増幅の遮断）。 + // 旧挙動（executor が failed Task を publish → 永続化）から意図的に変更。 + expect(json.error?.code).toBe(-32600); + expect(json.result).toBeUndefined(); // ジョブは作られない（最重要のセキュリティ不変条件）。 expect(jobCountForUser()).toBe(before); + // a2a_tasks 行も増えない（#2 の核心 — フラッドで行が増えないことを保証）。 + const a2aAfter = ((repo as any).db.prepare('SELECT COUNT(*) AS n FROM a2a_tasks').get() as { n: number }).n; + expect(a2aAfter).toBe(a2aBefore); }, 30_000); it('out-of-scope skill (summarize, allowlisted but not consented) → fail-closed, no job created', async () => { diff --git a/src/bridge/a2a/executor.test.ts b/src/bridge/a2a/executor.test.ts index 05f4fde..73263e3 100644 --- a/src/bridge/a2a/executor.test.ts +++ b/src/bridge/a2a/executor.test.ts @@ -542,54 +542,11 @@ describe('MaestroA2aExecutor', () => { const NOW_ISO = '2026-07-07T00:00:00.000Z'; const nowFn = () => NOW_ISO; - it('payload guard: oversized message → rejected Task (kind:task), createLocalTask not called', async () => { - const repo = makeFakeRepo(); - const eventBus = makeFakeEventBus(); - // maxPayloadBytes: 1 なので userMessage を JSON 化した時点で確実にオーバー - const limiter = new A2aLimiter({ ...DEFAULT_A2A_LIMITS, maxPayloadBytes: 1 }, () => 0); - const executor = new MaestroA2aExecutor(repo as any, { - pollMs: 0, - sleep: async () => {}, - now: nowFn, - limiter, - }); - const ctx = makeRequestContext({ a2aPrincipal: DEFAULT_PRINCIPAL, messageText: 'hello' }); - - await executor.execute(ctx as any, eventBus as any); - - expect(eventBus.finishedCalled).toBe(true); - // kind:'task' (pre-initial-Task path) with state='rejected' - const rejected = eventBus.published.find( - (e: any) => e.kind === 'task' && e.status?.state === 'rejected', - ); - expect(rejected).toBeDefined(); - // No job row created - expect(repo.createLocalTask).not.toHaveBeenCalled(); - }); - - it('rate guard: rate bucket drained → rejected Task (kind:task), createLocalTask not called', async () => { - const repo = makeFakeRepo(); - const eventBus = makeFakeEventBus(); - const limiter = new A2aLimiter({ ...DEFAULT_A2A_LIMITS, ratePerMinute: 1 }, () => 0); - // Pre-drain: consume the 1 available token so the next call is rejected - limiter.tryConsumeRate('grant-1'); - const executor = new MaestroA2aExecutor(repo as any, { - pollMs: 0, - sleep: async () => {}, - now: nowFn, - limiter, - }); - const ctx = makeRequestContext({ a2aPrincipal: DEFAULT_PRINCIPAL }); - - await executor.execute(ctx as any, eventBus as any); - - expect(eventBus.finishedCalled).toBe(true); - const rejected = eventBus.published.find( - (e: any) => e.kind === 'task' && e.status?.state === 'rejected', - ); - expect(rejected).toBeDefined(); - expect(repo.createLocalTask).not.toHaveBeenCalled(); - }); + // NOTE: payload + rate guards moved OUT of execute() into + // AuthGatedRequestHandler.enforceIngress() (2C-3 review #2 — write amplification). + // They now throw a JSON-RPC error BEFORE execute() runs, so nothing is persisted; + // their coverage lives in request-handler-auth-gate.test.ts. Only concurrency + + // skill-budget remain here — they need the reservation lifecycle inside execute(). it('concurrency guard: db count at cap → rejected Task (kind:task), createLocalTask not called', async () => { const repo = makeFakeRepo(); @@ -663,6 +620,35 @@ describe('MaestroA2aExecutor', () => { expect(submitted).toBeDefined(); }); + it('executor is UNGUARDED for payload/rate — enforcement lives solely in the ingress gate (review #9)', async () => { + // Explicit invariant marker: payload + rate moved to AuthGatedRequestHandler.enforceIngress(). + // The executor no longer rejects an oversized or over-rate message; it proceeds to create the + // task. This documents that the handler gate is load-bearing — if a future path reaches + // execute() without the gate, oversized/over-rate requests would run unchecked (and, being + // post-execute, WOULD persist a row — which is exactly why the guard must stay upstream, not here). + const repo = makeFakeRepo(); + const eventBus = makeFakeEventBus(); + // maxPayloadBytes: 1 AND a fully-drained rate bucket — both would have rejected in the old code. + const limiter = new A2aLimiter({ ...DEFAULT_A2A_LIMITS, maxPayloadBytes: 1, ratePerMinute: 1 }, () => 0); + limiter.tryConsumeRate('grant-1'); // drain — old executor would have rejected on rate too + const executor = new MaestroA2aExecutor(repo as any, { + pollMs: 0, + sleep: async () => {}, + now: nowFn, + limiter, + }); + const ctx = makeRequestContext({ a2aPrincipal: DEFAULT_PRINCIPAL, messageText: 'this message is far larger than one byte' }); + + await executor.execute(ctx as any, eventBus as any); + + // Not rejected for payload/rate — the task proceeds and the local task IS created. + const rejectedForGovernance = eventBus.published.find( + (e: any) => e.kind === 'task' && e.status?.state === 'rejected', + ); + expect(rejectedForGovernance).toBeUndefined(); + expect(repo.createLocalTask).toHaveBeenCalled(); + }); + it('reservation release: skill_budget reject after reservation → slot freed, no leak', async () => { const repo = makeFakeRepo(); const eventBus = makeFakeEventBus(); diff --git a/src/bridge/a2a/executor.ts b/src/bridge/a2a/executor.ts index 39ad80d..17dc4d4 100644 --- a/src/bridge/a2a/executor.ts +++ b/src/bridge/a2a/executor.ts @@ -200,21 +200,11 @@ export class MaestroA2aExecutor implements AgentExecutor { return; } - // 1b. ガバナンス: payload サイズ + レートリミット（scope 算出前に弾く = flood に scope DB 処理させない） - if (this.limiter) { - const L = this.limiter.limits; - const bytes = Buffer.byteLength(JSON.stringify(requestContext.userMessage ?? {}), 'utf8'); - if (bytes > L.maxPayloadBytes) { - await this.rejectGoverned(eventBus, taskId, contextId, principal, `payload too large: ${bytes} > ${L.maxPayloadBytes}`, 'payload', initialTaskPublished); - safeFinished(); - return; - } - if (!this.limiter.tryConsumeRate(principal.grantId)) { - await this.rejectGoverned(eventBus, taskId, contextId, principal, `rate limit exceeded (${L.ratePerMinute}/min)`, 'rate', initialTaskPublished); - safeFinished(); - return; - } - } + // 1b. ガバナンス: payload サイズ + レートリミットは AuthGatedRequestHandler.enforceIngress() + // が execute() 呼び出し前に施行する。ここで拒否 Task を publish すると SDK ResultManager が + // それを永続化してしまい（拒否のたびに a2a_tasks 行が増える書き込み増幅）、rate-limit の + // フラッド防御を自ら損なうため、payload/rate はこの層では扱わない。concurrency と + // skill-budget は予約ライフサイクルの都合でこの下に残す。 // 2. acting user の実際の可視スコープを算出（IDOR 防止・fail-closed） const scope = await computeEffectiveScope(this.repo, principal); diff --git a/src/bridge/a2a/request-handler-auth-gate.test.ts b/src/bridge/a2a/request-handler-auth-gate.test.ts index ca68495..8d93054 100644 --- a/src/bridge/a2a/request-handler-auth-gate.test.ts +++ b/src/bridge/a2a/request-handler-auth-gate.test.ts @@ -216,6 +216,76 @@ describe('AuthGatedRequestHandler — resubscribe slot cap (Task 7)', () => { }); }); +// ─── ingress governance: sendMessage / sendMessageStream (2C-3 review #2) ───── +// payload + rate are enforced here, BEFORE execute() runs, so a denial throws a JSON-RPC +// error and persists nothing (the SDK only writes an a2a_tasks row once execute() publishes). + +describe('AuthGatedRequestHandler — ingress governance (send/stream)', () => { + const MSG = { + message: { kind: 'message', role: 'user', messageId: 'm1', parts: [{ kind: 'text', text: 'hello world' }] }, + } as any; + + function makeSendHandler(limiter: A2aLimiter) { + const store = new InMemoryTaskStore(); + const execute = vi.fn(async () => {}); + const fakeExecutor = { execute } as any; + const baseCard = buildBaseCard(BASE_DEPS); + const h = new AuthGatedRequestHandler(baseCard, store, fakeExecutor, undefined, limiter); + return { h, execute }; + } + + it('sendMessage: oversized payload → throws -32600 and execute() is never invoked', async () => { + const limiter = new A2aLimiter(resolveA2aLimits({ maxPayloadBytes: 1, ratePerMinute: 100 })); + const { h, execute } = makeSendHandler(limiter); + await expect(h.sendMessage(MSG, AUTH_CTX)).rejects.toMatchObject({ code: -32600 }); + expect(execute).not.toHaveBeenCalled(); + }); + + it('sendMessage: rate bucket drained → throws -32600 and execute() is never invoked', async () => { + const limiter = new A2aLimiter(resolveA2aLimits({ ratePerMinute: 1, maxPayloadBytes: 65536 })); + limiter.tryConsumeRate('g1'); // drain the single token (AUTH_CTX principal grantId = g1) + const { h, execute } = makeSendHandler(limiter); + await expect(h.sendMessage(MSG, AUTH_CTX)).rejects.toMatchObject({ code: -32600 }); + expect(execute).not.toHaveBeenCalled(); + }); + + it('sendMessage: unauthenticated → throws -32600 (no failed Task persisted, no execute)', async () => { + const limiter = new A2aLimiter(resolveA2aLimits({ ratePerMinute: 100 })); + const { h, execute } = makeSendHandler(limiter); + await expect(h.sendMessage(MSG, UNAUTH_CTX)).rejects.toMatchObject({ code: -32600 }); + expect(execute).not.toHaveBeenCalled(); + }); + + it('sendMessageStream: oversized payload → SYNCHRONOUS throw before the generator (no SSE, no execute)', () => { + const limiter = new A2aLimiter(resolveA2aLimits({ maxPayloadBytes: 1, ratePerMinute: 100 })); + const { h, execute } = makeSendHandler(limiter); + expect(() => h.sendMessageStream(MSG, AUTH_CTX)).toThrow(A2AError); + expect(execute).not.toHaveBeenCalled(); + }); + + it('sendMessageStream: rate drained → synchronous throw before the generator', () => { + const limiter = new A2aLimiter(resolveA2aLimits({ ratePerMinute: 1, maxPayloadBytes: 65536 })); + limiter.tryConsumeRate('g1'); + const { h, execute } = makeSendHandler(limiter); + expect(() => h.sendMessageStream(MSG, AUTH_CTX)).toThrow(A2AError); + expect(execute).not.toHaveBeenCalled(); + }); + + it('within limits: gate passes and consumes a rate token (second call is throttled)', () => { + // sendMessageStream returns the generator synchronously; the base body (and execute()) + // only runs on the first .next(), which would block on a quiet stub — so we assert the + // gate side effects instead: the first call passes and drains the single token, the + // second call is rejected, proving the first consumed it. + const limiter = new A2aLimiter(resolveA2aLimits({ ratePerMinute: 1, maxPayloadBytes: 65536 })); + const { h } = makeSendHandler(limiter); + let gen: AsyncGenerator | undefined; + expect(() => { gen = h.sendMessageStream(MSG, AUTH_CTX); }).not.toThrow(); + expect(gen).toBeDefined(); + void gen!.return?.(undefined); // unstarted generator → completes without running the body + expect(() => h.sendMessageStream(MSG, AUTH_CTX)).toThrow(A2AError); + }); +}); + // ─── maxStreamSeconds fires for a quiet stream (Task 7 review fix) ─────────── describe('AuthGatedRequestHandler — maxStreamSeconds wall-clock race', () => { diff --git a/src/bridge/a2a/request-handler.ts b/src/bridge/a2a/request-handler.ts index bf999b5..48d30f9 100644 --- a/src/bridge/a2a/request-handler.ts +++ b/src/bridge/a2a/request-handler.ts @@ -42,8 +42,13 @@ type CardDeps = { baseUrl: string; issuer: string; version: string }; * context.user が存在しない・isAuthenticated !== true の場合は A2AError.invalidRequest を投げ、 * JsonRpcTransportHandler がそれを -32600 の JSON-RPC エラーレスポンスに変換する。 * - * sendMessage / sendMessageStream はゲートしない：executor.execute() が - * principal のチェックを行い、unauthorized ならタスクを failed 状態で終了させる（既存の保護）。 + * sendMessage / sendMessageStream: limiter 注入時は execute() を呼ぶ前に ingress + * ガバナンス（auth → grantId → payload → rate）をこの層で施行する。ここで拒否すれば + * クリーンな JSON-RPC エラーになり、タスク行も監査行も一切永続化されない（SDK は + * execute() がイベントを publish して初めて a2a_tasks に書き込むため）。concurrency と + * skill-budget は予約ライフサイクルの都合で execute() 側に残すが、rate をここへ上げた + * ことで両者の永続化される拒否も ratePerMinute に抑制される。scope 違反など principal + * ベースの拒否は従来どおり execute() が failed タスクとして終了させる。 * * Task 7: limiter が注入された場合、認証チェックの後に rate-limit と * resubscribe スロットキャップを適用する（auth → rate → slot の順序を保証）。 @@ -62,6 +67,45 @@ export class AuthGatedRequestHandler extends DefaultRequestHandler { this.limiter = limiter; } + /** + * Ingress governance for message/send + message/stream, enforced BEFORE the SDK invokes + * the executor. A denial here throws a clean JSON-RPC error and persists NOTHING — the SDK + * only writes a task row once execute() publishes an event, and its own error-catch path + * would persist a `failed` Task even for a thrown execute(). Enforcing rate + payload up + * here removes the write-amplification flood vector. Mirrors the resubscribe gate: + * auth → grantId (fail-closed) → payload → rate. Without a limiter (tests / metering off) + * this is auth-only and send/stream fall through to execute() as before. + */ + private enforceIngress(params: { message?: unknown }, context?: ServerCallContext): void { + this.requireAuthenticated(context); + const grantId = this.meteredGrantId(context); + if (!this.limiter) return; // metering off (e.g. tests without a limiter) + const bytes = Buffer.byteLength(JSON.stringify(params.message ?? {}), 'utf8'); + if (bytes > this.limiter.limits.maxPayloadBytes) { + throw A2AError.invalidRequest( + `payload too large: ${bytes} > ${this.limiter.limits.maxPayloadBytes}`, + ); + } + if (!this.limiter.tryConsumeRate(grantId!)) { + throw A2AError.invalidRequest('rate limit exceeded'); + } + } + + override async sendMessage(params: Parameters[0], context?: ServerCallContext) { + this.enforceIngress(params, context); + return super.sendMessage(params, context); + } + + override sendMessageStream( + params: Parameters[0], + context?: ServerCallContext, + ): ReturnType { + // Synchronous gate before super returns the generator: a denial throws at the transport's + // call site (before SSE headers) → clean JSON-RPC error, no eventBus, no execute(), no rows. + this.enforceIngress(params, context); + return super.sendMessageStream(params, context); + } + override async getTask(params: Parameters[0], context?: ServerCallContext) { this.requireAuthenticated(context); const grantId = this.meteredGrantId(context); diff --git a/src/bridge/app-harness-api.ts b/src/bridge/app-harness-api.ts index 3a4ad96..82f07e9 100644 --- a/src/bridge/app-harness-api.ts +++ b/src/bridge/app-harness-api.ts @@ -34,7 +34,7 @@ const SPACE_FILES_RE = /^\/api\/local\/spaces\/([^/]+)\/files(?:\/|$|\?)/; * ordinary (non-admin) active user. The shape must match the Express.User * interface declared in src/bridge/auth.ts: * id, email, name, avatarUrl, role, status, orgIds, - * defaultVisibility, defaultVisibilityOrgId + * defaultVisibility, defaultVisibilityOrgId, hasLocalCredential * * viewerOf() reads req.user directly when authActive && req.user is set. * canManageSpace() passes when space.ownerId === user.id, so the caller must @@ -51,6 +51,7 @@ function buildSyntheticViewer(userId: string): Express.User { orgIds: [], defaultVisibility: 'private', defaultVisibilityOrgId: null, + hasLocalCredential: false, // token-scoped synthetic viewer, not a real credential holder } as Express.User; } diff --git a/src/bridge/auth.has-local-credential.test.ts b/src/bridge/auth.has-local-credential.test.ts new file mode 100644 index 0000000..0875cbc --- /dev/null +++ b/src/bridge/auth.has-local-credential.test.ts @@ -0,0 +1,96 @@ +/** + * hasLocalCredential on the session user (issue #799): drives whether the + * password-change UI (Settings → Preferences) is shown to a user. It's + * computed by passport.deserializeUser (see auth.ts), so it must be verified + * on a REAL subsequent request — not just the login response — through a + * protected route. + * + * setupAuth ONCE per file (beforeAll), mirroring admin-api.local.test.ts: + * passport.serializeUser/deserializeUser register onto a global stack and + * don't replace, so calling setupAuth per-test would leave earlier tests' + * deserializers (bound to already-closed repos) running first and erroring + * out the whole chain. A dedicated file also keeps this isolated from + * auth.local.test.ts's per-test setupAuth calls (same global passport + * singleton within one file/worker). + */ +import { afterAll, beforeAll, describe, it, expect } from 'vitest'; +import { mkdtempSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import express, { type Express } from 'express'; +import request from 'supertest'; +import { Repository } from '../db/repository.js'; +import { runMigrations } from '../db/migrate.js'; +import { setupAuth } from './auth.js'; +import type { AuthConfig } from '../config.js'; + +const AUTH: AuthConfig = { + sessionSecret: 'test', sessionMaxAge: 3600_000, secureCookie: false, + adminEmails: [], providers: {}, local: { enabled: true, allowSignup: true }, +}; + +describe('hasLocalCredential on the session user', () => { + let tempDir = ''; + let repo: Repository; + let app: Express; + + beforeAll(() => { + tempDir = mkdtempSync(join(tmpdir(), 'maestro-haslocalcred-')); + repo = new Repository(join(tempDir, 'orchestrator.db')); + runMigrations(repo.getDb()); + + app = express(); + const auth = setupAuth(repo, AUTH); + app.use(auth.sessionMiddleware); + app.use(auth.passportInit); + app.use(auth.passportSession); + app.use('/auth', auth.authRouter); + app.get('/api/auth/me', (req, res) => { + if (!req.isAuthenticated() || !req.user) { res.status(401).json({ error: 'unauthenticated' }); return; } + res.json(req.user); + }); + // Test-only login shortcut: bypasses password verification so a test can + // put an arbitrary existing (e.g. OAuth-created) user id into the session + // and exercise passport.deserializeUser on the next request — the same + // code path a real OAuth callback's req.login would take. + app.post('/test/login/:id', (req, res, next) => { + req.login({ id: req.params.id } as Express.User, (err) => { + if (err) { next(err); return; } + res.sendStatus(204); + }); + }); + }); + + afterAll(() => { + repo.close(); + if (tempDir) { rmSync(tempDir, { recursive: true, force: true }); tempDir = ''; } + }); + + it('is true after a local login', async () => { + repo.createLocalUser({ email: 'local-cred@x.com', password: 'pw12345678', role: 'user', status: 'active' }); + const agent = request.agent(app); + await agent.post('/auth/local').type('form').send({ email: 'local-cred@x.com', password: 'pw12345678' }); + const res = await agent.get('/api/auth/me'); + expect(res.status).toBe(200); + expect(res.body.hasLocalCredential).toBe(true); + // secret material must never leak onto the session user + expect(res.body.password).toBeUndefined(); + expect(res.body.passwordHash).toBeUndefined(); + }); + + it('is false for an OAuth-only account', async () => { + const oauthUser = repo.findOrCreateUserByOAuth({ + provider: 'gitea', + providerId: 'gitea-123', + email: 'oauth@x.com', + name: 'OAuth User', + avatarUrl: undefined, + }); + repo.updateUser(oauthUser.id, { status: 'active' }); + const agent = request.agent(app); + await agent.post(`/test/login/${oauthUser.id}`); + const res = await agent.get('/api/auth/me'); + expect(res.status).toBe(200); + expect(res.body.hasLocalCredential).toBe(false); + }); +}); diff --git a/src/bridge/auth.ts b/src/bridge/auth.ts index 2e868b8..42c0fd6 100644 --- a/src/bridge/auth.ts +++ b/src/bridge/auth.ts @@ -212,6 +212,13 @@ declare global { orgIds: string[]; defaultVisibility: 'private' | 'org' | 'public'; defaultVisibilityOrgId: string | null; + /** + * Whether this account has a local (email+password) credential set, as + * opposed to being OAuth-only (Google/Gitea). Drives whether the + * password-change UI (Settings → Preferences) is shown — never carries + * the credential itself. See repo.hasLocalCredential. + */ + hasLocalCredential: boolean; } } } @@ -513,6 +520,7 @@ async function handleOAuthCallback( orgIds: [], defaultVisibility: user.defaultVisibility ?? 'private', defaultVisibilityOrgId: user.defaultVisibilityOrgId ?? null, + hasLocalCredential: repo.hasLocalCredential(user.id), }; done(null, sessionUser); } catch (err) { @@ -659,6 +667,7 @@ function registerGiteaStrategy(repo: Repository, authConfig: AuthConfig): void { orgIds, defaultVisibility: user.defaultVisibility ?? 'private', defaultVisibilityOrgId: user.defaultVisibilityOrgId ?? null, + hasLocalCredential: repo.hasLocalCredential(user.id), }; done(null, sessionUser); } catch (err) { @@ -771,6 +780,7 @@ function createAuthRouter( orgIds: resolveOrgIds(repo, u.id), defaultVisibility: u.defaultVisibility ?? 'private', defaultVisibilityOrgId: u.defaultVisibilityOrgId ?? null, + hasLocalCredential: repo.hasLocalCredential(u.id), }); const readCreds = (req: Request): { email: string; password: string } | null => { @@ -937,6 +947,7 @@ export function setupAuth( orgIds: resolveOrgIds(repo, id), defaultVisibility: baseUser.defaultVisibility ?? 'private', defaultVisibilityOrgId: baseUser.defaultVisibilityOrgId ?? null, + hasLocalCredential: repo.hasLocalCredential(id), }; done(null, enriched); } catch (err) { diff --git a/src/bridge/console-ws-api.test.ts b/src/bridge/console-ws-api.test.ts index f5b1240..bc5a039 100644 --- a/src/bridge/console-ws-api.test.ts +++ b/src/bridge/console-ws-api.test.ts @@ -46,6 +46,7 @@ import { decideAccess, handleConsoleSocket, createConsoleStatusRouter, + createConsoleStatusStubRouter, createConsoleSessionRouter, createConsoleSessionsRouter, createConsoleSessionCloseRouter, @@ -478,6 +479,44 @@ describe('createConsoleStatusRouter', () => { }); }); +describe('createConsoleStatusStubRouter', () => { + // Issue #785: when SSH is disabled, the real createConsoleStatusRouter + // never mounts (no registry/resolveTask/resolveSshAccess to wire it + // with), but the UI still polls this path every 5s. The stub always + // answers `{ active: false }` without touching any task/registry. + function buildApp(opts: { authActive?: boolean; user?: any } = {}) { + const app = express(); + if (opts.user) { + app.use((req, _res, next) => { (req as any).user = opts.user; next(); }); + } + app.use('/api', createConsoleStatusStubRouter({ + requireAuth: (_req: any, _res: any, next: any) => next(), + authActive: opts.authActive, + })); + return app; + } + + it('always returns 200 { active: false } regardless of task id', async () => { + const app = buildApp({ user: { id: 'alice', role: 'user' } }); + const res = await request(app).get('/api/local/tasks/anything-at-all/console/status'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ active: false }); + }); + + it('authActive=true and unauthenticated → 401', async () => { + const app = buildApp({ authActive: true }); // no user middleware + const res = await request(app).get('/api/local/tasks/t1/console/status'); + expect(res.status).toBe(401); + }); + + it('no-auth (authActive=false): synthesizes a local admin user instead of 401', async () => { + const app = buildApp({ authActive: false }); + const res = await request(app).get('/api/local/tasks/t1/console/status'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ active: false }); + }); +}); + describe('createConsoleSessionRouter', () => { it('no-auth (authActive=false): synthesizes a local admin user instead of 401', async () => { let seenUser: any; diff --git a/src/bridge/console-ws-api.ts b/src/bridge/console-ws-api.ts index df2de82..33d6348 100644 --- a/src/bridge/console-ws-api.ts +++ b/src/bridge/console-ws-api.ts @@ -431,6 +431,41 @@ export function createConsoleStatusRouter(deps: { return r; } +/** + * REST router exposing GET /local/tasks/:taskId/console/status as a minimal + * stub for environments where SSH is disabled (`ssh.enabled=false`) or the + * key isn't configured, so `createConsoleStatusRouter` (and the session + * registry / resolveTask / resolveSshAccess it needs) never gets mounted. + * + * Without SOME router on this path, the UI's App.tsx 5-second console/status + * poll (see the "issue #347" note on `createConsoleStatusRouter` above) 404s + * forever on every task, spamming the DevTools console with unsuppressible + * network errors even though the feature is intentionally off and there is + * no functional harm (issue #785). + * + * Unlike the real router, this never touches a task/registry — SSH consoles + * cannot exist when this stub is mounted, so the answer is unconditionally + * `{ active: false }`. The auth gate is still applied (mirrors the real + * router's `requireAuth` + no-auth synthetic-user behavior) purely for + * consistency; there's no data to protect here. + */ +export function createConsoleStatusStubRouter(deps: { requireAuth: any; authActive?: boolean }): Router { + const r = Router(); + r.get( + '/local/tasks/:taskId/console/status', + deps.requireAuth, + (req: Request, res: Response) => { + const user = resolveConsoleUser(req, deps.authActive ?? true); + if (!user) { + res.status(401).json({ error: 'unauthenticated' }); + return; + } + res.json({ active: false }); + }, + ); + return r; +} + /** Idle threshold: a session with no activity for > this is reported `idle`. */ const CONSOLE_IDLE_THRESHOLD_MS = 60_000; /** A session is `agent_active` if the agent wrote input within this window. */ diff --git a/src/bridge/server-subsystems.test.ts b/src/bridge/server-subsystems.test.ts index ede2426..1473986 100644 --- a/src/bridge/server-subsystems.test.ts +++ b/src/bridge/server-subsystems.test.ts @@ -41,6 +41,12 @@ describe('createCoreServer subsystem wiring', () => { // MCP admin router is mounted (not a 404 from the express fallthrough). const res = await request(app).get('/api/mcp/servers'); expect(res.status).not.toBe(404); + // Issue #785: even with SSH disabled, the console/status poll must not + // 404 — a stub router still answers 200 active=false so the UI's 5s + // poll doesn't spam DevTools with unsuppressible network errors. + const statusRes = await request(app).get('/api/local/tasks/some-task/console/status'); + expect(statusRes.status).toBe(200); + expect(statusRes.body).toEqual({ active: false }); }); it('does NOT mount MCP routers when the key is absent', async () => { diff --git a/src/bridge/space-api.test.ts b/src/bridge/space-api.test.ts index f5fc462..4b65f53 100644 --- a/src/bridge/space-api.test.ts +++ b/src/bridge/space-api.test.ts @@ -69,6 +69,64 @@ describe('space-api', () => { expect(list.find((s: any) => s.id === created.id)).toBeUndefined(); // archived hidden by default }); + it('PATCH display stores favorite/hidden and hiding clears favorite', async () => { + const created = (await request(app).post('/api/local/spaces').send({ title: '整理対象' })).body; + + const fav = await request(app) + .patch(`/api/local/spaces/${created.id}/display`) + .send({ favorite: true }); + expect(fav.status).toBe(200); + expect(fav.body).toEqual({ favorite: true, hidden: false }); + + const hidden = await request(app) + .patch(`/api/local/spaces/${created.id}/display`) + .send({ hidden: true }); + expect(hidden.status).toBe(200); + expect(hidden.body).toEqual({ favorite: false, hidden: true }); + + const list = (await request(app).get('/api/local/spaces')).body; + const row = list.find((s: any) => s.id === created.id); + expect(row.favorite).toBe(false); + expect(row.hidden).toBe(true); + }); + + it('display prefs are per-user and do not affect other viewers', async () => { + const owner = repo.createUser({ email: 'display-owner@example.com', name: 'Owner', role: 'user', status: 'active' }); + const viewer = repo.createUser({ email: 'display-viewer@example.com', name: 'Viewer', role: 'user', status: 'active' }); + const space = await repo.createSpace({ + kind: 'case', title: '共有表示設定', description: '', ownerId: owner.id, + visibility: 'public', visibilityScopeOrgId: null, brandColor: null, + }); + + const makeAppAs = (userId: string) => { + const a = express(); + a.use((req, _res, next) => { (req as any).user = { id: userId, role: 'user', orgIds: [] }; next(); }); + a.use('/api/local/spaces', createSpaceApi({ + repo, dataRoot: join(dir, 'data', 'users'), worktreeDir: join(dir, 'wt'), authActive: true, + })); + return a; + }; + + const ownerApp = makeAppAs(owner.id); + const viewerApp = makeAppAs(viewer.id); + const fav = await request(ownerApp).patch(`/api/local/spaces/${space.id}/display`).send({ favorite: true }); + expect(fav.status).toBe(200); + + const ownerList = (await request(ownerApp).get('/api/local/spaces')).body; + const viewerList = (await request(viewerApp).get('/api/local/spaces')).body; + expect(ownerList.find((s: any) => s.id === space.id).favorite).toBe(true); + expect(viewerList.find((s: any) => s.id === space.id).favorite).toBe(false); + }); + + it('PATCH display returns 404 for spaces the viewer cannot see', async () => { + const other = await repo.createSpace({ + kind: 'case', title: 'secret-display', description: '', ownerId: 'user-2', + visibility: 'private', visibilityScopeOrgId: null, brandColor: null, + }); + const res = await request(app).patch(`/api/local/spaces/${other.id}/display`).send({ favorite: true }); + expect(res.status).toBe(404); + }); + it('lists + serves files from the space persistent workspace and rejects traversal', async () => { const created = (await request(app).post('/api/local/spaces').send({ title: '案件F' })).body; const filesDir = join(dir, 'wt', 'space', created.id, 'files'); diff --git a/src/bridge/space-api.ts b/src/bridge/space-api.ts index f3eedfb..806a53b 100644 --- a/src/bridge/space-api.ts +++ b/src/bridge/space-api.ts @@ -63,7 +63,7 @@ export function createSpaceApi(deps: SpaceApiDeps): Router { router.get('/', async (req, res) => { const viewer = viewerOf(req, deps.authActive); await repo.ensurePersonalSpace(viewer.id, viewer.name ?? undefined); - const spaces = await repo.listSpaces({ viewer }); + const spaces = await repo.listSpacesWithDisplayPrefs({ viewer }); res.json(spaces.map(s => ({ ...s, myRole: repo.getSpaceMemberRole(s.id, viewer.id) }))); }); @@ -77,6 +77,28 @@ export function createSpaceApi(deps: SpaceApiDeps): Router { res.json({ ...space, myRole }); }); + router.patch("/:id/display", jsonParser, async (req, res) => { + const viewer = viewerOf(req, deps.authActive); + const space = await repo.getSpace(req.params.id, { viewer }); + if (!space) return res.status(404).json({ error: "not found" }); + + const patch: { favorite?: boolean; hidden?: boolean } = {}; + if (req.body?.favorite !== undefined) { + if (typeof req.body.favorite !== "boolean") return res.status(400).json({ error: "favorite must be boolean" }); + patch.favorite = req.body.favorite; + } + if (req.body?.hidden !== undefined) { + if (typeof req.body.hidden !== "boolean") return res.status(400).json({ error: "hidden must be boolean" }); + patch.hidden = req.body.hidden; + } + if (patch.favorite === undefined && patch.hidden === undefined) { + return res.status(400).json({ error: "favorite or hidden is required" }); + } + + const prefs = repo.updateSpaceDisplayPrefs(viewer.id, space.id, patch); + res.json(prefs); + }); + // 作成（案件スペース）。DB 行 → 雛形作成。雛形失敗時は行を消してロールバック。 router.post('/', jsonParser, async (req, res) => { const viewer = viewerOf(req, deps.authActive); diff --git a/src/bridge/ssh-subsystem.ts b/src/bridge/ssh-subsystem.ts index cb5c81a..0cd22ba 100644 --- a/src/bridge/ssh-subsystem.ts +++ b/src/bridge/ssh-subsystem.ts @@ -38,6 +38,7 @@ import { listAgentConsoleSessions, closeConsoleSessionByUser } from '../engine/t import { __setActiveSessionLookup } from '../engine/agent-loop.js'; import { createConsoleStatusRouter, + createConsoleStatusStubRouter, createConsoleSessionRouter, createConsoleSessionsRouter, createConsoleSessionCloseRouter, @@ -94,6 +95,21 @@ export function setupSshSubsystem(app: express.Application, deps: SshSubsystemDe if (!sshConfig.enabled) { setSshSubsystem(null); __setActiveSessionLookup(null); + // SSH is off: the real console/status router (and the registry / + // resolveTask / resolveSshAccess it needs) never gets mounted below, + // but the UI still polls GET .../console/status every 5s regardless of + // SSH config. Without a stand-in, that poll 404s forever and spams the + // DevTools console (issue #785). Mount a minimal stub that always + // answers `{ active: false }` — scope note: the sibling + // `!isKeyConfigured()` branch below has the SAME 404-polling problem + // but is deliberately left untouched here (separate issue). + app.use( + '/api', + createConsoleStatusStubRouter({ + authActive, + requireAuth: authActive ? requireAuth : (_req: Request, _res: Response, next: NextFunction) => next(), + }), + ); } else if (!isKeyConfigured()) { logger.warn('[ssh] MCP_ENCRYPTION_KEY not configured — SSH features disabled'); setSshSubsystem(null); @@ -282,6 +298,7 @@ export function setupSshSubsystem(app: express.Application, deps: SshSubsystemDe orgIds: resolveOrgIds(repo, user.id), defaultVisibility: 'private', defaultVisibilityOrgId: null, + hasLocalCredential: repo.hasLocalCredential(user.id), }; const task = await repo.getLocalTask(idNum, { viewer }); if (!task) return null; diff --git a/src/config-manager.extra-body.test.ts b/src/config-manager.extra-body.test.ts new file mode 100644 index 0000000..97268f4 --- /dev/null +++ b/src/config-manager.extra-body.test.ts @@ -0,0 +1,101 @@ +// src/config-manager.extra-body.test.ts +// +// extraBody の「部分更新でキー削除が反映される」保証を検証する。 +// +// 到達経路の実態: +// - extraBody は現状 llm.workers[] / provider.workers[] の配列内にのみ存在する +// - deepMergeConfig は配列を丸ごと置換する（再帰しない）ため、workers 配列の +// PUT では extraBody の旧キー残留はそもそも起きない（Test A がこれを検証） +// - deepMergeConfig 内の `key === 'extraBody'` ガードは、将来 per-worker の +// partial merge 経路（配列でなくオブジェクトとして worker を merge する経路） +// が追加された場合の防御。Test B が未知セクション経由の再帰でこのガード自体を +// 検証する（ガードを外すと Test B は失敗する = 実効性のある回帰テスト） +import { describe, it, expect, beforeEach } from 'vitest'; +import { mkdtempSync, writeFileSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { ConfigManager } from './config-manager.js'; + +describe('ConfigManager extraBody atomic replacement', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), 'config-manager-extra-body-')); + }); + + it('workers 配列更新で extraBody の旧キーが残らない（配列丸ごと置換）', () => { + // 実際の UI/API 経路: llm.workers はまるごと配列で PUT される。 + // deepMergeConfig は配列を再帰せず丸ごと置換するので、 + // extraBody から削除したキー（top_k）は結果に残らない。 + const configPath = join(tempDir, 'config.yaml'); + writeFileSync(configPath, [ + 'llm:', + ' workers:', + ' - id: gpu1', + ' connection_type: direct', + ' endpoint: http://gpu1.example/v1', + ' model: test-model', + ' extra_body:', + ' reasoning_effort: max', + ' top_k: 20', + ].join('\n')); + + const cm = new ConfigManager(configPath); + expect((cm.getConfig() as any).llm.workers[0].extraBody) + .toEqual({ reasoning_effort: 'max', top_k: 20 }); + + const result = cm.updateConfig({ + llm: { + workers: [ + { + id: 'gpu1', + connectionType: 'direct', + endpoint: 'http://gpu1.example/v1', + model: 'test-model', + extraBody: { reasoning_effort: 'max' }, // top_k を削除した状態で PUT + }, + ], + }, + }); + expect(result.ok).toBe(true); + + const updated = (cm.getConfig() as any).llm.workers[0]; + expect(updated.extraBody).toEqual({ reasoning_effort: 'max' }); + expect(updated.extraBody).not.toHaveProperty('top_k'); + }); + + it('deepMergeConfig が再帰到達する経路でも extraBody は丸ごと置換される（ガード検証）', () => { + // deepMergeConfig の extraBody ガード自体の検証。 + // updateConfig は未知のトップレベルセクションも素通しで merge → YAML 書き出し + // →再読込するため、プレーンオブジェクトのセクション内に extraBody を置くと + // deepMergeConfig の再帰がそこに到達する。ガードが無いと再帰マージで + // 旧キー b が残留し、このテストは失敗する。 + const configPath = join(tempDir, 'config.yaml'); + writeFileSync(configPath, [ + 'llm:', + ' workers:', + ' - id: gpu1', + ' connection_type: direct', + ' endpoint: http://gpu1.example/v1', + ' model: test-model', + 'synthetic_section:', + ' extra_body:', + ' a: 1', + ' b: 2', + ].join('\n')); + + const cm = new ConfigManager(configPath); + expect((cm.getConfig() as any).syntheticSection.extraBody).toEqual({ a: 1, b: 2 }); + + const result = cm.updateConfig({ + syntheticSection: { + extraBody: { a: 1 }, // b を削除した partial update + }, + }); + expect(result.ok).toBe(true); + + const updated = (cm.getConfig() as any).syntheticSection.extraBody; + expect(updated).toEqual({ a: 1 }); + expect(updated).not.toHaveProperty('b'); + }); +}); diff --git a/src/config-manager.ts b/src/config-manager.ts index 1de1cbd..015d0f5 100644 --- a/src/config-manager.ts +++ b/src/config-manager.ts @@ -308,7 +308,13 @@ function deepMergeConfig(base: any, override: any): any { if (typeof override !== 'object' || Array.isArray(override)) return override; const result = { ...base }; for (const [key, value] of Object.entries(override)) { - if (typeof value === 'object' && value !== null && !Array.isArray(value) && typeof result[key] === 'object') { + // extraBody は不透明 JSON なので常に丸ごと置換（キー削除を可能にする）。 + // 現状 extraBody は workers 配列内にのみ存在し、配列は上の分岐で丸ごと + // 置換されるためこの分岐は防御的（将来 per-worker partial merge 経路が + // 追加された場合の取りこぼし防止）。 + if (key === 'extraBody') { + result[key] = value; + } else if (typeof value === 'object' && value !== null && !Array.isArray(value) && typeof result[key] === 'object') { result[key] = deepMergeConfig(result[key], value); } else { result[key] = value; diff --git a/src/config-normalize.extra-body.test.ts b/src/config-normalize.extra-body.test.ts new file mode 100644 index 0000000..c728026 --- /dev/null +++ b/src/config-normalize.extra-body.test.ts @@ -0,0 +1,296 @@ +/** + * Tests for extraBody / reasoningEfforts / reasoningEffortMode propagation + * through every config-normalize.ts path (Task 2 of the LLM extra-body / + * reasoning-effort passthrough feature). + * + * Coverage matrix: + * - normalizeLlmWorker: extraBody validated (drop non-plain-object), + * reasoningEfforts filtered/deduped (drop empty result), + * reasoningEffortMode coerced to one of the two allowed literals or dropped + * - syncProviderFromLlm mirroredWorkers branch: 3 fields carried into + * provider.workers when provider.workers didn't already match by id + * - syncProviderFromLlm early-return branch: 3 fields synced per-field onto + * pre-existing matching provider.workers rows (mirrors returnProgress + * handling added for Codex P2) + */ +import { describe, expect, it } from 'vitest'; +import { normalizeConfig } from './config-normalize.js'; + +describe('normalizeLlmWorker — extraBody/reasoningEfforts/reasoningEffortMode validation', () => { + it('keeps a plain-object extraBody', () => { + const out = normalizeConfig({ + configVersion: 2, + llm: { + workers: [{ + id: 'gpu1', + connectionType: 'direct', + endpoint: 'http://x:8080/v1', + model: 'm', + extraBody: { reasoning_effort: 'high' }, + }], + }, + } as never); + expect(out.llm?.workers[0]).toMatchObject({ extraBody: { reasoning_effort: 'high' } }); + }); + + it.each([ + ['string', 'nope'], + ['array', ['a', 'b']], + ['null', null], + ])('drops extraBody when it is a %s, not a plain object', (_label, badValue) => { + const out = normalizeConfig({ + configVersion: 2, + llm: { + workers: [{ + id: 'gpu1', + connectionType: 'direct', + endpoint: 'http://x:8080/v1', + model: 'm', + extraBody: badValue, + }], + }, + } as never); + expect(out.llm?.workers[0]).not.toHaveProperty('extraBody'); + }); + + it('filters non-string/blank entries and dedupes reasoningEfforts', () => { + const out = normalizeConfig({ + configVersion: 2, + llm: { + workers: [{ + id: 'gpu1', + connectionType: 'direct', + endpoint: 'http://x:8080/v1', + model: 'm', + reasoningEfforts: ['high', 'high', '', ' ', 42, 'low'], + }], + }, + } as never); + expect(out.llm?.workers[0]).toMatchObject({ reasoningEfforts: ['high', 'low'] }); + }); + + it('drops reasoningEfforts entirely when filtering leaves nothing', () => { + const out = normalizeConfig({ + configVersion: 2, + llm: { + workers: [{ + id: 'gpu1', + connectionType: 'direct', + endpoint: 'http://x:8080/v1', + model: 'm', + reasoningEfforts: ['', ' ', 42], + }], + }, + } as never); + expect(out.llm?.workers[0]).not.toHaveProperty('reasoningEfforts'); + }); + + it.each(['body', 'chat_template_kwargs'] as const)( + 'accepts reasoningEffortMode %s', + (mode) => { + const out = normalizeConfig({ + configVersion: 2, + llm: { + workers: [{ + id: 'gpu1', + connectionType: 'direct', + endpoint: 'http://x:8080/v1', + model: 'm', + reasoningEffortMode: mode, + }], + }, + } as never); + expect(out.llm?.workers[0]).toMatchObject({ reasoningEffortMode: mode }); + }, + ); + + it('drops reasoningEffortMode when it is not one of the two allowed literals', () => { + const out = normalizeConfig({ + configVersion: 2, + llm: { + workers: [{ + id: 'gpu1', + connectionType: 'direct', + endpoint: 'http://x:8080/v1', + model: 'm', + reasoningEffortMode: 'bogus', + }], + }, + } as never); + expect(out.llm?.workers[0]).not.toHaveProperty('reasoningEffortMode'); + }); + + it('omitted fields stay absent', () => { + const out = normalizeConfig({ + configVersion: 2, + llm: { + workers: [{ id: 'gpu1', connectionType: 'direct', endpoint: 'http://x:8080/v1', model: 'm' }], + }, + } as never); + expect(out.llm?.workers[0]).not.toHaveProperty('extraBody'); + expect(out.llm?.workers[0]).not.toHaveProperty('reasoningEfforts'); + expect(out.llm?.workers[0]).not.toHaveProperty('reasoningEffortMode'); + }); +}); + +describe('extraBody/reasoningEfforts/reasoningEffortMode mirroring into provider.workers', () => { + it('v2 llm.workers fields survive normalization and mirror into provider.workers (mirroredWorkers branch)', () => { + const out = normalizeConfig({ + configVersion: 2, + llm: { + workers: [{ + id: 'gpu1', + connectionType: 'direct', + endpoint: 'http://x:8080/v1', + model: 'm', + extraBody: { foo: 'bar' }, + reasoningEfforts: ['high', 'low'], + reasoningEffortMode: 'chat_template_kwargs', + }], + }, + } as never); + expect(out.llm?.workers[0]).toMatchObject({ + extraBody: { foo: 'bar' }, + reasoningEfforts: ['high', 'low'], + reasoningEffortMode: 'chat_template_kwargs', + }); + expect(out.provider?.workers?.[0]).toMatchObject({ + id: 'gpu1', + extraBody: { foo: 'bar' }, + reasoningEfforts: ['high', 'low'], + reasoningEffortMode: 'chat_template_kwargs', + }); + }); + + it('v1 provider.workers fields carry into the generated llm block (workerFromProvider)', () => { + const out = normalizeConfig({ + provider: { + baseUrl: 'http://x:11434/v1', + model: 'm', + workers: [{ + id: 'gpu1', + endpoint: 'http://x:8080/v1', + extraBody: { foo: 'bar' }, + reasoningEfforts: ['high'], + reasoningEffortMode: 'body', + }], + }, + } as never); + expect(out.llm?.workers[0]).toMatchObject({ + id: 'gpu1', + extraBody: { foo: 'bar' }, + reasoningEfforts: ['high'], + reasoningEffortMode: 'body', + }); + }); +}); + +describe('v1 provider.workers[] legacy path validates passthrough fields (Codex P2)', () => { + it.each([ + ['string', 'oops'], + ['array', [1, 2]], + ])('drops extraBody when the legacy provider.workers[] value is a %s', (_label, badValue) => { + const out = normalizeConfig({ + provider: { + baseUrl: 'http://x:11434/v1', + model: 'm', + workers: [{ + id: 'gpu1', + endpoint: 'http://x:8080/v1', + extraBody: badValue, + }], + }, + } as never); + expect(out.llm?.workers[0]).not.toHaveProperty('extraBody'); + expect(out.provider?.workers?.[0]).not.toHaveProperty('extraBody'); + }); + + it('filters blank entries and dedupes reasoningEfforts on the legacy provider.workers[] path, matching v2 behavior', () => { + // Matches normalizeLlmWorker's existing semantics exactly: entries are + // kept when `trim().length > 0` but the *stored* value is NOT itself + // trimmed (e.g. ' low ' survives filtering with its surrounding spaces + // intact) — see the identical assertion shape in the v2 test above + // ('filters non-string/blank entries and dedupes reasoningEfforts'). + const out = normalizeConfig({ + provider: { + baseUrl: 'http://x:11434/v1', + model: 'm', + workers: [{ + id: 'gpu1', + endpoint: 'http://x:8080/v1', + reasoningEfforts: ['high', '', 'high', ' low '], + }], + }, + } as never); + expect(out.llm?.workers[0]).toMatchObject({ reasoningEfforts: ['high', ' low '] }); + }); + + it('drops reasoningEffortMode on the legacy provider.workers[] path when not an allowed literal', () => { + const out = normalizeConfig({ + provider: { + baseUrl: 'http://x:11434/v1', + model: 'm', + workers: [{ + id: 'gpu1', + endpoint: 'http://x:8080/v1', + reasoningEffortMode: 'bogus', + }], + }, + } as never); + expect(out.llm?.workers[0]).not.toHaveProperty('reasoningEffortMode'); + expect(out.provider?.workers?.[0]).not.toHaveProperty('reasoningEffortMode'); + }); +}); + +// syncProviderFromLlm early-return branch: loadConfig が同 id の legacy +// provider.workers を先に合成する一般的な v2 構成では mirroredWorkers branch +// を通らず early-return する。returnProgress (Codex P2) と同じパターンで +// extraBody/reasoningEfforts/reasoningEffortMode も per-field 同期する必要がある。 +describe('extraBody/reasoningEfforts/reasoningEffortMode sync when provider.workers already match', () => { + it('copies all 3 fields onto pre-existing matching provider.workers rows', () => { + const out = normalizeConfig({ + configVersion: 2, + provider: { + workers: [{ id: 'default', endpoint: 'http://x:8080/v1' }], + }, + llm: { + workers: [{ + id: 'default', + connectionType: 'direct', + endpoint: 'http://x:8080/v1', + model: 'm', + extraBody: { foo: 'bar' }, + reasoningEfforts: ['high'], + reasoningEffortMode: 'chat_template_kwargs', + }], + }, + } as never); + expect(out.provider?.workers?.[0]).toMatchObject({ + id: 'default', + extraBody: { foo: 'bar' }, + reasoningEfforts: ['high'], + reasoningEffortMode: 'chat_template_kwargs', + }); + }); + + it('clears stale fields on provider.workers when the v2 side no longer sets them', () => { + const out = normalizeConfig({ + configVersion: 2, + provider: { + workers: [{ + id: 'default', + endpoint: 'http://x:8080/v1', + extraBody: { foo: 'bar' }, + reasoningEfforts: ['high'], + reasoningEffortMode: 'chat_template_kwargs', + }], + }, + llm: { + workers: [{ id: 'default', connectionType: 'direct', endpoint: 'http://x:8080/v1', model: 'm' }], + }, + } as never); + expect(out.provider?.workers?.[0]).not.toHaveProperty('extraBody'); + expect(out.provider?.workers?.[0]).not.toHaveProperty('reasoningEfforts'); + expect(out.provider?.workers?.[0]).not.toHaveProperty('reasoningEffortMode'); + }); +}); diff --git a/src/config-normalize.ts b/src/config-normalize.ts index faab09c..6ec35d1 100644 --- a/src/config-normalize.ts +++ b/src/config-normalize.ts @@ -169,6 +169,17 @@ function resolveConfigVersion(raw: unknown): 1 | 2 { * `tools.task_upload_max_size_mb` / `tools.trash_retention_days` * are mirrored into `storage.*` * - legacy keys are PRESERVED so downstream readers keep working. + * + * Also syncs the validated extraBody/reasoningEfforts/reasoningEffortMode + * back onto `out.provider.workers` (see `syncProviderFromLlm` call below): + * `config.provider.workers` — not `config.llm.workers` — is what the + * runtime actually reads to build LLM clients (worker-manager.ts, + * worker.ts, worker-bootstrap.ts all consume `config.provider.workers` + * during this compat window, and `normalizeWorkerDefs` in config.ts spreads + * worker fields through unvalidated). Without this sync, a malformed value + * on a pure v1 config (`configVersion` unset) would be dropped from the + * `llm.*` mirror but survive untouched on `provider.workers` — the exact + * field the request body is actually built from (Codex P2). */ function migrateV1InPlace(out: Record): void { const provider = (out.provider ?? {}) as ProviderConfig; @@ -179,6 +190,8 @@ function migrateV1InPlace(out: Record): void { : llmFromProvider(provider); out.llm = llm; + syncProviderFromLlm(out, llm); + out.storage = buildStorage(out); } @@ -271,6 +284,15 @@ function syncProviderFromLlm(out: Record, llm: LlmConfig): void if (!src) continue; if (src.returnProgress !== undefined) existing.returnProgress = src.returnProgress; else delete existing.returnProgress; + // extraBody/reasoningEfforts/reasoningEffortMode: clear stale values + // first (matches the delete-when-absent semantics above), then + // re-apply via the shared validator so a malformed `src` (defensive — + // `src` should already be normalized upstream, but this keeps the + // contract regardless) can't resurrect a bad value on `existing`. + delete existing.extraBody; + delete existing.reasoningEfforts; + delete existing.reasoningEffortMode; + applyPassthroughFields(existing, src as unknown as Record); } return; } @@ -294,6 +316,7 @@ function syncProviderFromLlm(out: Record, llm: LlmConfig): void def.proxy = true; def.proxyType = 'litellm'; } + applyPassthroughFields(def, w as unknown as Record); return def; }); @@ -393,6 +416,7 @@ function workerFromProvider(w: WorkerDef, providerModel: string | undefined): Ll if (w.healthcheckIntervalSeconds !== undefined) { worker.healthcheckIntervalSeconds = w.healthcheckIntervalSeconds; } + applyPassthroughFields(worker, w as unknown as Record); return worker; } @@ -441,9 +465,53 @@ function normalizeLlmWorker(w: Partial & Record): if (typeof w.healthcheckIntervalSeconds === 'number') { worker.healthcheckIntervalSeconds = w.healthcheckIntervalSeconds; } + applyPassthroughFields(worker, w as Record); return worker; } +/** + * Validate + copy the passthrough LLM fields (`extraBody` / `reasoningEfforts` + * / `reasoningEffortMode`) from a raw source object onto a target worker. + * + * These three fields skip normal type-checking further downstream: + * `OpenAICompatClient.sanitizeExtraBody` only strips reserved *keys* via + * `Object.entries(extra)` — it does not guard against `extra` itself being a + * non-object (e.g. `Object.entries("oops")` yields `[['0','o'], ...]`, which + * gets spread into the wire request body as garbage fields). So a malformed + * value must never survive normalization, on ANY code path that produces a + * worker (v1 `provider.workers[]`, v2 `llm.workers[]`, or the mirrors between + * them) — not just the hand-written v2 `normalizeLlmWorker` path. + * + * Malformed values are dropped (left unset on `target`), never copied as-is. + * Sets the field on `target` when `src` has a valid value; otherwise leaves + * `target`'s existing value for that field untouched (callers that need + * "clear stale field when absent" semantics, e.g. the early-return + * per-field sync in `syncProviderFromLlm`, must delete first — see there). + */ +function applyPassthroughFields( + target: { + extraBody?: Record; + reasoningEfforts?: string[]; + reasoningEffortMode?: 'body' | 'chat_template_kwargs'; + }, + src: Record, +): void { + if (src.extraBody !== null && typeof src.extraBody === 'object' && !Array.isArray(src.extraBody)) { + target.extraBody = src.extraBody as Record; + } + if (Array.isArray(src.reasoningEfforts)) { + const efforts = Array.from(new Set( + (src.reasoningEfforts as unknown[]).filter( + (e): e is string => typeof e === 'string' && e.trim().length > 0, + ), + )); + if (efforts.length > 0) target.reasoningEfforts = efforts; + } + if (src.reasoningEffortMode === 'chat_template_kwargs' || src.reasoningEffortMode === 'body') { + target.reasoningEffortMode = src.reasoningEffortMode; + } +} + function defaultWorker(endpoint: string, model: string): LlmWorkerDef { if (model === EMPTY_MODEL) { logger.warn( diff --git a/src/config.extra-body.test.ts b/src/config.extra-body.test.ts new file mode 100644 index 0000000..ffe76e9 --- /dev/null +++ b/src/config.extra-body.test.ts @@ -0,0 +1,24 @@ +import { describe, it, expect } from 'vitest'; +import { transformKeys, toSnakeKeys } from './config.js'; + +describe('transformKeys extra_body passthrough', () => { + it('extra_body の中身のキーは camelCase 変換しない', () => { + const input = { + llm: { workers: [{ id: 'w1', extra_body: { reasoning_effort: 'max', chat_template_kwargs: { enable_thinking: true } } }] }, + }; + const out = transformKeys(input) as any; + expect(out.llm.workers[0].extraBody).toEqual({ + reasoning_effort: 'max', + chat_template_kwargs: { enable_thinking: true }, + }); + }); + + it('toSnakeKeys は extraBody の中身を snake_case 変換しない（往復不変）', () => { + const camel = { llm: { workers: [{ id: 'w1', extraBody: { reasoning_effort: 'max', someCamelKey: 1 } }] } }; + const snake = toSnakeKeys(camel) as any; + expect(snake.llm.workers[0].extra_body).toEqual({ reasoning_effort: 'max', someCamelKey: 1 }); + // 往復して壊れないこと + const back = transformKeys(snake) as any; + expect(back.llm.workers[0].extraBody).toEqual(camel.llm.workers[0].extraBody); + }); +}); diff --git a/src/config.ts b/src/config.ts index f8712b6..bbae930 100644 --- a/src/config.ts +++ b/src/config.ts @@ -171,6 +171,14 @@ export interface WorkerDef { * without an Authorization header. */ apiKey?: string; + /** OpenAI 互換 request body へ浅いマージする任意 JSON（例: {reasoning_effort: 'max'}）。 + * 中身のキーは変換されずそのまま wire に載る。秘密値を入れないこと（config API でマスクされない）。 */ + extraBody?: Record; + /** このワーカーが受け付ける reasoning effort の宣言リスト（自由文字列、例 ['low','medium','high','max']）。 */ + reasoningEfforts?: string[]; + /** ジョブ単位 effort の注入先。body=トップレベル reasoning_effort（vLLM）、 + * chat_template_kwargs=chat_template_kwargs.reasoning_effort（llama-server）。既定 body。 */ + reasoningEffortMode?: 'body' | 'chat_template_kwargs'; } export interface ProviderConfig { @@ -453,6 +461,14 @@ export interface LlmWorkerDef { /** llama.cpp の prompt 評価進捗ストリーム（return_progress）を要求する。既定 off。 */ returnProgress?: boolean; healthcheckIntervalSeconds?: number; + /** OpenAI 互換 request body へ浅いマージする任意 JSON（例: {reasoning_effort: 'max'}）。 + * 中身のキーは変換されずそのまま wire に載る。秘密値を入れないこと（config API でマスクされない）。 */ + extraBody?: Record; + /** このワーカーが受け付ける reasoning effort の宣言リスト（自由文字列、例 ['low','medium','high','max']）。 */ + reasoningEfforts?: string[]; + /** ジョブ単位 effort の注入先。body=トップレベル reasoning_effort（vLLM）、 + * chat_template_kwargs=chat_template_kwargs.reasoning_effort（llama-server）。既定 body。 */ + reasoningEffortMode?: 'body' | 'chat_template_kwargs'; } /** @@ -641,13 +657,17 @@ function toCamel(s: string): string { return s.replace(/_([a-z])/g, (_, c: string) => c.toUpperCase()); } +/** これらのキーの「値」はプロバイダーへ素通しする JSON なのでキー変換しない + * （migrate-config.ts の複製変換からも参照される） */ +export const OPAQUE_CONFIG_KEYS = new Set(['extra_body', 'extraBody']); + export function transformKeys(obj: unknown): unknown { if (Array.isArray(obj)) return obj.map(transformKeys); if (obj !== null && typeof obj === 'object') { return Object.fromEntries( Object.entries(obj as Record).map(([k, v]) => [ toCamel(k), - transformKeys(v), + OPAQUE_CONFIG_KEYS.has(k) ? v : transformKeys(v), ]) ); } @@ -664,7 +684,7 @@ export function toSnakeKeys(obj: unknown): unknown { return Object.fromEntries( Object.entries(obj as Record).map(([k, v]) => [ toSnake(k), - toSnakeKeys(v), + OPAQUE_CONFIG_KEYS.has(k) ? v : toSnakeKeys(v), ]) ); } diff --git a/src/db/migrate.ts b/src/db/migrate.ts index ca07ae8..9d46996 100644 --- a/src/db/migrate.ts +++ b/src/db/migrate.ts @@ -230,6 +230,7 @@ export function runMigrations(db: Database.Database): void { migrateLlmUsageDaily(db); migrateLlmUsageHourly(db); migrateSpaces(db); + migrateSpaceUserPrefs(db); migrateSpaceMembers(db); migrateCalendarEvents(db); migrateSpaceInvites(db); @@ -449,6 +450,26 @@ function migrateSpaces(db: Database.Database): void { backfillBrowserSessionProfileSpaceIds(db); } +/** + * スペース表示設定（ユーザー別）。お気に入り・非表示は権限に影響しない + * 一覧整理用の状態として保存する。schema.sql と Repository.initSchema にミラーがある。 + */ +function migrateSpaceUserPrefs(db: Database.Database): void { + db.exec(` + CREATE TABLE IF NOT EXISTS space_user_prefs ( + user_id TEXT NOT NULL, + space_id TEXT NOT NULL, + favorite INTEGER NOT NULL DEFAULT 0 CHECK (favorite IN (0,1)), + hidden INTEGER NOT NULL DEFAULT 0 CHECK (hidden IN (0,1)), + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (user_id, space_id), + FOREIGN KEY (space_id) REFERENCES spaces(id) ON DELETE CASCADE + ); + CREATE INDEX IF NOT EXISTS idx_space_user_prefs_user ON space_user_prefs(user_id); + CREATE INDEX IF NOT EXISTS idx_space_user_prefs_space ON space_user_prefs(space_id); + `); +} + /** * スペース・メンバー（協働者）テーブルを追加する。追加のみ・冪等。 * owner は members 行を持たず owner_id で判定する追加協働者の表。 diff --git a/src/db/repositories/schema.ts b/src/db/repositories/schema.ts index bdeb491..e7b65c9 100644 --- a/src/db/repositories/schema.ts +++ b/src/db/repositories/schema.ts @@ -98,6 +98,20 @@ const __dirname = dirname(__filename); CREATE INDEX IF NOT EXISTS idx_spaces_visibility ON spaces(visibility, visibility_scope_org_id); CREATE UNIQUE INDEX IF NOT EXISTS idx_spaces_personal_owner ON spaces(owner_id) WHERE kind = 'personal'; `); + // スペース表示設定（ユーザー別）。schema.sql / migrate.ts と三重ミラー。 + db.exec(` + CREATE TABLE IF NOT EXISTS space_user_prefs ( + user_id TEXT NOT NULL, + space_id TEXT NOT NULL, + favorite INTEGER NOT NULL DEFAULT 0 CHECK (favorite IN (0,1)), + hidden INTEGER NOT NULL DEFAULT 0 CHECK (hidden IN (0,1)), + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (user_id, space_id), + FOREIGN KEY (space_id) REFERENCES spaces(id) ON DELETE CASCADE + ); + CREATE INDEX IF NOT EXISTS idx_space_user_prefs_user ON space_user_prefs(user_id); + CREATE INDEX IF NOT EXISTS idx_space_user_prefs_space ON space_user_prefs(space_id); + `); ensureColumn(db, 'local_tasks', 'space_id', 'TEXT'); ensureColumn(db, 'local_tasks', 'workspace_mode', "TEXT NOT NULL DEFAULT 'persistent'"); ensureColumn(db, 'jobs', 'space_id', 'TEXT'); diff --git a/src/db/repositories/spaces.ts b/src/db/repositories/spaces.ts index cd5a4b2..aba97e2 100644 --- a/src/db/repositories/spaces.ts +++ b/src/db/repositories/spaces.ts @@ -36,6 +36,13 @@ export interface Space { updatedAt: string; } +export interface SpaceDisplayPrefs { + favorite: boolean; + hidden: boolean; +} + +export interface SpaceWithDisplayPrefs extends Space, SpaceDisplayPrefs {} + export type SpaceMemberRoleValue = 'owner' | 'editor' | 'viewer'; @@ -158,6 +165,68 @@ export function rowToSpace(row: SpaceRow): Space { } + export async function listSpacesWithDisplayPrefs(db: Database.Database, opts: { viewer: Express.User; includeArchived?: boolean }): Promise { + const viewerClause = buildSpaceVisibilityWhere(opts.viewer, 's'); + const statusClause = opts.includeArchived ? '1=1' : "s.status = 'open'"; + const rows = db + .prepare(` + SELECT s.*, + COALESCE(p.favorite, 0) AS pref_favorite, + COALESCE(p.hidden, 0) AS pref_hidden + FROM spaces s + LEFT JOIN space_user_prefs p + ON p.space_id = s.id AND p.user_id = ? + WHERE ${statusClause} AND ${viewerClause.clause} + ORDER BY s.kind = 'personal' DESC, s.updated_at DESC + `) + .all(opts.viewer.id, ...viewerClause.params) as Array; + return rows.map((row) => ({ + ...rowToSpace(row), + favorite: row.pref_favorite === 1, + hidden: row.pref_hidden === 1, + })); + } + + + export function getSpaceDisplayPrefs(db: Database.Database, userId: string, spaceId: string): SpaceDisplayPrefs { + const row = db + .prepare(`SELECT favorite, hidden FROM space_user_prefs WHERE user_id = ? AND space_id = ?`) + .get(userId, spaceId) as { favorite: number; hidden: number } | undefined; + return { + favorite: row?.favorite === 1, + hidden: row?.hidden === 1, + }; + } + + + export function updateSpaceDisplayPrefs( + db: Database.Database, + userId: string, + spaceId: string, + patch: Partial, + ): SpaceDisplayPrefs { + const current = getSpaceDisplayPrefs(db, userId, spaceId); + const hidden = patch.hidden ?? current.hidden; + const favorite = hidden ? false : (patch.favorite ?? current.favorite); + db + .prepare(` + INSERT INTO space_user_prefs (user_id, space_id, favorite, hidden, updated_at) + VALUES (@userId, @spaceId, @favorite, @hidden, datetime('now')) + ON CONFLICT(user_id, space_id) DO UPDATE SET + favorite = excluded.favorite, + hidden = excluded.hidden, + updated_at = excluded.updated_at + `) + .run({ + userId, + spaceId, + favorite: favorite ? 1 : 0, + hidden: hidden ? 1 : 0, + }); + return { favorite, hidden }; + } + + export async function updateSpace(db: Database.Database, id: string, patch: { title?: string; description?: string; brandColor?: string | null; visibility?: 'private' | 'org' | 'public'; visibilityScopeOrgId?: string | null }): Promise { const sets: string[] = []; const vals: unknown[] = []; diff --git a/src/db/repository.ts b/src/db/repository.ts index e2b6ae7..26f6918 100644 --- a/src/db/repository.ts +++ b/src/db/repository.ts @@ -4,7 +4,7 @@ import * as schemaRepo from './repositories/schema.js'; import * as jobsRepo from './repositories/jobs.js'; import { JobStatus, Job, CreateJobParams, SubtaskInfo } from './repositories/jobs.js'; import * as spacesRepo from './repositories/spaces.js'; -import { Space, SpaceMemberRoleValue, SpaceMember, SpaceInviteRole, SpaceInvite, CreateSpaceParams } from './repositories/spaces.js'; +import { Space, SpaceMemberRoleValue, SpaceMember, SpaceInviteRole, SpaceInvite, CreateSpaceParams, SpaceDisplayPrefs, SpaceWithDisplayPrefs } from './repositories/spaces.js'; import * as usersRepo from './repositories/users.js'; import { User, CreateUserParams, FindOrCreateByOAuthParams, CreateLocalUserParams, LocalOrg, LocalOrgMember, GiteaOrgInput, GiteaOrg } from './repositories/users.js'; import * as workerNodesRepo from './repositories/worker-nodes.js'; @@ -42,7 +42,7 @@ export type { A2aClientRow, A2aDelegationRow } from './repositories/a2a.js'; export type { ScheduledTaskKind, ScheduledTask, CreateScheduledTaskParams, UpdateScheduledTaskParams } from './repositories/scheduled-tasks.js'; export type { WorkerNode, UpsertWorkerNodeParams } from './repositories/worker-nodes.js'; export type { User, CreateUserParams, FindOrCreateByOAuthParams, CreateLocalUserParams, LocalOrg, LocalOrgMember, GiteaOrgInput, GiteaOrg } from './repositories/users.js'; -export type { Space, SpaceMemberRoleValue, SpaceMember, SpaceInviteRole, SpaceInvite, CreateSpaceParams } from './repositories/spaces.js'; +export type { Space, SpaceMemberRoleValue, SpaceMember, SpaceInviteRole, SpaceInvite, CreateSpaceParams, SpaceDisplayPrefs, SpaceWithDisplayPrefs } from './repositories/spaces.js'; export type { JobStatus, JobRole, JobProfile, TaskClass, Job, CreateJobParams, SubtaskInfo } from './repositories/jobs.js'; export { localTaskRepoName } from './repositories/shared.js'; @@ -111,6 +111,18 @@ export class Repository { return spacesRepo.listSpaces(this.db, opts); } + async listSpacesWithDisplayPrefs(opts: { viewer: Express.User; includeArchived?: boolean }): Promise { + return spacesRepo.listSpacesWithDisplayPrefs(this.db, opts); + } + + getSpaceDisplayPrefs(userId: string, spaceId: string): SpaceDisplayPrefs { + return spacesRepo.getSpaceDisplayPrefs(this.db, userId, spaceId); + } + + updateSpaceDisplayPrefs(userId: string, spaceId: string, patch: Partial): SpaceDisplayPrefs { + return spacesRepo.updateSpaceDisplayPrefs(this.db, userId, spaceId, patch); + } + async updateSpace(id: string, patch: { title?: string; description?: string; brandColor?: string | null; visibility?: 'private' | 'org' | 'public'; visibilityScopeOrgId?: string | null }): Promise { return spacesRepo.updateSpace(this.db, id, patch); } diff --git a/src/db/schema.sql b/src/db/schema.sql index 6d0bf09..be114d3 100644 --- a/src/db/schema.sql +++ b/src/db/schema.sql @@ -112,6 +112,20 @@ CREATE INDEX IF NOT EXISTS idx_spaces_owner ON spaces(owner_id); CREATE INDEX IF NOT EXISTS idx_spaces_visibility ON spaces(visibility, visibility_scope_org_id); CREATE UNIQUE INDEX IF NOT EXISTS idx_spaces_personal_owner ON spaces(owner_id) WHERE kind = 'personal'; +-- ─── スペース表示設定（ユーザー別）──────────────────────────────── +-- お気に入り・非表示は権限や共有状態ではなく、閲覧者ごとの一覧整理だけに使う。 +CREATE TABLE IF NOT EXISTS space_user_prefs ( + user_id TEXT NOT NULL, + space_id TEXT NOT NULL, + favorite INTEGER NOT NULL DEFAULT 0 CHECK (favorite IN (0,1)), + hidden INTEGER NOT NULL DEFAULT 0 CHECK (hidden IN (0,1)), + updated_at TEXT NOT NULL DEFAULT (datetime('now')), + PRIMARY KEY (user_id, space_id), + FOREIGN KEY (space_id) REFERENCES spaces(id) ON DELETE CASCADE +); +CREATE INDEX IF NOT EXISTS idx_space_user_prefs_user ON space_user_prefs(user_id); +CREATE INDEX IF NOT EXISTS idx_space_user_prefs_space ON space_user_prefs(space_id); + -- ─── スペース・メンバー（協働者）───────────────────────────────── -- owner_id 以外の追加協働者。owner は members 行を持たず owner_id で判定する。 -- 可視性は buildVisibilityWhere(spaceColumn) の OR ブランチで membership を通す。 diff --git a/src/engine/reflection/llm-client.ts b/src/engine/reflection/llm-client.ts index 90fe694..9574ab1 100644 --- a/src/engine/reflection/llm-client.ts +++ b/src/engine/reflection/llm-client.ts @@ -19,6 +19,8 @@ export interface ReflectionLlmConfig { * client's conservative 32k default, which would block valid prompts. */ contextLimitTokens?: number; + /** Worker の extraBody（予約キー除去つきで OpenAI 互換 request body へ浅いマージ）。 */ + extraBody?: Record; } export interface ReflectionLlmResult { @@ -123,7 +125,7 @@ async function callOnce( cfg.contextLimitTokens, // real model window; avoid the 32k default blocking large reflection prompts undefined, undefined, - { proxy: cfg.proxy === true }, + { proxy: cfg.proxy === true, extraBody: cfg.extraBody }, ); const messages: Message[] = [ { role: 'system', content: systemPrompt }, diff --git a/src/engine/reflection/reflection-runner.ts b/src/engine/reflection/reflection-runner.ts index 948ebc0..35cae18 100644 --- a/src/engine/reflection/reflection-runner.ts +++ b/src/engine/reflection/reflection-runner.ts @@ -29,6 +29,8 @@ export interface RunReflectionDeps { llmProxy?: boolean; /** Reflection worker's model context window (tokens) for the prompt guard. */ llmContextLimitTokens?: number; + /** Reflection worker's extraBody（同じ worker def から渡す浅いマージ用 JSON）。 */ + llmExtraBody?: Record; } export async function runReflectionJob( @@ -118,6 +120,7 @@ export async function runReflectionJob( proxy: deps.llmProxy === true, userId: meta.userId, contextLimitTokens: deps.llmContextLimitTokens, + extraBody: deps.llmExtraBody, }; let llmResult; diff --git a/src/llm/openai-compat.extra-body.test.ts b/src/llm/openai-compat.extra-body.test.ts new file mode 100644 index 0000000..ed0b8d1 --- /dev/null +++ b/src/llm/openai-compat.extra-body.test.ts @@ -0,0 +1,119 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { OpenAICompatClient, type Message } from './openai-compat.js'; + +// fetch モックで送信 body をキャプチャする（openai-compat.test.ts と同じパターン） +function makeSseResponse(chunks: Array | '[DONE]'>): Response { + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + for (const chunk of chunks) { + const data = chunk === '[DONE]' ? chunk : JSON.stringify(chunk); + controller.enqueue(encoder.encode(`data: ${data}\n\n`)); + } + controller.close(); + }, + }); + + return new Response(stream, { + status: 200, + headers: { 'Content-Type': 'text/event-stream' }, + }); +} + +async function drain(client: OpenAICompatClient, messages: Message[]): Promise { + for await (const _event of client.chat(messages)) { + // 送信 body のキャプチャが目的。イベント内容は他テストで検証済み。 + } +} + +describe('OpenAICompatClient extraBody', () => { + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + }); + + it('extraBody の非予約キーがリクエスト body にマージされる', async () => { + const fetchMock = vi.fn().mockResolvedValue( + makeSseResponse([{ choices: [{ delta: { content: 'ok' } }] }, '[DONE]']), + ); + vi.stubGlobal('fetch', fetchMock); + + const client = new OpenAICompatClient( + 'http://llm.test/v1', + 'test-model', + undefined, + { maxAttempts: 1, backoffMs: [0], retryableStatus: [] }, + undefined, + undefined, + undefined, + undefined, + { extraBody: { reasoning_effort: 'max', top_k: 20 } }, + ); + + await drain(client, [{ role: 'user', content: 'hi' }]); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const requestInit = fetchMock.mock.calls[0][1] as RequestInit; + const body = JSON.parse(requestInit.body as string); + expect(body.reasoning_effort).toBe('max'); + expect(body.top_k).toBe(20); + }); + + it('extraBody の予約キーは除去され、client 側の値が勝つ', async () => { + const fetchMock = vi.fn().mockResolvedValue( + makeSseResponse([{ choices: [{ delta: { content: 'ok' } }] }, '[DONE]']), + ); + vi.stubGlobal('fetch', fetchMock); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const client = new OpenAICompatClient( + 'http://llm.test/v1', + 'test-model', + undefined, + { maxAttempts: 1, backoffMs: [0], retryableStatus: [] }, + undefined, + undefined, + undefined, + undefined, + { + extraBody: { + model: 'evil', + messages: [], + stream: false, + reasoning_effort: 'max', + }, + }, + ); + + await drain(client, [{ role: 'user', content: 'hi' }]); + + const requestInit = fetchMock.mock.calls[0][1] as RequestInit; + const body = JSON.parse(requestInit.body as string); + expect(body.model).toBe('test-model'); + expect(body.messages).toEqual([{ role: 'user', content: 'hi' }]); + expect(body.stream).toBe(true); + expect(body.reasoning_effort).toBe('max'); + + warnSpy.mockRestore(); + }); + + it('extraBody 未指定なら body のキー集合は従来どおり（回帰なし）', async () => { + const fetchMock = vi.fn().mockResolvedValue( + makeSseResponse([{ choices: [{ delta: { content: 'ok' } }] }, '[DONE]']), + ); + vi.stubGlobal('fetch', fetchMock); + + const client = new OpenAICompatClient( + 'http://llm.test/v1', + 'test-model', + undefined, + { maxAttempts: 1, backoffMs: [0], retryableStatus: [] }, + ); + + await drain(client, [{ role: 'user', content: 'hi' }]); + + const requestInit = fetchMock.mock.calls[0][1] as RequestInit; + const body = JSON.parse(requestInit.body as string); + expect(Object.keys(body).sort()).toEqual(['messages', 'model', 'stream', 'stream_options'].sort()); + }); +}); diff --git a/src/llm/openai-compat.ts b/src/llm/openai-compat.ts index 5f8a1b5..80d50d1 100644 --- a/src/llm/openai-compat.ts +++ b/src/llm/openai-compat.ts @@ -337,6 +337,32 @@ export interface OpenAICompatClientOptions { * unknown body fields, so this must never default to on. */ requestPromptProgress?: boolean; + /** + * Arbitrary extra fields to shallow-merge into the request body (e.g. + * `reasoning_effort` for vLLM-hosted deepseek). Reserved keys that the + * client itself constructs (model/messages/stream/stream_options/tools/ + * tool_choice/temperature) are stripped — the client's own values always + * win. Sanitized once in the constructor. + */ + extraBody?: Record; +} + +/** extra_body から除去する予約キー（client が組み立てる固定フィールド） */ +const RESERVED_BODY_KEYS = new Set([ + 'model', 'messages', 'stream', 'stream_options', 'tools', 'tool_choice', 'temperature', +]); + +function sanitizeExtraBody(extra: Record | undefined): Record { + if (!extra) return {}; + const out: Record = {}; + for (const [k, v] of Object.entries(extra)) { + if (RESERVED_BODY_KEYS.has(k)) { + logger.warn(`OpenAICompatClient: extra_body の予約キー '${k}' を無視しました`); + continue; + } + out[k] = v; + } + return out; } /** @@ -385,9 +411,11 @@ export class OpenAICompatClient { this.maxStreamMs = options?.maxStreamMs ?? this.timeoutMs * 2; this.proxy = options?.proxy === true; this.requestPromptProgress = options?.requestPromptProgress === true; + this.extraBody = sanitizeExtraBody(options?.extraBody); } private readonly requestPromptProgress: boolean; + private readonly extraBody: Record; private buildAbortErrorMessage(externalSignal?: AbortSignal, hardCapHit = false): string { if (externalSignal?.aborted) { @@ -511,6 +539,7 @@ export class OpenAICompatClient { } const body: Record = { + ...this.extraBody, messages, stream: true, stream_options: { include_usage: true }, diff --git a/src/progress/delegate-runs.test.ts b/src/progress/delegate-runs.test.ts index aa92c72..7ee4e41 100644 --- a/src/progress/delegate-runs.test.ts +++ b/src/progress/delegate-runs.test.ts @@ -195,6 +195,38 @@ describe('reconstructDelegateRuns', () => { expect(runs[0].filesChanged).toEqual([]); }); + it('lastErrorTool: tool_result で isError:true が2件連続すると最後の tool 名が残る', () => { + const events = [ + ev({ seq: 1, kind: 'delegate_start', payload: { delegateRunId: 'R14', parentRunId: null, description: 'test', depth: 1 } }), + ev({ seq: 2, kind: 'tool_result', correlationId: 'R14', payload: { tool: 'WebFetch', isError: true } }), + ev({ seq: 3, kind: 'tool_result', correlationId: 'R14', payload: { tool: 'Bash', isError: true } }), + ev({ seq: 4, kind: 'delegate_complete', payload: { delegateRunId: 'R14', parentRunId: null, description: 'test', depth: 1, next: 'ABORT', status: 'aborted' } }), + ]; + const runs = reconstructDelegateRuns(events); + expect(runs[0].lastErrorTool).toBe('Bash'); + }); + + it('lastErrorTool: isError な tool_result が無ければ null のまま', () => { + const events = [ + ev({ seq: 1, kind: 'delegate_start', payload: { delegateRunId: 'R15', parentRunId: null, description: 'test', depth: 1 } }), + ev({ seq: 2, kind: 'tool_result', correlationId: 'R15', payload: { tool: 'WebFetch', isError: false } }), + ev({ seq: 3, kind: 'delegate_complete', payload: { delegateRunId: 'R15', parentRunId: null, description: 'test', depth: 1, next: 'ABORT', status: 'aborted' } }), + ]; + const runs = reconstructDelegateRuns(events); + expect(runs[0].lastErrorTool).toBeNull(); + }); + + it('lastErrorTool: success run でも isError な tool_result があれば値が入る', () => { + const events = [ + ev({ seq: 1, kind: 'delegate_start', payload: { delegateRunId: 'R16', parentRunId: null, description: 'test', depth: 1 } }), + ev({ seq: 2, kind: 'tool_result', correlationId: 'R16', payload: { tool: 'WebFetch', isError: true } }), + ev({ seq: 3, kind: 'tool_result', correlationId: 'R16', payload: { tool: 'WebFetch', isError: false } }), + ev({ seq: 4, kind: 'delegate_complete', payload: { delegateRunId: 'R16', parentRunId: null, description: 'test', depth: 1, next: 'COMPLETE', status: 'success' } }), + ]; + const runs = reconstructDelegateRuns(events); + expect(runs[0].lastErrorTool).toBe('WebFetch'); + }); + it('tool_call に args がなくてもクラッシュしない', () => { const events = [ ev({ seq: 1, kind: 'delegate_start', payload: { delegateRunId: 'R13', parentRunId: null, description: 'test', depth: 1 } }), diff --git a/src/progress/delegate-runs.ts b/src/progress/delegate-runs.ts index c5a599f..6ea28e3 100644 --- a/src/progress/delegate-runs.ts +++ b/src/progress/delegate-runs.ts @@ -19,6 +19,7 @@ export interface DelegateRun { totalTokens: number; toolSummary: Array<{ tool: string; count: number }>; filesChanged: string[]; + lastErrorTool: string | null; } function payloadOf(e: EventBase): Record { @@ -50,6 +51,7 @@ export function reconstructDelegateRuns(events: EventBase[]): DelegateRun[] { totalTokens: 0, toolSummary: [], filesChanged: [], + lastErrorTool: null, }); order.push(id); } @@ -99,6 +101,14 @@ export function reconstructDelegateRuns(events: EventBase[]): DelegateRun[] { } } } + if (e.kind === 'tool_result') { + const p = payloadOf(e); + const tool = typeof p.tool === 'string' ? p.tool : null; + if (tool && p.isError) { + // seq 昇順で走査しているため、最後に代入された値が「最後に失敗したツール」になる + run.lastErrorTool = tool; + } + } if (e.kind === 'llm_call_end') { const p = payloadOf(e); const promptTokens = typeof p.promptTokens === 'number' ? p.promptTokens : 0; diff --git a/src/scripts/migrate-config.test.ts b/src/scripts/migrate-config.test.ts index e7777c3..ba68561 100644 --- a/src/scripts/migrate-config.test.ts +++ b/src/scripts/migrate-config.test.ts @@ -7,6 +7,7 @@ * care which runtime ran, only that the output is correct. */ import { afterEach, describe, expect, it } from 'vitest'; +import { transformCamel } from './migrate-config.js'; import { spawnSync } from 'child_process'; import { copyFileSync, existsSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'fs'; import { tmpdir } from 'os'; @@ -125,3 +126,20 @@ describe('migrate-config CLI', () => { expect(r.stderr).toMatch(/config_version=99/); }); }); + +describe('transformCamel', () => { + it('extra_body の中身のキーは camelCase 変換せず素通しする（src/config.ts transformKeys と同じ不変条件）', () => { + const input = { + provider: { + workers: [ + { id: 'w1', extra_body: { reasoning_effort: 'max', chat_template_kwargs: { enable_thinking: true } } }, + ], + }, + }; + const out = transformCamel(input) as any; + expect(out.provider.workers[0].extraBody).toEqual({ + reasoning_effort: 'max', + chat_template_kwargs: { enable_thinking: true }, + }); + }); +}); diff --git a/src/scripts/migrate-config.ts b/src/scripts/migrate-config.ts index 845cf8f..e2472cc 100644 --- a/src/scripts/migrate-config.ts +++ b/src/scripts/migrate-config.ts @@ -27,9 +27,10 @@ */ import { existsSync, readFileSync, writeFileSync, copyFileSync } from 'fs'; import { resolve } from 'path'; +import { pathToFileURL } from 'url'; import { parse as parseYaml, stringify as stringifyYaml } from 'yaml'; import { normalizeConfig, UnsupportedConfigVersionError } from '../config-normalize.js'; -import { toSnakeKeys } from '../config.js'; +import { toSnakeKeys, OPAQUE_CONFIG_KEYS } from '../config.js'; interface Cli { dryRun: boolean; @@ -82,15 +83,21 @@ function printHelp(): void { ].join('\n')); } -/** snake_case → camelCase recursive (matches src/config.ts transformKeys). */ +/** snake_case → camelCase recursive (matches src/config.ts transformKeys). + * extra_body 等の不透明キー（OPAQUE_CONFIG_KEYS）の値はプロバイダーへ素通しする + * JSON なので、src/config.ts と同様にキー変換せずそのまま通す。 + * Exported for tests. */ function toCamel(s: string): string { return s.replace(/_([a-z])/g, (_, c: string) => c.toUpperCase()); } -function transformCamel(obj: unknown): unknown { +export function transformCamel(obj: unknown): unknown { if (Array.isArray(obj)) return obj.map(transformCamel); if (obj !== null && typeof obj === 'object') { return Object.fromEntries( - Object.entries(obj as Record).map(([k, v]) => [toCamel(k), transformCamel(v)]), + Object.entries(obj as Record).map(([k, v]) => [ + toCamel(k), + OPAQUE_CONFIG_KEYS.has(k) ? v : transformCamel(v), + ]), ); } return obj; @@ -286,5 +293,13 @@ function main(argv: string[]): number { return 0; } -const code = main(process.argv.slice(2)); -process.exit(code); +// 直接実行されたときだけ CLI を走らせる（node dist/... / tsx src/... の両経路とも +// migrate-config.sh は絶対パスで invoke する）。テストからの import で main / +// process.exit が発火しないようにするためのガード。 +const invokedAsScript = + process.argv[1] != null && + import.meta.url === pathToFileURL(resolve(process.argv[1])).href; +if (invokedAsScript) { + const code = main(process.argv.slice(2)); + process.exit(code); +} diff --git a/src/worker-bootstrap.ts b/src/worker-bootstrap.ts index ce86fdd..b3e99c4 100644 --- a/src/worker-bootstrap.ts +++ b/src/worker-bootstrap.ts @@ -190,7 +190,11 @@ export async function start(opts: StartWorkerOptions = {}): Promise { undefined, // proxy mode so title/classification usage is recorded as // source='gateway' with the backendId route (not mislabeled 'direct'). - { proxy: titleWorker.proxy === true, maxStreamMs: resolveMaxStreamMs(config.provider) }, + { + proxy: titleWorker.proxy === true, + maxStreamMs: resolveMaxStreamMs(config.provider), + extraBody: titleWorker.extraBody, + }, ); generateTitle = async (body: string, ownerId?: string): Promise => { diff --git a/src/worker.extra-body.test.ts b/src/worker.extra-body.test.ts new file mode 100644 index 0000000..ed8b171 --- /dev/null +++ b/src/worker.extra-body.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; +import type { AppConfig } from './config.js'; +import type { PieceDef } from './engine/piece-runner.js'; +import type { Repository, Job } from './db/repository.js'; + +// createJobLlmClient (private) is the main job-execution client construction +// site. Spy on the OpenAICompatClient constructor to verify the worker def's +// extraBody reaches the options object passed to it. +const constructorSpy = vi.fn(); + +vi.mock('./llm/openai-compat.js', async (importOriginal) => { + const actual = await importOriginal(); + class SpyOpenAICompatClient extends actual.OpenAICompatClient { + constructor(...args: ConstructorParameters) { + constructorSpy(...args); + super(...args); + } + } + return { ...actual, OpenAICompatClient: SpyOpenAICompatClient }; +}); + +describe('Worker LLM client construction — extraBody propagation', () => { + beforeEach(() => { + constructorSpy.mockClear(); + }); + + it('createJobLlmClient passes the worker def extraBody through to OpenAICompatClient options', async () => { + const { Worker } = await import('./worker.js'); + const { LocalProgressReporter } = await import('./progress/local-reporter.js'); + + const extraBody = { reasoning_effort: 'max', top_k: 20 }; + + const config: AppConfig = { + provider: { + model: 'test-model', + workers: [ + { + id: 'worker-1', + endpoint: 'http://localhost:11434/v1', + extraBody, + }, + ], + }, + worktreeDir: '/tmp/worker-extra-body-test', + concurrency: 1, + maxMovements: 30, + retry: { maxAttempts: 3, backoffSeconds: [60, 300, 900] }, + ask: { maxPerJob: 2 }, + subtasks: { maxDepth: 2 }, + tools: { + searxngUrl: 'http://localhost:8080', + visionModel: 'vision', + visionTimeout: 60, + visionMaxTokens: 1024, + webfetchTimeout: 30, + websearchTimeout: 15, + webfetchAllowedHosts: [], + }, + } as AppConfig; + + const repo = {} as Repository; + const worker = new Worker('worker-1', 'http://localhost:11434/v1', 'test-model', repo, config); + + const job = { id: 'job-1', requiredRole: 'auto' } as unknown as Job; + const piece = {} as PieceDef; + const reporter = {} as InstanceType; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (worker as any).createJobLlmClient(job, piece, reporter); + + expect(constructorSpy).toHaveBeenCalledTimes(1); + const options = constructorSpy.mock.calls[0]?.[8]; + expect(options).toMatchObject({ extraBody }); + }); + + it('leaves options.extraBody undefined when the worker def has none (no behavior change)', async () => { + const { Worker } = await import('./worker.js'); + + const config: AppConfig = { + provider: { + model: 'test-model', + workers: [{ id: 'worker-1', endpoint: 'http://localhost:11434/v1' }], + }, + worktreeDir: '/tmp/worker-extra-body-test', + concurrency: 1, + maxMovements: 30, + retry: { maxAttempts: 3, backoffSeconds: [60, 300, 900] }, + ask: { maxPerJob: 2 }, + subtasks: { maxDepth: 2 }, + tools: { + searxngUrl: 'http://localhost:8080', + visionModel: 'vision', + visionTimeout: 60, + visionMaxTokens: 1024, + webfetchTimeout: 30, + websearchTimeout: 15, + webfetchAllowedHosts: [], + }, + } as AppConfig; + + const repo = {} as Repository; + const worker = new Worker('worker-1', 'http://localhost:11434/v1', 'test-model', repo, config); + + const job = { id: 'job-1', requiredRole: 'auto' } as unknown as Job; + const piece = {} as PieceDef; + const reporter = {} as unknown; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (worker as any).createJobLlmClient(job, piece, reporter); + + const options = constructorSpy.mock.calls[0]?.[8]; + expect(options?.extraBody).toBeUndefined(); + }); +}); diff --git a/src/worker.ts b/src/worker.ts index b04ac7f..2e6965d 100644 --- a/src/worker.ts +++ b/src/worker.ts @@ -1034,7 +1034,11 @@ export class Worker { this.contextLimitTokens, this.config.safety?.promptGuardRatio, undefined, - { proxy: workerDefForAnswer.proxy === true, maxStreamMs: this.resolveMaxStreamMs() }, + { + proxy: workerDefForAnswer.proxy === true, + maxStreamMs: this.resolveMaxStreamMs(), + extraBody: workerDefForAnswer.extraBody, + }, ); const messages: import('./llm/openai-compat.js').Message[] = [ @@ -1431,6 +1435,7 @@ export class Worker { proxy: isProxyWorker, maxStreamMs: this.resolveMaxStreamMs(), requestPromptProgress: workerDefForLlm.returnProgress === true, + extraBody: workerDefForLlm.extraBody, }, ); return { llmClient, isProxyWorker }; @@ -2798,6 +2803,7 @@ export class Worker { llmApiKey: this.getWorkerDef().apiKey, llmProxy: this.getWorkerDef().proxy === true, llmContextLimitTokens: this.contextLimitTokens, + llmExtraBody: this.getWorkerDef().extraBody, }, job ); diff --git a/ui/e2e/calendar.spec.ts b/ui/e2e/calendar.spec.ts index c59855b..4eb23ab 100644 --- a/ui/e2e/calendar.spec.ts +++ b/ui/e2e/calendar.spec.ts @@ -114,6 +114,11 @@ test('cross calendar: top-bar tab opens, grid renders, month nav works', async ( test('cross calendar: a space with activity shows its dot + day panel groups by space', async ({ page }) => { const fatalErrors = trackFatalErrors(page); const { caseTitle, spaceId } = await createAndOpenCaseSpace(page, 'E2E横断カレンダー'); + + // The cross-cal dot reflects a space's activity for the day — task or + // event (see issue #804: it used to be task-only, leaving single-day + // events with no cross-calendar indicator). This space has an event and + // no task, so the dot must come from the event alone. const eventTitle = `横断予定-${Date.now()}`; await addTodayEvent(page, eventTitle); @@ -123,10 +128,12 @@ test('cross calendar: a space with activity shows its dot + day panel groups by const today = e2eLocalToday(); const todayCell = page.getByTestId(`cross-cal-day-${today}`); - // Today's cell carries the activity dot for our space. + // Today's cell carries the activity dot for our space (event-gated only). await expect(todayCell.getByTestId(`cross-cal-dot-${spaceId}`)).toBeVisible({ timeout: 10_000 }); - // Click the day → panel grouped by space, with our space heading + its event. + // Click the day → panel grouped by space, with our space heading and its + // event (a space qualifies for the panel with either task or event + // activity — see CrossSpaceCalendar.tsx's activeSpaces filter). await todayCell.click(); const panel = page.getByTestId('cross-cal-day-panel'); await expect(panel).toBeVisible(); diff --git a/ui/e2e/pieces-editor.spec.ts b/ui/e2e/pieces-editor.spec.ts index 0d2cdd8..0916cb7 100644 --- a/ui/e2e/pieces-editor.spec.ts +++ b/ui/e2e/pieces-editor.spec.ts @@ -69,8 +69,10 @@ test('create a custom piece from the sidebar, then edit + save it via the editor await nameInput.fill(pieceName); await nameInput.press('Enter'); - // The new custom piece appears in the sidebar list. - await expect(page.getByText(pieceName, { exact: false })).toBeVisible({ timeout: 15_000 }); + // The new custom piece appears in the sidebar list AND is auto-selected, so + // its name also shows in the editor header (PieceEditor.tsx's

) — + // scope to the sidebar row button to avoid a strict-mode double match. + await expect(page.getByRole('button', { name: pieceName })).toBeVisible({ timeout: 15_000 }); // It was persisted as a custom piece (the endpoint the page reads returns // { pieces: [...] } with custom:true on user pieces). @@ -88,6 +90,10 @@ test('create a custom piece from the sidebar, then edit + save it via the editor await page.getByRole('button', { name: 'YAML' }).click(); const yaml = page.locator('textarea').first(); await expect(yaml).toBeVisible({ timeout: 15_000 }); + // Terminal moves go through the `complete` tool, not rules[].next (which + // only accepts other movement names / WAIT_SUBTASKS — see + // docs/movement-control-flow-tools.md). Tool/edit access is a workspace tool + // policy setting now, not a piece field (allowed_tools/edit are removed). const edited = [ `name: ${pieceName}`, 'description: edited by e2e', @@ -95,14 +101,10 @@ test('create a custom piece from the sidebar, then edit + save it via the editor 'initial_movement: execute', 'movements:', ' - name: execute', - ' edit: true', ' persona: worker', " instruction: 'do the thing'", - ' allowed_tools: [Read, Write, Edit]', ' default_next: COMPLETE', - ' rules:', - " - condition: '完了'", - ' next: COMPLETE', + ' rules: []', '', ].join('\n'); await yaml.fill(edited); @@ -125,10 +127,15 @@ test('create a custom piece from the sidebar, then edit + save it via the editor expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); -// Negative/visibility: built-in pieces are not deletable and the editor renders -// them read-only (no Save/Delete footer). 'chat' is a built-in piece always -// present. Selecting it shows the read-only badge instead of the editable footer. -test('a built-in piece opens read-only (no Save, no Delete)', async ({ page }) => { +// Negative/visibility: built-in pieces are never deletable, by anyone +// (PieceEditor.tsx: Delete is gated on effectiveSource !== 'builtin', with no +// admin exception). The readonly badge/footer gate, by contrast, is +// `!isAdmin && (builtin || global-custom)` — and with auth disabled the E2E +// synthetic user is always admin (App.tsx: isAdmin = auth.mode === +// 'authenticated' ? user.role === 'admin' : true), so admins CAN edit +// (Save) built-in pieces here; only non-admins get the true read-only view. +// 'chat' is a built-in piece always present. +test('a built-in piece opens editable for admin (no Delete, Save appears once dirty)', async ({ page }) => { const fatalErrors = trackFatalErrors(page); await page.goto('/ui?page=pieces'); @@ -137,11 +144,13 @@ test('a built-in piece opens read-only (no Save, no Delete)', async ({ page }) = // Open the built-in 'chat' piece from the Default Pieces section. await page.getByRole('button', { name: /chat/i }).first().click(); - // The editor marks it read-only (PieceEditor renders the readonly badge and - // omits the editable footer's Save button for built-ins). - await expect(page.getByText(/read-?only/i).first()).toBeVisible({ timeout: 15_000 }); - await expect(page.getByRole('button', { name: 'Save' })).toHaveCount(0); + // No readonly badge (this session is admin) and no Delete (built-ins are + // never deletable), but the editable footer with Save is present. + const saveBtn = page.getByRole('button', { name: 'Save' }); + await expect(saveBtn).toBeVisible({ timeout: 15_000 }); + await expect(page.getByText(/read-?only/i)).toHaveCount(0); await expect(page.getByRole('button', { name: 'Delete' })).toHaveCount(0); + await expect(saveBtn).toBeDisabled(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); diff --git a/ui/e2e/spaces.spec.ts b/ui/e2e/spaces.spec.ts index 7654eb6..390eb46 100644 --- a/ui/e2e/spaces.spec.ts +++ b/ui/e2e/spaces.spec.ts @@ -217,10 +217,13 @@ test('spaces foundation: rail, create case space, open detail tabs', async ({ pa }); // Mobile (=md both are visible together -// (unchanged), so this invariant only holds at narrow widths. +// phone. Startup auto-opens the personal workspace (App.tsx's one-shot +// personalSpaceAutoOpened guard), so detail is the initial full-width view; +// backing out returns to the full-width rail (the guard does not re-fire) and +// selecting another space replaces the rail with detail plus a +// "← ワークスペース一覧" back control that returns to the rail. At >=md both +// are visible together (unchanged), so this invariant only holds at narrow +// widths. test('spaces mobile: rail <-> detail switch with back control at 390px', async ({ page }) => { await page.setViewportSize({ width: 390, height: 844 }); @@ -229,9 +232,18 @@ test('spaces mobile: rail <-> detail switch with back control at 390px', async ( // URL state (page=spaces) instead — it's the same selection, sans drawer. await page.goto('/ui?page=spaces'); - // No space selected: the rail is the visible full-width list; detail hidden. + // Startup auto-opens the personal workspace: detail is the visible + // full-width view, rail is hidden, and the back control is already present. const rail = page.getByTestId('space-rail'); const detail = page.getByTestId('space-detail'); + const back = page.getByTestId('space-back-to-list'); + await expect(detail).toBeVisible(); + await expect(rail).toBeHidden(); + await expect(back).toBeVisible(); + + // Tapping back clears the selection and returns to the full-width rail. The + // auto-open guard is one-shot, so it does not immediately re-select. + await back.click(); await expect(rail).toBeVisible(); await expect(detail).toBeHidden(); await expect(page.getByTestId('space-back-to-list')).toHaveCount(0); @@ -250,7 +262,6 @@ test('spaces mobile: rail <-> detail switch with back control at 390px', async ( await expect(detail).toBeVisible(); await expect(detail.getByText(caseTitle)).toBeVisible(); await expect(rail).toBeHidden(); - const back = page.getByTestId('space-back-to-list'); await expect(back).toBeVisible(); // Tapping back clears the selection and returns to the full-width rail. @@ -331,10 +342,10 @@ test('space inline chat: create + open conversation without leaving for Tasks', // (urlState serializes page=spaces) and sets the chat= param for the // selected inline chat. It must NOT switch to page=tasks (which would // drop page= entirely AND have no chat=). - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); await expect(page).toHaveURL(/[?&]chat=\d+/); expect(page.url()).toContain('chat='); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); // The Spaces detail (not the Tasks board) is still mounted. await expect(detail).toBeVisible(); @@ -353,9 +364,8 @@ test('space inline chat: create + open conversation without leaving for Tasks', // No overlay close affordance — the redesign dropped the overlay entirely. await expect(page.getByTestId('detail-panel-close')).toHaveCount(0); // Invariant: still inline on Spaces, same chat, never bounced to Tasks. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); await expect(page).toHaveURL(/[?&]chat=\d+/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); // Return to the conversation via the 会話 tab so the rest of the flow // (creating a second chat) starts from chat again. await conversation.getByTestId('space-chat-tab-chat').click(); @@ -390,8 +400,7 @@ test('space inline chat: create + open conversation without leaving for Tasks', expect(secondChatId).not.toBe(firstChatId); await expect(conversation).toBeVisible(); // Still on Spaces, still has a chat — never bounced to Tasks. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -497,8 +506,7 @@ test('space files: icon grid, upload appears as tile, click opens preview modal' await expect(dialog).toHaveCount(0); // 6. No-nav invariant: file management never leaves Spaces for the Tasks board. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); await expect(detail).toBeVisible(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); @@ -679,9 +687,8 @@ test('space output link: chat output-path link opens preview modal', async ({ pa await expect(dialog).toHaveCount(0); // 7. No-nav invariant: clicking the link previewed in place, never bounced to Tasks. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); await expect(page).toHaveURL(/[?&]chat=\d+/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); await expect(detail).toBeVisible(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); @@ -689,12 +696,17 @@ test('space output link: chat output-path link opens preview modal', async ({ pa // Redesigned space-chat UX (feat/space-chat-ux). The inline conversation now // carries a SINGLE in-place tab bar (data-testid="space-chat-tabs", role=tablist) -// with 会話 (chat) + 概要 (overview) + 進捗 (activity) + トレース (trace) and NO -// files tab (files live at the space level). Clicking a non-chat tab swaps the -// content IN PLACE — no overlay, no ✕ close button — and 会話 returns to the -// conversation. Throughout, we never leave for the Tasks page (page=spaces + -// chat= stay put). No LLM is configured in E2E, so we assert UI/structure only, -// never agent output. +// with 会話 (chat) + 概要 (overview) + 進捗 (activity) + ファイル (files) + +// トレース (trace). This chat-level ファイル tab is distinct from the +// space-level files (SpaceDetail.tsx comment: it shows the chat's own +// input/output/logs under runs/{taskId}, not the shared space deliverables). +// On desktop (md+) SpaceDetail renders a ChatDetailSplit: 会話 (with its +// composer) is ALWAYS mounted on the left and never unmounts; a non-chat tab +// opens the selected detail panel in a right pane alongside it (no overlay, +// no ✕ close button) — clicking 会話 just collapses the right pane back to +// full-width chat, it doesn't remount the conversation. Throughout, we never +// leave for the Tasks page (space= + chat= stay put; no page= param). No LLM +// is configured in E2E, so we assert UI/structure only, never agent output. test('space chat tabs: single in-place tab bar, overview in place, back to chat', async ({ page }) => { const fatalErrors: string[] = []; const benignAsset = /\.(ico|png|svg|map|webmanifest|json)$/i; @@ -755,15 +767,16 @@ test('space chat tabs: single in-place tab bar, overview in place, back to chat' const overviewTab = page.getByTestId('space-chat-tab-overview'); await expect(chatTab).toBeVisible(); await expect(overviewTab).toBeVisible(); - // No files tab inside this in-place bar (files are a space-level concern). - await expect(tabs.getByTestId('space-chat-tab-files')).toHaveCount(0); + // The chat-level ファイル tab is present too (per-chat I/O/logs, distinct + // from the space-level files tab). + await expect(tabs.getByTestId('space-chat-tab-files')).toBeVisible(); // 会話 is the default-selected tab; the message composer (textarea) is present. await expect(chatTab).toHaveAttribute('aria-selected', 'true'); await expect(conversation.locator('textarea')).toBeVisible(); - // 3. Click 概要 → its content renders IN PLACE. There is NO overlay and NO ✕ - // close button (headerless DetailPanel ⇒ no detail-panel-close testid), and - // the overview tab becomes aria-selected. The conversation composer is gone. + // 3. Click 概要 → it opens as a right-pane detail panel. There is NO overlay + // and NO ✕ close button (headerless DetailPanel ⇒ no detail-panel-close + // testid), and the overview tab becomes aria-selected. await overviewTab.click(); await expect(overviewTab).toHaveAttribute('aria-selected', 'true'); await expect(chatTab).toHaveAttribute('aria-selected', 'false'); @@ -772,20 +785,22 @@ test('space chat tabs: single in-place tab bar, overview in place, back to chat' await expect(conversation.getByRole('tabpanel')).toBeVisible(); // No overlay close affordance anywhere on the page. await expect(page.getByTestId('detail-panel-close')).toHaveCount(0); - // Composer is swapped out (overview content replaced the conversation in place). - await expect(conversation.locator('textarea')).toHaveCount(0); + // The chat composer stays mounted (ChatDetailSplit's left pane never + // unmounts on desktop — see the top-of-file comment) — exactly one + // textarea, not the zero a full replacement would produce. + await expect(conversation.locator('textarea')).toHaveCount(1); - // 4. Click 会話 → back to the conversation; composer returns, chat selected. + // 4. Click 会話 → the right pane collapses back to full-width chat. await chatTab.click(); await expect(chatTab).toHaveAttribute('aria-selected', 'true'); await expect(overviewTab).toHaveAttribute('aria-selected', 'false'); await expect(conversation.locator('textarea')).toBeVisible(); // 5. No navigation to Tasks throughout: page=spaces + chat= held the entire time. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); await expect(page).toHaveURL(/[?&]chat=\d+/); expect(page.url()).toContain('chat='); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); await expect(detail).toBeVisible(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); @@ -932,8 +947,7 @@ test('space settings tab: AGENTS.md per-space persistence + MCP/SSH panels rende await expect(page.getByTestId('space-ssh-panel')).toBeVisible(); // 6. No-nav invariant: settings is in-place, never leaves Spaces for Tasks. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); await expect(detail).toBeVisible(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); @@ -1094,8 +1108,7 @@ test('space MCP isolation: server registered in A is visible in A, absent in B', await expect(page.getByTestId('space-mcp-panel')).not.toContainText(mcpName); // No-nav invariant + no fatal client errors. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1153,8 +1166,7 @@ test('space chat DELETE: toolbar delete removes the chat and returns to the list await expect.poll(() => new URL(page.url()).searchParams.get('chat')).toBeFalsy(); // No-nav invariant: deletion stayed on Spaces, never bounced to Tasks. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1197,8 +1209,7 @@ test('space chat CANCEL: ChatPane stop button cancels a running task', async ({ const status = await waitForTerminalJob(page, taskId, 20_000); expect(status).toBe('cancelled'); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1242,8 +1253,7 @@ test('space chat SHARE: toolbar share publishes a share token', async ({ page, c timeout: 15_000, }); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1287,8 +1297,7 @@ test('space chat CONTINUE: button gated on terminal job; opens ContinueWithPiece await expect(page.locator('#continue-instruction')).toBeVisible(); await expect.poll(async () => pieceSelect.locator('option').count()).toBeGreaterThan(0); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1325,7 +1334,7 @@ test('space chat VISIBILITY: no visibility control (fixed member-only scope)', a // tests in this file still assert it and are stale — see issue #782 follow-up). await expect(page).toHaveURL(/[?&]space=[^&]+/); await expect(page).toHaveURL(/[?&]chat=\d+/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1345,15 +1354,17 @@ test('space chat FEEDBACK: 概要 tab renders OverviewTab; rating round-trip sav 'E2E: フィードバックテスト用チャット。最初のメッセージ。', ); - // Always-verifiable: switch to 概要 → OverviewTab renders in place. The - // composer (textarea) is swapped out and the shared tabpanel is mounted. + // Always-verifiable: switch to 概要 → OverviewTab opens as a right-pane + // detail panel and the shared tabpanel is mounted. const overviewTab = conversation.getByTestId('space-chat-tab-overview'); await expect(overviewTab).toBeVisible(); await overviewTab.click(); await expect(overviewTab).toHaveAttribute('aria-selected', 'true'); const panel = conversation.getByRole('tabpanel'); await expect(panel).toBeVisible(); - await expect(conversation.locator('textarea')).toHaveCount(0); + // The chat composer stays mounted (ChatDetailSplit's left pane never + // unmounts on desktop) — exactly one textarea. + await expect(conversation.locator('textarea')).toHaveCount(1); // Active path: the FeedbackPanel only renders once the job is succeeded/failed. const status = await waitForTerminalJob(page, taskId); @@ -1397,8 +1408,7 @@ test('space chat FEEDBACK: 概要 tab renders OverviewTab; rating round-trip sav conversationAfter.getByRole('tabpanel').getByRole('button', { name: /^(Change|変更)$/ }), ).toBeVisible({ timeout: 15_000 }); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1421,14 +1431,14 @@ test('space settings sub-tabs: agents/memory/pieces/skills/mcp/ssh all render th // memory → the MemoryEntriesPanel header + new-entry button render. The header // text is i18n (English in the E2E browser locale, Japanese otherwise). await page.getByTestId('space-settings-nav-memory').click(); - await expect(page.getByText(/Memory entries|メモリエントリ/)).toBeVisible({ timeout: 15_000 }); + await expect(page.getByRole('heading', { name: /Memory entries|メモリエントリ/ })).toBeVisible({ timeout: 15_000 }); await expect(page.getByRole('button', { name: /New entry|新しいエントリ/ })).toBeVisible(); - // pieces → the SpacePiecesPanel list (Default / Custom sections) renders. These - // section labels are literal (non-i18n) strings; match the built-in piece list. + // pieces → the SpacePiecesPanel list (Default / Custom sections) renders. + // These section labels are i18n (spaces.json settings.pieces.default/custom). await page.getByTestId('space-settings-nav-pieces').click(); - await expect(page.getByText('Default', { exact: true })).toBeVisible({ timeout: 15_000 }); - await expect(page.getByText('Custom', { exact: true })).toBeVisible(); + await expect(page.getByText(/^(Default|標準)$/)).toBeVisible({ timeout: 15_000 }); + await expect(page.getByText(/^(Custom|カスタム)$/)).toBeVisible(); // skills → the SkillsForm heading renders (the install-from-URL input is // intentionally hidden in space mode — the Git install path isn't space-scoped @@ -1442,8 +1452,7 @@ test('space settings sub-tabs: agents/memory/pieces/skills/mcp/ssh all render th await page.getByTestId('space-settings-nav-ssh').click(); await expect(page.getByTestId('space-ssh-panel')).toBeVisible({ timeout: 15_000 }); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1493,11 +1502,10 @@ test('space settings MEMORY: entry saved in space A persists + is absent in spac await expect(detail).toBeVisible(); await page.getByTestId('space-tab-settings').click(); await page.getByTestId('space-settings-nav-memory').click(); - await expect(page.getByText(/Memory entries|メモリエントリ/)).toBeVisible({ timeout: 15_000 }); + await expect(page.getByRole('heading', { name: /Memory entries|メモリエントリ/ })).toBeVisible({ timeout: 15_000 }); await expect(page.getByText(memName, { exact: true })).toHaveCount(0); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1542,8 +1550,7 @@ test('space calendar: tab renders month grid + month nav', async ({ page }) => { await page.getByTestId('space-cal-prev').click(); await expect(title).toHaveText(initial!); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1556,7 +1563,7 @@ test('space calendar: add event → list + day badge + persists across reload', const today = e2eLocalToday(); const todayCell = page.getByTestId(`space-cal-day-${today}`); - // Badge baseline: today's event count badge (📌N) is absent before adding. + // Baseline: today's event count badge (📌N) is absent before adding. await expect(todayCell.getByText(/^📌/)).toHaveCount(0); // Open today's day panel and add an event. @@ -1577,13 +1584,26 @@ test('space calendar: add event → list + day badge + persists across reload', await expect(panel.getByText(eventTitle, { exact: true })).toBeVisible({ timeout: 10_000 }); await expect(panel.getByText('14:30')).toBeVisible(); - // The day cell event badge increments to 📌1 after the month query invalidates. - await expect(todayCell.getByText('📌1')).toBeVisible({ timeout: 10_000 }); + // Grab the event id from its panel row so we can assert both month-grid + // indicators by id (a single-day event drives TWO separate render paths). + const row = panel.locator('[data-testid^="space-cal-event-"]').filter({ hasText: eventTitle }).first(); + const eventId = (await row.getAttribute('data-testid'))!.replace('space-cal-event-', ''); + expect(eventId).toMatch(/^\d+$/); - // Persists across a full reload (server-backed, not just client state). + // Month grid shows BOTH indicators for the single-day event: + // - the 📌1 day-cell badge (counts.eventCount path, added in #808 / #804). + // exact:true so it pins count===1, not a substring of 📌1x. + // - a colSpan-1 week bar (layoutWeekBars path) — a separate code path, so + // assert it explicitly (a badge-only check would miss a phantom-bar bug). + await expect(todayCell.getByText('📌1', { exact: true })).toBeVisible({ timeout: 10_000 }); + await expect(page.getByTestId(`space-cal-bar-${eventId}`)).toBeVisible({ timeout: 10_000 }); + + // Persists across a full reload (server-backed, not just client state): + // day-cell badge, week bar, and the day-panel event all come back. await page.reload(); await page.getByTestId('space-tab-calendar').click(); - await expect(page.getByTestId(`space-cal-day-${today}`).getByText('📌1')).toBeVisible({ timeout: 10_000 }); + await expect(page.getByTestId(`space-cal-day-${today}`).getByText('📌1', { exact: true })).toBeVisible({ timeout: 10_000 }); + await expect(page.getByTestId(`space-cal-bar-${eventId}`)).toBeVisible({ timeout: 10_000 }); await page.getByTestId(`space-cal-day-${today}`).click(); await expect(page.getByTestId('space-cal-day-panel').getByText(eventTitle, { exact: true })).toBeVisible({ timeout: 10_000 }); @@ -1621,12 +1641,20 @@ test('space calendar: edit + delete an event', async ({ page }) => { await page.getByTestId('space-cal-event-save').click(); await expect(panel.getByText(newTitle, { exact: true })).toBeVisible({ timeout: 10_000 }); await expect(panel.getByText(origTitle, { exact: true })).toHaveCount(0); + // While the event exists the day cell carries both month-grid indicators: + // the 📌1 badge (eventCount path) and the colSpan-1 week bar (layoutWeekBars + // path). Assert both so a regression in either path is caught. + await expect(todayCell.getByText('📌1', { exact: true })).toBeVisible({ timeout: 10_000 }); + await expect(page.getByTestId(`space-cal-bar-${eventId}`)).toBeVisible({ timeout: 10_000 }); - // Delete: confirm the window.confirm() then assert the row disappears + badge clears. + // Delete: confirm the window.confirm() then assert the row disappears and + // BOTH month-grid indicators clear (badge tracks eventCount, bar comes from + // the events array — separate paths, so a delete could orphan one). page.once('dialog', (d) => d.accept()); await panel.getByTestId(`space-cal-event-delete-${eventId}`).click(); await expect(panel.getByTestId(`space-cal-event-${eventId}`)).toHaveCount(0, { timeout: 10_000 }); await expect(todayCell.getByText(/^📌/)).toHaveCount(0, { timeout: 10_000 }); + await expect(page.getByTestId(`space-cal-bar-${eventId}`)).toHaveCount(0, { timeout: 10_000 }); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1658,8 +1686,7 @@ test('space calendar: day panel lists tasks created in the space', async ({ page await expect(page.getByTestId('space-conversation')).toBeVisible(); await expect(page).toHaveURL(/[?&]chat=\d+/); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1706,8 +1733,7 @@ test('space settings MEMBERS: panel renders + invite shows no-auth hint', async await expect(picker).toBeVisible({ timeout: 15_000 }); await expect(picker.getByText('認証を有効化するとワークスペースを共有できます。')).toBeVisible(); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1743,8 +1769,7 @@ test('space sources: curated ソース list view (incl. empty-state) is absent o await expect(page.getByTestId('space-sources')).toHaveCount(0); await expect(page.getByTestId('space-sources-empty')).toHaveCount(0); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1799,8 +1824,7 @@ test('space sources: source/ folder still accumulates and is browsable; curated page.locator('[data-testid="space-file-tile"][data-name="index.jsonl"]'), ).toHaveCount(0); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1857,8 +1881,7 @@ test('space settings BROWSER: panel + three groups render with empty states', as await expect(recordings.getByRole('heading', { name: '録画' })).toBeVisible(); await expect(recordings).toContainText('このワークスペースにはまだ録画がありません'); - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1904,7 +1927,7 @@ test('space files DELETE: select a file and delete it (gone); ソース list vie await expect(tile).toHaveCount(0, { timeout: 15000 }); // No-nav invariant. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); await expect(detail).toBeVisible(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); @@ -1978,13 +2001,15 @@ test('space RENAME: header pencil renames the workspace and the title updates', const fatalErrors = trackFatalErrors(page); const { detail, caseTitle, rail } = await createAndOpenCaseSpace(page, 'E2E改名'); - // Open the inline rename editor, change the title, save. - await page.getByTestId('space-rename').click(); - const input = page.getByTestId('space-rename-input'); + // Open the header pencil, which reuses the create/edit SpaceFormDialog + // (space-title-input pre-filled with the current title, same submit button + // as create). There is no dedicated inline rename input/save pair. + await page.getByTestId('space-edit').click(); + const input = page.getByTestId('space-title-input'); await expect(input).toBeVisible(); const renamed = `${caseTitle}-改`; await input.fill(renamed); - await page.getByTestId('space-rename-save').click(); + await page.getByTestId('space-form-submit').click(); // The header reflects the new title, and the rail row updates too. await expect(detail).toContainText(renamed); @@ -2013,8 +2038,7 @@ test('space DELETE: case workspace delete returns to the list (selection cleared await expect.poll(() => new URL(page.url()).searchParams.get('space')).toBeFalsy(); // Still on Spaces, never bounced to Tasks. - await expect(page).toHaveURL(/[?&]page=spaces(&|$)/); - expect(page.url()).not.toContain('page=tasks'); + expect(new URL(page.url()).searchParams.get('page')).toBeFalsy(); expect(fatalErrors, `fatal errors:\n${fatalErrors.join('\n')}`).toEqual([]); }); diff --git a/ui/playwright.config.ts b/ui/playwright.config.ts index 90eddc8..741671e 100644 --- a/ui/playwright.config.ts +++ b/ui/playwright.config.ts @@ -37,6 +37,23 @@ const WORKTREE_DIR = join(e2eTmp, 'workspaces'); // Written to a gitignored file next to this config so specs can read it. writeFileSync(join(__dirname, '.e2e-db-path'), DB_PATH); +// Dedicated, deterministic config for the E2E webServer, handed over via +// AAO_CONFIG. Two reasons: +// 1) Isolation — without this, dist/main.js loads the repo-root ./config.yaml +// if a developer happens to have one (it's gitignored, so present locally +// but absent in CI). That silently changes E2E behavior between machines +// (e.g. pointing the worker at a real LLM). Pinning AAO_CONFIG makes the +// run identical everywhere; everything unset here falls back to defaults. +// 2) SSH enabled — the settings sub-tab specs assert the space-scoped SSH +// panel (space-ssh-panel) renders. That testid only mounts once +// /api/ssh/connections answers 200; with SSH off the route 404s and the +// panel shows its "SSH disabled" view (no testid). In isolation the spec +// scraped the brief loading-state render and passed by luck; under full- +// suite load that window closed and both SSH specs failed. Enabling SSH +// (needs the MCP_ENCRYPTION_KEY set below) makes the panel deterministic. +const E2E_CONFIG = join(e2eTmp, 'e2e-config.yaml'); +writeFileSync(E2E_CONFIG, 'ssh:\n enabled: true\n'); + export default defineConfig({ testDir: resolve(__dirname, 'e2e'), // The build+boot can be slow; keep a roomy per-test budget. @@ -50,6 +67,7 @@ export default defineConfig({ use: { baseURL: BASE_URL, headless: true, + locale: 'ja-JP', // This sandbox disables the Chromium sandbox (bwrap/userns off). Launch // with --no-sandbox or every browser launch fails to spawn. launchOptions: { @@ -65,8 +83,9 @@ export default defineConfig({ }, ], - // Boot the real orchestrator (worker mode) with auth OFF (no config.yaml => - // defaults => synthetic 'local' user). dist/ must exist (npm run build:all). + // Boot the real orchestrator (worker mode) with auth OFF (the pinned + // E2E_CONFIG has no auth section => defaults => synthetic 'local' user). + // dist/ must exist (npm run build:all). webServer: { command: 'node dist/main.js', cwd: repoRoot, @@ -80,6 +99,9 @@ export default defineConfig({ PORT: String(PORT), DB_PATH, WORKTREE_DIR, + // Pin the config to the throwaway E2E file so the run never inherits a + // developer's repo-root ./config.yaml (see E2E_CONFIG above). + AAO_CONFIG: E2E_CONFIG, AAO_MODE: 'worker', LOG_LEVEL: 'warn', // Mount the MCP subsystem (gated on a 64-hex MCP_ENCRYPTION_KEY) so the diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 808a275..18cf1db 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -45,6 +45,8 @@ export interface AuthUser { orgIds?: string[]; defaultVisibility?: Visibility; defaultVisibilityOrgId?: string | null; + /** True for accounts with a local (email+password) credential; false/undefined for OAuth-only accounts. */ + hasLocalCredential?: boolean; } type AuthMode = diff --git a/ui/src/api/spaces.ts b/ui/src/api/spaces.ts index 948b7d0..8afdadd 100644 --- a/ui/src/api/spaces.ts +++ b/ui/src/api/spaces.ts @@ -16,6 +16,8 @@ export interface Space { workspaceDir: string | null; createdAt: string; updatedAt: string; + favorite?: boolean; + hidden?: boolean; /** 閲覧者自身のメンバーロール。一覧（GET /spaces）・詳細（GET /spaces/:id）の両方で付与。 * 非メンバー（根オーナー含む。owner_id はメンバー行ではない）は null。 */ myRole?: SpaceMemberRole | null; @@ -28,6 +30,20 @@ export async function fetchSpaces(): Promise { return data as Space[]; } +export async function updateSpaceDisplayPrefs( + id: string, + patch: { favorite?: boolean; hidden?: boolean }, +): Promise<{ favorite: boolean; hidden: boolean }> { + const res = await fetch(`${BASE}/local/spaces/${id}/display`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(patch), + }); + const data = await res.json().catch(() => ({})); + if (!res.ok) throw new Error(data?.error ?? 'Failed to update workspace display settings'); + return data as { favorite: boolean; hidden: boolean }; +} + export type SpaceMemberRole = 'owner' | 'editor' | 'viewer'; export interface SpaceMember { diff --git a/ui/src/components/detail/DetailPanel.tsx b/ui/src/components/detail/DetailPanel.tsx index bc078ff..223126a 100644 --- a/ui/src/components/detail/DetailPanel.tsx +++ b/ui/src/components/detail/DetailPanel.tsx @@ -47,6 +47,7 @@ interface LocalDetailPanelProps { fileManagement?: FileManagement; subtaskActivities?: SubtaskActivity[]; onSubtaskFilePreview?: SubtaskFilePreviewHandler; + onWorkspaceFilePreview?: (path: string) => void; shareToken?: string | null; onShareChange?: () => void; /** When true, omit the DetailHeader (tab row + close button) and render only @@ -64,7 +65,7 @@ export function LocalDetailPanel({ task, taskId, section, currentPath, entries, pathSegments, loading, detailTab, detailWidth, showWidthToggle, onTabChange, onWidthToggle, onClose, onDelete, onSectionChange, onNavigate, onPreview, onViewFullLog, - onRefresh, isRefreshing, fileManagement, subtaskActivities, onSubtaskFilePreview, + onRefresh, isRefreshing, fileManagement, subtaskActivities, onSubtaskFilePreview, onWorkspaceFilePreview, shareToken, onShareChange, headerless = false, readonly = false, }: LocalDetailPanelProps) { const { t } = useTranslation('detail'); @@ -297,7 +298,7 @@ export function LocalDetailPanel({ {task?.latestJob?.status === 'waiting_human' && task?.latestJob?.waitReason === 'browser_login' && ( )} - {deferredDetailTab === 'overview' && } + {deferredDetailTab === 'overview' && } {deferredDetailTab === 'activity' && } {deferredDetailTab === 'files' && } {deferredDetailTab === 'trace' && } diff --git a/ui/src/components/detail/tabs/DelegateRunsSection.test.tsx b/ui/src/components/detail/tabs/DelegateRunsSection.test.tsx index 0681e7c..a3dedd1 100644 --- a/ui/src/components/detail/tabs/DelegateRunsSection.test.tsx +++ b/ui/src/components/detail/tabs/DelegateRunsSection.test.tsx @@ -8,40 +8,52 @@ import { initReactI18next } from 'react-i18next'; // Initialize i18next with minimal translations for this test. // The component uses the 'detail' namespace. +const TEST_DETAIL_RESOURCES = { + 'delegateRuns.eventsEmpty': 'No events', + 'delegateRuns.subtaskGroupTitle': 'Subtask #{{n}}', + 'delegateRuns.subtaskSectionHeading': 'Delegate runs in subtasks', + 'delegateRuns.result': 'Result', + 'delegateRuns.abortReason': 'Abort reason', + 'delegateRuns.failedTool': 'Last failing tool: {{tool}}', + 'delegateRuns.noDescription': '(no description)', + 'delegateRuns.toolCount': '{{count}} tools', + // 実ロケール (ui/src/i18n/locales/en/detail.json) は toolCount に _one/_other の複数形 + // バリアントを持つ。addResourceBundle の overwrite は完全一致するキーしか上書きしないため、 + // 実ロケールが先に読み込まれた場合に備えてここでも明示的に単数形を無効化しておく。 + 'delegateRuns.toolCount_one': '{{count}} tools', + 'delegateRuns.toolCount_other': '{{count}} tools', + 'delegateRuns.childCount': '{{total}} children', + 'delegateRuns.childCountFailed': '{{total}} children, {{failed}} failed', + 'delegateRuns.moreEvents': '{{count}} more events (see the Trace tab for the full list)', + 'delegateRuns.toolSummary': 'Tools used', + 'delegateRuns.filesChanged': 'Changed files', + 'delegateRuns.eventsToggle': 'Detailed events ({{count}})', + 'subtasks.delegateSection': 'Delegated runs', + 'subtasks.delegateStatus.success': 'Done', + 'subtasks.delegateStatus.aborted': 'Aborted', + 'subtasks.delegateStatus.running': 'Running', + 'subtasks.delegateRunningTool': '{{tool}} running', +}; +const TEST_COMMON_RESOURCES = { loading: 'Loading...' }; + beforeAll(async () => { - if (!i18n.isInitialized) { - await i18n.use(initReactI18next).init({ - lng: 'en', - fallbackLng: 'en', - resources: { - en: { - detail: { - 'delegateRuns.eventsEmpty': 'No events', - 'delegateRuns.subtaskGroupTitle': 'Subtask #{{n}}', - 'delegateRuns.subtaskSectionHeading': 'Delegate runs in subtasks', - 'delegateRuns.result': 'Result', - 'delegateRuns.abortReason': 'Abort reason', - 'delegateRuns.noDescription': '(no description)', - 'delegateRuns.toolCount': '{{count}} tools', - 'delegateRuns.childCount': '{{total}} children', - 'delegateRuns.childCountFailed': '{{total}} children, {{failed}} failed', - 'delegateRuns.moreEvents': '{{count}} more events (see the Trace tab for the full list)', - 'delegateRuns.toolSummary': 'Tools used', - 'delegateRuns.filesChanged': 'Changed files', - 'delegateRuns.eventsToggle': 'Detailed events ({{count}})', - 'subtasks.delegateSection': 'Delegated runs', - 'subtasks.delegateStatus.success': 'Done', - 'subtasks.delegateStatus.aborted': 'Aborted', - 'subtasks.delegateStatus.running': 'Running', - 'subtasks.delegateRunningTool': '{{tool}} running', - }, - common: { loading: 'Loading...' }, - }, + // DelegateRunsSection.tsx は lib/utils（isPreviewable）経由で実 i18n シングルトン + // (../../../i18n) を静的 import しており、モジュール読み込み時の副作用で本番ロケール + // （toolCount_one/_other など複数形リソースを含む）が先に初期化されてしまうことがある。 + // isInitialized の状態に関わらず常に再 init し、このテストが前提とする簡易文言に + // 確実に揃える（i18next の init は複数回呼んでよい）。 + await i18n.use(initReactI18next).init({ + lng: 'en', + fallbackLng: 'en', + resources: { + en: { + detail: TEST_DETAIL_RESOURCES, + common: TEST_COMMON_RESOURCES, }, - defaultNS: 'common', - interpolation: { escapeValue: false }, - }); - } + }, + defaultNS: 'common', + interpolation: { escapeValue: false }, + }); }); vi.mock('../../../api', () => ({ @@ -113,6 +125,21 @@ vi.mock('../../../api', () => ({ toolCalls: 0, resultPreview: 'Failed', totalTokens: 100, + lastErrorTool: 'Bash', + }, + { + delegateRunId: 'p5', + parentRunId: null, + description: 'ツール不明の失敗した委譲', + depth: 1, + status: 'aborted', + startTs: '2026-01-01T00:07:00Z', + endTs: '2026-01-01T00:07:30Z', + eventCount: 1, + toolCalls: 0, + resultPreview: 'Failed without tool', + totalTokens: 0, + lastErrorTool: null, }, { delegateRunId: 'p3', @@ -448,6 +475,44 @@ describe('DelegateRunsSection', () => { expect(screen.getByText('output/summary.txt')).toBeInTheDocument(); }); + it('onWorkspaceFilePreview を渡すと、filesChanged のパスをクリックしてそのパス文字列で呼ばれる', async () => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const onWorkspaceFilePreview = vi.fn(); + render( + + + , + ); + + // 親委譲のカードを開く（filesChanged が設定されている） + await screen.findByText('親委譲'); + const parentButton = screen.getByText('親委譲').closest('button') as HTMLButtonElement; + await userEvent.click(parentButton); + + const fileButton = screen.getByText('output/result.md'); + expect(fileButton.tagName).toBe('BUTTON'); + await userEvent.click(fileButton); + + expect(onWorkspaceFilePreview).toHaveBeenCalledWith('output/result.md'); + }); + + it('onWorkspaceFilePreview 未提供なら filesChanged のパスは従来通り div のまま（クリックできない）', async () => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + + // 親委譲のカードを開く（filesChanged が設定されている） + await screen.findByText('親委譲'); + const parentButton = screen.getByText('親委譲').closest('button') as HTMLButtonElement; + await userEvent.click(parentButton); + + const fileEl = screen.getByText('output/result.md'); + expect(fileEl.tagName).not.toBe('BUTTON'); + }); + it('filesChanged 空または undefined なら見出しごと出ない', async () => { const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); render( @@ -497,4 +562,81 @@ describe('DelegateRunsSection', () => { // トグルが開いた状態になる expect(eventsToggle).toHaveAttribute('aria-expanded', 'true'); }); + + it('説明文 span に title 属性が付く（狭い時に truncate されてもホバーで全文参照できる）', async () => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + + const descriptionSpan = await screen.findByText('親委譲'); + expect(descriptionSpan).toHaveAttribute('title', '親委譲'); + }); + + it('説明文 span に min-w-[8ch] クラスが含まれる（優先的に幅を確保する）', async () => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + + const descriptionSpan = await screen.findByText('親委譲'); + expect(descriptionSpan).toHaveClass('min-w-[8ch]'); + }); + + it('lastErrorTool ありの aborted run で「最後に失敗したツール」行が表示される', async () => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + + // 失敗した子委譲のカードを開く（親「失敗した親委譲」を開いてから子を開く） + await screen.findByText('失敗した親委譲'); + const parentButton = screen.getByText('失敗した親委譲').closest('button') as HTMLButtonElement; + await userEvent.click(parentButton); + + await screen.findByText('失敗した子委譲'); + const childButton = screen.getByText('失敗した子委譲').closest('button') as HTMLButtonElement; + await userEvent.click(childButton); + + expect(screen.getByText('Last failing tool: Bash')).toBeInTheDocument(); + }); + + it('lastErrorTool が null の aborted run では「最後に失敗したツール」行が表示されない', async () => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + + await screen.findByText('ツール不明の失敗した委譲'); + const button = screen.getByText('ツール不明の失敗した委譲').closest('button') as HTMLButtonElement; + await userEvent.click(button); + + expect(screen.queryByText(/Last failing tool/)).not.toBeInTheDocument(); + }); + + it('メタ情報 span は shrink-0 を含まない（狭い時に優先的に truncate される）', async () => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + + await screen.findByText('親委譲'); + const parentButton = screen.getByText('親委譲').closest('button') as HTMLButtonElement; + const metaSpan = Array.from(parentButton.querySelectorAll('span')).find((el) => + el.textContent?.includes('tools') && el.textContent?.includes('·'), + ); + expect(metaSpan).toBeDefined(); + expect(metaSpan).not.toHaveClass('shrink-0'); + expect(metaSpan).toHaveClass('truncate', 'min-w-0'); + }); }); diff --git a/ui/src/components/detail/tabs/DelegateRunsSection.tsx b/ui/src/components/detail/tabs/DelegateRunsSection.tsx index 8494dd7..5ef7b47 100644 --- a/ui/src/components/detail/tabs/DelegateRunsSection.tsx +++ b/ui/src/components/detail/tabs/DelegateRunsSection.tsx @@ -6,6 +6,7 @@ import { fetchDelegateRuns, fetchDelegateRunTimeline, type TraceEventLite } from import { buildDelegateRunTree, currentRunningTool, delegateStatusBadge, formatElapsed, formatTokens, summarizeDescendants, type DelegateRunNode, type DelegateRun, type SubtaskDelegateGroup } from '../../../lib/delegateRuns'; import { useNow } from '../../../hooks/useNow'; import { summarizeTraceEvent } from '../../../lib/traceEvent'; +import { isPreviewable } from '../../../lib/utils'; function EventLine({ event }: { event: TraceEventLite }) { const summary = summarizeTraceEvent(event); @@ -18,7 +19,7 @@ function EventLine({ event }: { event: TraceEventLite }) { ); } -function RunCard({ taskId, node, indent = 0, jobId }: { taskId: number; node: DelegateRunNode; indent?: number; jobId?: string }) { +function RunCard({ taskId, node, indent = 0, jobId, onWorkspaceFilePreview }: { taskId: number; node: DelegateRunNode; indent?: number; jobId?: string; onWorkspaceFilePreview?: (path: string) => void }) { const { t } = useTranslation('detail'); const [open, setOpen] = useState(false); const [eventsOpen, setEventsOpen] = useState(false); @@ -71,10 +72,13 @@ function RunCard({ taskId, node, indent = 0, jobId }: { taskId: number; node: De {childBadgeContent} )} - + {node.description || t('delegateRuns.noDescription')} - + {t('delegateRuns.toolCount', { count: node.toolCalls })} · {formatElapsed(node.startTs, node.endTs, now)}{node.totalTokens && node.totalTokens > 0 ? ` · ${formatTokens(node.totalTokens)}` : ''} @@ -96,6 +100,11 @@ function RunCard({ taskId, node, indent = 0, jobId }: { taskId: number; node: De
{t(node.status === 'aborted' ? 'delegateRuns.abortReason' : 'delegateRuns.result')}
+ {node.status === 'aborted' && node.lastErrorTool && ( +
+ {t('delegateRuns.failedTool', { tool: node.lastErrorTool })} +
+ )}
{node.resultPreview}
@@ -117,11 +126,23 @@ function RunCard({ taskId, node, indent = 0, jobId }: { taskId: number; node: De
{t('delegateRuns.filesChanged')}
- {node.filesChanged.map((path, idx) => ( -
- {path} -
- ))} + {node.filesChanged.map((path, idx) => { + const previewable = isPreviewable(path); + return previewable && onWorkspaceFilePreview ? ( + + ) : ( +
+ {path} +
+ ); + })}
)} @@ -151,7 +172,7 @@ function RunCard({ taskId, node, indent = 0, jobId }: { taskId: number; node: De )} {node.children.map((c) => ( - + ))} )} @@ -159,7 +180,7 @@ function RunCard({ taskId, node, indent = 0, jobId }: { taskId: number; node: De ); } -export function DelegateRunsSection({ taskId }: { taskId: number }) { +export function DelegateRunsSection({ taskId, onWorkspaceFilePreview }: { taskId: number; onWorkspaceFilePreview?: (path: string) => void }) { const { t } = useTranslation('detail'); const { data } = useQuery({ queryKey: ['delegate-runs', taskId], @@ -182,7 +203,7 @@ export function DelegateRunsSection({ taskId }: { taskId: number }) {
{t('subtasks.delegateSection')}
{tree.map((n) => ( - + ))} {subtasks.map((group) => { const groupTree = buildDelegateRunTree(group.runs); @@ -192,7 +213,7 @@ export function DelegateRunsSection({ taskId }: { taskId: number }) { {t('delegateRuns.subtaskGroupTitle', { n: group.issueNumber })}
{groupTree.map((n) => ( - + ))} ); diff --git a/ui/src/components/detail/tabs/OverviewTab.tsx b/ui/src/components/detail/tabs/OverviewTab.tsx index cd369ed..b5c9128 100644 --- a/ui/src/components/detail/tabs/OverviewTab.tsx +++ b/ui/src/components/detail/tabs/OverviewTab.tsx @@ -451,9 +451,10 @@ interface OverviewTabProps { subtaskActivities?: SubtaskActivity[]; readonly?: boolean; onSubtaskFilePreview?: SubtaskFilePreviewHandler; + onWorkspaceFilePreview?: (path: string) => void; } -export function OverviewTab({ task, subtaskActivities, onSubtaskFilePreview, readonly = false }: OverviewTabProps) { +export function OverviewTab({ task, subtaskActivities, onSubtaskFilePreview, onWorkspaceFilePreview, readonly = false }: OverviewTabProps) { const status = task.latestJob?.status ?? 'queued'; return ( @@ -494,7 +495,7 @@ export function OverviewTab({ task, subtaskActivities, onSubtaskFilePreview, rea {/* delegate サブ実行: SpawnSubTask の有無に関係なく表示（runが無ければ自己非表示）。 SubtasksPanel は subtasks.length>0 でしかマウントされないため、delegate のみの タスクでも見えるよう独立して描画する。 */} - + ); } diff --git a/ui/src/components/layout/TopBar.tsx b/ui/src/components/layout/TopBar.tsx index fbb36e7..69a3255 100644 --- a/ui/src/components/layout/TopBar.tsx +++ b/ui/src/components/layout/TopBar.tsx @@ -3,7 +3,7 @@ import { useTranslation } from 'react-i18next'; import type { PageId } from '../../lib/urlState'; import type { AuthUser } from '../../App'; import { ThemeToggle } from './ThemeToggle'; -import { ChangePasswordDialog } from '../admin/LocalUserDialogs'; +import { UserAccountMenu } from './UserAccountMenu'; interface TopBarProps { currentPage: PageId; @@ -92,7 +92,6 @@ export function TopBar({ }: TopBarProps) { const { t } = useTranslation('layout'); const visibleNav = visibleNavItemsFor(isAdmin, authEnabled); - const [showPwChange, setShowPwChange] = useState(false); // Collapse-to-hamburger decision from MEASURED widths, so it flips exactly // when the tabs stop fitting — no width estimate, no 2-line state. @@ -226,41 +225,8 @@ export function TopBar({ ⌘K )} - - {user && ( -
-
- {user.avatarUrl ? ( - {user.name - ) : ( -
- {(user.name ?? user.email).charAt(0)} -
- )} - - {user.name ?? user.email} - -
- - - {t('user.logout')} - -
- )} - {showPwChange && setShowPwChange(false)} />} + {!user && } + {user && } diff --git a/ui/src/components/layout/UserAccountMenu.test.tsx b/ui/src/components/layout/UserAccountMenu.test.tsx new file mode 100644 index 0000000..e6170b4 --- /dev/null +++ b/ui/src/components/layout/UserAccountMenu.test.tsx @@ -0,0 +1,117 @@ +// @vitest-environment jsdom +import '../../test/dom-setup'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import type { AuthUser } from '../../App'; + +// react-i18next: pass keys through (mirrors WorkerStatusWidget.test.tsx). The +// component only relies on translated *keys*, not the rendered copy, so this +// keeps the test independent of locale JSON wording. +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +import { UserAccountMenu } from './UserAccountMenu'; + +// ThemeToggle (rendered inside the open panel) reads matchMedia for +// prefers-color-scheme; jsdom doesn't implement it (see ChatPane.drafts.test.tsx +// for the same minimal stub). +beforeEach(() => { + vi.stubGlobal('matchMedia', (query: string) => ({ + matches: false, + media: query, + addEventListener: () => {}, + removeEventListener: () => {}, + })); +}); + +function mkUser(over: Partial = {}): AuthUser { + return { + id: 'u1', + email: 'alice@example.com', + name: 'Alice', + avatarUrl: null, + role: 'user', + ...over, + }; +} + +describe('UserAccountMenu', () => { + it('starts closed and opens the panel on trigger click', async () => { + const user = userEvent.setup(); + render(); + + expect(screen.queryByTestId('user-account-menu-panel')).not.toBeInTheDocument(); + const trigger = screen.getByTestId('user-account-menu-trigger'); + expect(trigger).toHaveAttribute('aria-haspopup', 'true'); + expect(trigger).toHaveAttribute('aria-expanded', 'false'); + + await user.click(trigger); + + expect(trigger).toHaveAttribute('aria-expanded', 'true'); + expect(screen.getByTestId('user-account-menu-panel')).toBeInTheDocument(); + }); + + it('shows user name/email and closes on outside click', async () => { + const user = userEvent.setup(); + render( +
+ + +
, + ); + await user.click(screen.getByTestId('user-account-menu-trigger')); + const panel = screen.getByTestId('user-account-menu-panel'); + expect(within(panel).getByText('Alice')).toBeInTheDocument(); + expect(within(panel).getByText('alice@example.com')).toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'outside' })); + expect(screen.queryByTestId('user-account-menu-panel')).not.toBeInTheDocument(); + }); + + it('closes on Escape and returns focus to the trigger', async () => { + const user = userEvent.setup(); + render(); + const trigger = screen.getByTestId('user-account-menu-trigger'); + await user.click(trigger); + expect(screen.getByTestId('user-account-menu-panel')).toBeInTheDocument(); + + await user.keyboard('{Escape}'); + + expect(screen.queryByTestId('user-account-menu-panel')).not.toBeInTheDocument(); + expect(trigger).toHaveFocus(); + }); + + it('renders a logout link pointing at /auth/logout', async () => { + const user = userEvent.setup(); + render(); + await user.click(screen.getByTestId('user-account-menu-trigger')); + const logoutLink = screen.getByRole('link', { name: 'user.logout' }); + expect(logoutLink).toHaveAttribute('href', '/auth/logout'); + }); + + it('calls onNavigate("settings") and closes when the settings item is clicked', async () => { + const user = userEvent.setup(); + const onNavigate = vi.fn(); + render(); + await user.click(screen.getByTestId('user-account-menu-trigger')); + await user.click(screen.getByRole('button', { name: 'nav.settings' })); + expect(onNavigate).toHaveBeenCalledWith('settings'); + expect(screen.queryByTestId('user-account-menu-panel')).not.toBeInTheDocument(); + }); + + it('renders the theme toggle inside the open panel', async () => { + const user = userEvent.setup(); + render(); + await user.click(screen.getByTestId('user-account-menu-trigger')); + const panel = screen.getByTestId('user-account-menu-panel'); + // ThemeToggle renders a role="group" with 3 theme option buttons (system/light/dark). + expect(within(panel).getByRole('group')).toBeInTheDocument(); + expect(within(panel).getAllByRole('button', { name: /theme\./ }).length).toBeGreaterThanOrEqual(3); + + const systemBtn = within(panel).getByRole('button', { name: 'theme.system' }); + await user.click(systemBtn); + expect(systemBtn).toHaveAttribute('aria-pressed', 'true'); + }); +}); diff --git a/ui/src/components/layout/UserAccountMenu.tsx b/ui/src/components/layout/UserAccountMenu.tsx new file mode 100644 index 0000000..68fb490 --- /dev/null +++ b/ui/src/components/layout/UserAccountMenu.tsx @@ -0,0 +1,103 @@ +import { useEffect, useRef, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import type { PageId } from '../../lib/urlState'; +import type { AuthUser } from '../../App'; +import { ThemeToggle } from './ThemeToggle'; + +interface UserAccountMenuProps { + user: AuthUser; + onNavigate: (page: PageId) => void; +} + +/** + * TopBar 右上のアカウントメニュー。アバター/名前のトリガーをクリックするとパネルが開き、 + * ユーザー名/メール（読み取り専用）、ダークモード切替、設定リンク、ログアウトを表示する。 + * 開閉制御は FileToolbar.tsx の FileSortMenu と同じ mousedown+Escape クリックアウト実装を踏襲。 + * role="menu" 等の完全な ARIA menu パターンは実装コストに見合わないため使わず（issue #799 + * 設計メモ参照）、通常の button/link 群 + aria-haspopup/aria-expanded で足りる。 + */ +export function UserAccountMenu({ user, onNavigate }: UserAccountMenuProps) { + const { t } = useTranslation('layout'); + const [open, setOpen] = useState(false); + const containerRef = useRef(null); + const triggerRef = useRef(null); + const displayName = user.name ?? user.email; + + useEffect(() => { + if (!open) return; + const handleMouseDown = (e: MouseEvent) => { + if (containerRef.current && !containerRef.current.contains(e.target as Node)) { + setOpen(false); + } + }; + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') { + setOpen(false); + triggerRef.current?.focus(); + } + }; + document.addEventListener('mousedown', handleMouseDown); + document.addEventListener('keydown', handleKeyDown); + return () => { + document.removeEventListener('mousedown', handleMouseDown); + document.removeEventListener('keydown', handleKeyDown); + }; + }, [open]); + + const close = () => setOpen(false); + + return ( +
+ + {open && ( +
+
+

{displayName}

+ {user.name &&

{user.email}

} +
+
+ {t('theme.label')} + +
+ + + {t('user.logout')} + +
+ )} +
+ ); +} diff --git a/ui/src/components/settings/ConfigForm.extra-body-validity.test.tsx b/ui/src/components/settings/ConfigForm.extra-body-validity.test.tsx new file mode 100644 index 0000000..8160380 --- /dev/null +++ b/ui/src/components/settings/ConfigForm.extra-body-validity.test.tsx @@ -0,0 +1,300 @@ +// @vitest-environment jsdom +/** + * Save-blocking through the REAL ConfigForm save bar (Task 6 reviewer fix): + * an invalid extra_body draft must disable Save & Apply even when a + * DIFFERENT field is dirty — otherwise the operator clicks Save, the + * invalid extra_body edit is silently dropped, and everything looks saved. + * + * Wiring under test: ExtraBodyField → onValidityChange (SectionFormProps) + * → ConfigForm's invalidKeys set → Save & Apply disabled + hint. Also the + * stale-key failure mode: removing the offending worker row must clear its + * invalid flag (unmount cleanup), or Save would stay bricked forever. + * + * ConfigForm's import graph initializes the real i18n, so (like + * AuthForm.test.tsx and friends) we pin the language to 'en' and assert + * against i18n.t(...) instead of raw keys. + */ +import '../../test/dom-setup'; +import { beforeAll, beforeEach, afterEach, describe, it, expect, vi } from 'vitest'; +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { renderWithProviders } from '../../test/render-helpers'; +import i18n from '../../i18n'; +import { ConfigForm } from './ConfigForm'; + +// ConfigForm pulls useAuthState from App.tsx (huge module graph) and loads +// the config via react-query; stub both so we can drive the real save bar + +// section-form wiring without a server. `mockConfigData` is assigned +// per-test before render (the mock factory closes over it lazily). +let mockConfigData: any; +vi.mock('../../App', () => ({ useAuthState: () => ({ mode: 'disabled' }) })); +vi.mock('../../lib/unsavedGuard', () => ({ useUnsavedGuard: () => {} })); +vi.mock('../../hooks/useConfig', () => ({ + useConfig: () => ({ data: mockConfigData, isLoading: false, error: null, refetch: vi.fn() }), +})); + +beforeAll(async () => { + await i18n.changeLanguage('en'); +}); + +beforeEach(() => { + // Never let ModelSelect hit the network. + vi.stubGlobal( + 'fetch', + vi.fn(async () => ({ ok: false, status: 500, json: async () => ({}) })) as any, + ); +}); +afterEach(() => { + vi.unstubAllGlobals(); +}); + +const tSettings = (key: string) => i18n.t(key, { ns: 'settings' }) as string; + +function renderConfigForm(config: any) { + mockConfigData = { config, etag: 'etag-1', overriddenByEnv: {} }; + renderWithProviders(); +} + +const saveButton = () => + screen.getByRole('button', { name: /Save & Apply/ }) as HTMLButtonElement; + +const discardButton = () => + screen.getByRole('button', { name: /Discard/ }) as HTMLButtonElement; + +async function dirtyAnotherField() { + // Rename the worker id so the draft is dirty independently of extra_body. + const id = screen.getByDisplayValue('w1'); + await userEvent.clear(id); + await userEvent.type(id, 'w1-renamed'); +} + +describe('ConfigForm blocks Save & Apply while extra_body draft is invalid', () => { + it('disables Save & Apply and shows a hint while extra_body is invalid, even with another dirty field', async () => { + renderConfigForm({ llm: { workers: [{ id: 'w1', endpoint: 'http://a/v1' }] } }); + await dirtyAnotherField(); + expect(saveButton()).toBeEnabled(); + + const textarea = screen.getByLabelText('extra_body'); + await userEvent.click(textarea); + await userEvent.paste('{not valid json'); + expect(saveButton()).toBeDisabled(); + expect(screen.getByText(tSettings('configForm.invalidBlocked'))).toBeInTheDocument(); + }); + + it('re-enables Save & Apply once the JSON is fixed', async () => { + renderConfigForm({ llm: { workers: [{ id: 'w1', endpoint: 'http://a/v1' }] } }); + await dirtyAnotherField(); + + const textarea = screen.getByLabelText('extra_body'); + await userEvent.click(textarea); + await userEvent.paste('{not valid json'); + expect(saveButton()).toBeDisabled(); + + await userEvent.clear(textarea); + await userEvent.click(textarea); + await userEvent.paste('{"reasoning_effort":"high"}'); + expect(saveButton()).toBeEnabled(); + expect(screen.queryByText(tSettings('configForm.invalidBlocked'))).toBeNull(); + }); + + it('clears the invalid flag when the offending worker row is removed (no stale key bricking Save)', async () => { + renderConfigForm({ llm: { workers: [{ id: 'w1', endpoint: 'http://a/v1' }] } }); + await dirtyAnotherField(); + + const textarea = screen.getByLabelText('extra_body'); + await userEvent.click(textarea); + await userEvent.paste('{not valid json'); + expect(saveButton()).toBeDisabled(); + + await userEvent.click(screen.getByTitle(tSettings('llmWorkers.removeWorker'))); + // Row unmounted → its invalid flag must be cleared; the draft is still + // dirty (worker array changed), so Save must be clickable again. + expect(saveButton()).toBeEnabled(); + }); +}); + +/** + * Multi-worker index-reuse regression (Task 6 P2 codex finding). + * + * With an array-index React `key` / validity fieldKey, deleting/reordering a + * row causes React to reuse the row-0 component instance for a DIFFERENT + * worker. `ExtraBodyField`'s local draft/error only re-syncs from the + * `value` prop when that prop differs from what the field itself last + * emitted — and an invalid draft is, by design, never emitted (see the + * ExtraBodyField doc comment), so `lastEmitted` stays at its pre-edit value. + * When the surviving worker's `extraBody` happens to equal that same + * pre-edit value (e.g. both undefined), the re-sync guard sees no change + * and the stale invalid text "ghosts" onto the wrong row — and/or the + * invalid-flag key never gets a matching unmount, bricking Save even after + * the offending worker is gone. Keying by a stable per-row uid (see + * `uidsRef` in LlmWorkersForm) fixes this because React then unmounts the + * removed row's own component instead of reusing it for its neighbour. + */ +describe('ConfigForm extra_body identity survives row delete/reorder (index-key regression)', () => { + function twoWorkers() { + return { + llm: { + workers: [ + { id: 'w1', endpoint: 'http://a/v1' }, + { id: 'w2', endpoint: 'http://b/v1' }, + ], + }, + }; + } + + it('(a) deleting the invalid row 0 re-enables Save and leaves no ghost text on the surviving row', async () => { + renderConfigForm(twoWorkers()); + + const textareaBefore = screen.getAllByLabelText('extra_body'); + expect(textareaBefore).toHaveLength(2); + await userEvent.click(textareaBefore[0]); + await userEvent.paste('{not valid json'); + expect(saveButton()).toBeDisabled(); + + // Delete row 0 (the invalid one). Only row 1 (worker w2) should remain. + const removeButtons = screen.getAllByTitle(tSettings('llmWorkers.removeWorker')); + await userEvent.click(removeButtons[0]); + + // The surviving row is worker w2, which never had extra_body touched — + // its textarea must show ITS OWN (empty) draft, not the invalid text + // that was typed into the now-deleted row 0. + const textareaAfter = screen.getAllByLabelText('extra_body'); + expect(textareaAfter).toHaveLength(1); + expect(textareaAfter[0]).toHaveValue(''); + expect(screen.queryByText(tSettings('llmWorkers.extraBodyInvalid'))).toBeNull(); + + // The offending row's invalid flag must be cleared on its unmount — + // Save is re-enabled (the worker-array delete itself is a dirty edit). + expect(saveButton()).toBeEnabled(); + }); + + it('(b) deleting row 1 leaves row 0’s invalid draft and blocked Save untouched', async () => { + renderConfigForm(twoWorkers()); + + const textareas = screen.getAllByLabelText('extra_body'); + await userEvent.click(textareas[0]); + await userEvent.paste('{not valid json'); + expect(saveButton()).toBeDisabled(); + + const removeButtons = screen.getAllByTitle(tSettings('llmWorkers.removeWorker')); + await userEvent.click(removeButtons[1]); + + const textareaAfter = screen.getAllByLabelText('extra_body'); + expect(textareaAfter).toHaveLength(1); + expect(textareaAfter[0]).toHaveValue('{not valid json'); + expect(screen.getByText(tSettings('llmWorkers.extraBodyInvalid'))).toBeInTheDocument(); + expect(saveButton()).toBeDisabled(); + }); + + it('(c) moving the invalid row down carries its draft to the new position', async () => { + renderConfigForm(twoWorkers()); + + const textareas = screen.getAllByLabelText('extra_body'); + await userEvent.click(textareas[0]); + await userEvent.paste('{not valid json'); + expect(saveButton()).toBeDisabled(); + + const moveDownButtons = screen.getAllByTitle(tSettings('llmWorkers.moveDown')); + await userEvent.click(moveDownButtons[0]); + + const textareasAfter = screen.getAllByLabelText('extra_body'); + expect(textareasAfter).toHaveLength(2); + // The invalid draft followed worker w1 to row index 1; row 0 (now w2) + // shows its own empty draft. + expect(textareasAfter[0]).toHaveValue(''); + expect(textareasAfter[1]).toHaveValue('{not valid json'); + expect(screen.getByText(tSettings('llmWorkers.extraBodyInvalid'))).toBeInTheDocument(); + expect(saveButton()).toBeDisabled(); + }); +}); + +/** + * Discard Changes / external-refetch stuck-invalid regression (Task 6 P2 + * codex finding, reviewer follow-up to the fix above). + * + * `ExtraBodyField` only re-syncs its local draft+error from the `value` + * prop when that prop differs from what the field itself last emitted + * (`lastEmitted.current`). An invalid draft is by design never emitted, so + * `value` (and `lastEmitted.current`) both stay at their pre-edit value + * (often `undefined`) the whole time the operator is typing bad JSON. If + * the operator then discards (or the draft gets reset out from under them + * by an external refetch) WITHOUT the value prop changing, the field never + * notices — its red frame + error text persist, and `invalidKeys` in + * ConfigForm keeps the phantom key forever, since neither side has a + * reason to clear it. Save & Apply stays permanently blocked until a full + * page reload, even though the "invalid" draft has already been thrown + * away. + */ +describe('ConfigForm clears phantom invalid state on Discard / external refetch (Task 6 P2 fix)', () => { + it('BUG: Discard Changes with a dirty OTHER field must clear the phantom invalid extra_body state', async () => { + renderConfigForm({ llm: { workers: [{ id: 'w1', endpoint: 'http://a/v1' }] } }); + await dirtyAnotherField(); + + const textarea = screen.getByLabelText('extra_body'); + await userEvent.click(textarea); + await userEvent.paste('{not valid json'); + expect(screen.getByText(tSettings('llmWorkers.extraBodyInvalid'))).toBeInTheDocument(); + expect(saveButton()).toBeDisabled(); + + await userEvent.click(discardButton()); + + // The reported bug: without the fix, the red error text and the + // invalidBlocked hint both survive discard, and Save stays disabled + // forever — even after the operator makes an unrelated, perfectly + // valid edit. + expect(screen.queryByText(tSettings('llmWorkers.extraBodyInvalid'))).toBeNull(); + expect(screen.queryByText(tSettings('configForm.invalidBlocked'))).toBeNull(); + + const id = screen.getByDisplayValue('w1'); + await userEvent.clear(id); + await userEvent.type(id, 'w1-again'); + expect(saveButton()).toBeEnabled(); + }); + + it('discard with a VALID in-progress extra_body edit still reverts to the original value (no regression)', async () => { + renderConfigForm({ + llm: { workers: [{ id: 'w1', endpoint: 'http://a/v1', extraBody: { reasoning_effort: 'low' } }] }, + }); + await dirtyAnotherField(); + + const textarea = screen.getByLabelText('extra_body'); + await userEvent.clear(textarea); + await userEvent.click(textarea); + await userEvent.paste('{"reasoning_effort":"high"}'); + expect(screen.getByLabelText('extra_body')).toHaveValue('{"reasoning_effort":"high"}'); + + await userEvent.click(discardButton()); + + expect(screen.getByLabelText('extra_body')).toHaveValue( + JSON.stringify({ reasoning_effort: 'low' }, null, 2), + ); + // Fully reverted (the id rename was discarded too) → nothing left dirty. + expect(saveButton()).toBeDisabled(); + expect(discardButton()).toBeDisabled(); + }); + + it('an external data refetch (e.g. a save-conflict reload) also clears a phantom invalid extra_body key', async () => { + const original = { llm: { workers: [{ id: 'w1', endpoint: 'http://a/v1' }] } }; + mockConfigData = { config: original, etag: 'etag-1', overriddenByEnv: {} }; + const { rerender } = renderWithProviders(); + + const textarea = screen.getByLabelText('extra_body'); + await userEvent.click(textarea); + await userEvent.paste('{not valid json'); + expect(screen.getByText(tSettings('llmWorkers.extraBodyInvalid'))).toBeInTheDocument(); + + // Simulate the [data] sync effect's refetch path: a fresh `data` object + // (new reference, as react-query produces after `refetch()`) replaces + // the draft without the operator clicking anything. + mockConfigData = { config: original, etag: 'etag-2', overriddenByEnv: {} }; + rerender(); + + expect(screen.queryByText(tSettings('llmWorkers.extraBodyInvalid'))).toBeNull(); + expect(screen.getByLabelText('extra_body')).toHaveValue(''); + + const id = screen.getByDisplayValue('w1'); + await userEvent.clear(id); + await userEvent.type(id, 'w1-renamed'); + expect(saveButton()).toBeEnabled(); + }); +}); diff --git a/ui/src/components/settings/ConfigForm.tsx b/ui/src/components/settings/ConfigForm.tsx index efe66a2..93e76dc 100644 --- a/ui/src/components/settings/ConfigForm.tsx +++ b/ui/src/components/settings/ConfigForm.tsx @@ -52,6 +52,7 @@ function PreferencesFormWrapper() { user={{ defaultVisibility: auth.user.defaultVisibility ?? 'private', defaultVisibilityOrgId: auth.user.defaultVisibilityOrgId ?? null, + hasLocalCredential: auth.user.hasLocalCredential, }} /> ); @@ -161,14 +162,39 @@ function ConfigFormInner({ section }: ConfigFormProps) { const [saving, setSaving] = useState(false); const [toast, setToast] = useState(null); const [toastIsError, setToastIsError] = useState(false); + // Field keys currently flagged invalid via SectionFormProps.onValidityChange. + // While non-empty, Save & Apply is disabled: an invalid draft never reaches + // the config draft (the field withholds onChange), so allowing a save from + // another dirty field would silently discard the invalid field's edit while + // looking saved. Reporting components clear their key on unmount (see the + // contract on SectionFormProps), so section switches / row removals cannot + // leave Save bricked by a stale key. + const [invalidKeys, setInvalidKeys] = useState>(new Set()); + // Bumped whenever the draft is replaced wholesale out from under any + // in-progress local field state (Discard Changes, or a fresh `data` + // load/refetch below). Section forms with local "in-progress draft" + // state that doesn't purely derive from the `value` prop (see + // ExtraBodyField in LlmWorkersForm) fold this into their row keys to + // force a remount — see SectionFormProps.resetToken for the contract. + const [resetToken, setResetToken] = useState(0); - // Sync fetched config into draft + // Sync fetched config into draft. This effect only re-runs when the + // `data` object identity changes — i.e. the initial load and any + // subsequent refetch (e.g. after a save-conflict reload) — never on + // every render, so bumping resetToken here only fires on a genuine + // external replacement of the draft, matching the Discard Changes case + // below. Also clears invalidKeys: any field-local invalid draft that + // hasn't reached this fresh `data` (and therefore never reached the + // server) is being discarded along with the rest of the draft, so a + // phantom invalid key must not survive to permanently block Save. useEffect(() => { if (data) { setDraft(data.config); setEtag(data.etag); setOverriddenByEnv(data.overriddenByEnv); setIsDirty(false); + setInvalidKeys(new Set()); + setResetToken(t => t + 1); } }, [data]); @@ -177,10 +203,27 @@ function ConfigFormInner({ section }: ConfigFormProps) { setIsDirty(true); }, []); + const handleValidityChange = useCallback((fieldKey: string, valid: boolean) => { + setInvalidKeys(prev => { + if (valid ? !prev.has(fieldKey) : prev.has(fieldKey)) return prev; // no-op → keep identity + const next = new Set(prev); + if (valid) next.delete(fieldKey); + else next.add(fieldKey); + return next; + }); + }, []); + const handleDiscard = () => { if (data) { setDraft(data.config); setIsDirty(false); + // Discard doesn't change `data`, so the [data] sync effect above + // won't fire — clear any phantom invalid key and bump resetToken + // here directly so field-local draft state (e.g. ExtraBodyField's + // in-progress textarea + JSON error) remounts from the reverted + // value instead of getting stuck showing a stale error forever. + setInvalidKeys(new Set()); + setResetToken(t => t + 1); } }; @@ -218,7 +261,7 @@ function ConfigFormInner({ section }: ConfigFormProps) { if (error) return
{t('configForm.loadError')}
; if (!draft) return null; - const formProps = { config: draft, onChange: handleChange, overriddenByEnv }; + const formProps = { config: draft, onChange: handleChange, overriddenByEnv, onValidityChange: handleValidityChange, resetToken }; const sectionForm = (() => { switch (section) { @@ -290,6 +333,7 @@ function ConfigFormInner({ section }: ConfigFormProps) { })(); const dirty = dirtyCount > 0; + const blockedByInvalid = invalidKeys.size > 0; return (
@@ -309,6 +353,11 @@ function ConfigFormInner({ section }: ConfigFormProps) { {toast} + ) : blockedByInvalid ? ( + + + {t('configForm.invalidBlocked')} + ) : dirty ? ( @@ -328,7 +377,7 @@ function ConfigFormInner({ section }: ConfigFormProps) {