diff --git a/.project-docs/30-worklog/tasks/20260827-ml09-data-service-remediation-5e7c2a91.md b/.project-docs/30-worklog/tasks/20260827-ml09-data-service-remediation-5e7c2a91.md new file mode 100644 index 0000000..58a5d0b --- /dev/null +++ b/.project-docs/30-worklog/tasks/20260827-ml09-data-service-remediation-5e7c2a91.md @@ -0,0 +1,130 @@ +# Task: ML-09 data service remediation + +## Identity + +- Task ID: 20260827-ml09-data-service-remediation-5e7c2a91 +- Mode: Feature +- Branch: codex/20260827-ml09-data-service-remediation-5e7c2a91-ml09-data-service-remediation +- Worktree: D:\Datas\OthersProjects\makelore-ml09-data-service-remediation-5e7c2a91 +- Base commit: 239e20d2cf2242d636f9d42684f76f5777d9d1ee +- Owner: codex +- Status: Ready for Integration + +## Scope + +- ML-09 repository-local remediation on the exact ML-08 client head + `239e20d2cf2242d636f9d42684f76f5777d9d1ee`. +- Resolve every accepted Standards/Spec review finding in one focused pass: + normal Pi worker Data Service tool allowlist, Pi camelCase Skill input, + loopback preview Origin acceptance, browser-generation lifecycle fencing, + README/Skill documentation, narrow parser sharing, and removal of two + unused compatibility seams. +- Keep the remediation limited to existing Data Service, Pi, Skill, preview, + routing, and tests; do not alter the unrelated Pi E2E selector failure. + +## Intent And Constraints + +- Follow the MakeLore Data Service P0 specification sections 9.4, 10.1-10.3, + 11-13 and ticket ML-09 in the accepted ticket graph. Main remains the only + Works credential owner; preview capability remains explicit, exact-Origin, + data-only, loopback-bound, and generation-bound. +- Preserve child explicit profiles, existing Agent Browser abstraction, route + auth/error/response ownership, ten public Data Service operations, and the + fixed SDK/Skill protocol. Do not add a generic route/repository/wrapper, + Firebase compatibility, retries, broad CORS, or unrelated E2E changes. +- Confirmed facts: the client coordinator has integrated ML-01 through ML-08 + and is clean at the stated base; the server contract/ADR are the canonical + cross-repository inputs. The known full-E2E Pi model-combobox timeout is + pre-existing and outside this remediation unless red evidence ties it to + this diff. Review ledger accepted eight findings: allowlist, camelCase + Skill field, loopback Origin, generation equality, README, narrow parser + sharing, unused `onLifecycle`, and unused `handleDataServiceRoute`. +- Unverified external inputs remain signed-in live acceptance, deployed server + state, and production packaging; this task cannot claim them. + +## Planning Gate + +- Result: Passed on 2026-08-27. +- Concurrent gate ran `check_project_docs.py`, then `task_context.py start` + with an occupied coordinator as source, producing the isolated worktree and + matching Git-common ownership. `status --json` verified the task ID, feature + mode, absolute worktree, branch, and exact base. +- Read the active task record, project positioning, integrated current state, + decision index, system overview, architecture/domain/evidence/reflection/ + commitment/stale indexes, client coordinator scope, and the server's + canonical Data Service spec, ticket graph, integration contract, and ADR. +- Other active Data Service coordinator scope is read-only context; it owns + the parent integration worktree, while this task owns only this remediation + worktree. ML-01 through ML-08 source worktrees are released or integrated; + no semantic file-ownership conflict is unresolved. + +## Plan + +1. Inspect current Pi tool registration, Skill assets, preview session/server, + route modules, shared route helpers, and lifecycle wiring; add red tests for + each accepted finding before the smallest corresponding implementation. +2. Implement the eight fixes without widening P0 boundaries; run affected + focused suites after each cohesive group. +3. Run full unit (including pressure), typecheck, lint, build, Electron, and + affected E2E checks available in this environment; retain any real + prerequisite or pre-existing failure as an explicit deviation. +4. Run diff/doc-drift checks, update this task record with outcome and evidence, + commit once, and complete the Task Documentation Gate. + +## Outcome + +- Completed the eight accepted ML-09 remediation findings on the exact ML-08 + client head: + - Normal Pi parent workers now default to the exact ten Data Service tools + from the shared contract; read-only and coding child profiles remain their + original explicit arrays, and the packaged child proof rejects all parent + product tools including Data Service tools. + - The bundled Skill now uses Pi's camelCase `injectProjectData: true` open + input. The raw Host HTTP snake_case adapter remains separate. + - Preview sessions accept credential-free HTTP and HTTPS loopback Origins, + including default ports, while retaining loopback, exact-origin, no-path, + and no-credentials restrictions. Lifecycle invalidation is fenced to the + session's current browser generation. + - README now names the fifth opt-in `data-service` Skill and documents its + short inspect/confirm/configure/SDK/preview read-back workflow without + duplicating the protocol. + - Identical Data Service record/body/path/query/revision/data parsers now + live in the narrow `data-service-parsers.ts` module. Each route retains its + own parser failure type/messages, authentication, response projection, and + CORS behavior. + - Removed the unused Agent Browser constructor `onLifecycle` option and the + unused `handleDataServiceRoute` alias; production `subscribeLifecycle` and + `handleDataServiceRoutes` remain. + +## Verification + +- RED: the new allowlist, Skill, Origin/generation, README, and alias + assertions failed at the unfixed base as expected. +- Focused remediation suites: 10 files, 142 passed, 2 skipped. +- Full unit and pressure suites: 188 files, 1,612 passed, 2 skipped; pressure + suite 1 passed. +- `pnpm run typecheck`: passed. +- `pnpm run lint:check`: passed with five existing warnings in + `src/pages/Home/index.tsx` and `src/pages/Makelore/index.tsx`; no errors. +- `pnpm run build`: passed, including Vite, staged Pi runtime, and Windows + Electron packaging. Existing chunk-size, optional-resource, and NSIS patch + warnings remained non-fatal. +- `pnpm run test:electron:windows`: 2 files, 4 passed. +- Affected E2E: `project-configuration-skills.spec.ts` 1 passed. The Pi chat + spec had 1 passed and 1 timeout on the known model-combobox selector; the + failure is the same pre-existing baseline issue and has no diff evidence, + so it was not changed. +- `git diff --check` and the project-docs ownership/drift checks passed. + +## Follow-ups + +- Parent integration must merge this single remediation commit into the client + coordinator branch, then run the cross-repository X-01 against real + PostgreSQL and a logged-in MakeLore preview. This task did not claim live + acceptance or server deployment state. + +## Promotion Candidates + +- None recorded; the remediation changes are repository-local and the + canonical cross-repository contract/ADR already describe the retained + boundaries. diff --git a/README.md b/README.md index 26e85f9..61e1f11 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,8 @@ Pi 正式包必须继续运行 `pnpm run verify:artifact:pi`、`pnpm run smoke:p ### 项目内置编码 Skills -- 项目随产品提供 `agent-browser`（开发浏览器）、`frontend-slides`（项目演示）、`grilling`（方案质询）和 `planning-with-files`（项目规划）四个编码 Skill。它们从 vendor-neutral 的 `resources/coding-skills/` 打包，由 Electron Main 按 Agent 选择直接加载；未选择的 Skill 不进入该 Agent 的 Pi 资源集合。 +- 项目随产品提供 `agent-browser`（开发浏览器）、`frontend-slides`（项目演示）、`grilling`（方案质询）、`planning-with-files`（项目规划）和 `data-service`（开发数据）五个编码 Skill。它们从 vendor-neutral 的 `resources/coding-skills/` 打包，由 Electron Main 按 Agent 选择直接加载；未选择的 Skill 不进入该 Agent 的 Pi 资源集合。 +- `data-service` 只在用户显式请求后触发：先检查并说明最小集合，用户确认后配置一次、复制 SDK 资产，再用本地预览执行 put/read-back；它不用于已发布作品。 - 创建项目伙伴时，`agent-browser`、`grilling` 与 `planning-with-files` 默认勾选；`frontend-slides` 作为专项能力可手动选择。用户可以在创建或维护伙伴时调整选择。最终选择写入项目 Agent 的 `skillIds`，未选择的 Skill 保持拒绝权限。 - `grilling` 会在复杂实现前逐项确认高影响决策，用户确认前不执行变更。`planning-with-files` 只在复杂、可分阶段或需要跨会话恢复的任务中使用，并把 `task_plan.md`、`findings.md` 和 `progress.md` 直接保存到当前项目根目录，不写入 Skill 安装目录、用户目录或 `.niancode/agent-planning/`。 - `frontend-slides` 只在用户准备项目展示、汇报或结题时自动调用，生成项目目录中的固定 16:9 HTML 演示和相对路径素材；它不生成 `.pptx`，不访问云部署服务。 diff --git a/electron/agent-browser/module.ts b/electron/agent-browser/module.ts index d1273e3..f444e7e 100644 --- a/electron/agent-browser/module.ts +++ b/electron/agent-browser/module.ts @@ -187,7 +187,6 @@ export interface AgentBrowserReadPayloadInput { export interface AgentBrowserModuleOptions { payloadStore?: AgentBrowserPayloadStore; cdpGuard?: AgentBrowserCdpGuard; - onLifecycle?(event: AgentBrowserLifecycleEvent): void; previewDataSession?: PreviewDataSession; } @@ -230,7 +229,6 @@ export class AgentBrowserModule { ) { this.payloadStore = options.payloadStore ?? new AgentBrowserPayloadStore(); this.cdpGuard = options.cdpGuard ?? new AgentBrowserCdpGuard(); - if (options.onLifecycle) this.lifecycleListeners.add(options.onLifecycle); if (options.previewDataSession) this.configurePreviewDataSession(options.previewDataSession); } diff --git a/electron/api/routes/data-service-parsers.ts b/electron/api/routes/data-service-parsers.ts new file mode 100644 index 0000000..0a558cd --- /dev/null +++ b/electron/api/routes/data-service-parsers.ts @@ -0,0 +1,177 @@ +import type { IncomingMessage } from 'node:http'; + +export const MAX_CURSOR_LENGTH = 1_024; +export const COLLECTION_PATTERN = /^[a-z][a-z0-9_-]{0,47}$/; +export const DOCUMENT_ID_PATTERN = /^[A-Za-z0-9._~-]{1,128}$/; + +export type ParserFailure = ( + status: number, + code: string, + message: string, +) => never; + +export type ParserMessages = Readonly<{ + invalidRequest: string; + invalidRevision: string; + requestTooLarge: string; +}>; + +export function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +export function boundedString(value: unknown, maximum: number): string | null { + if (typeof value !== 'string') return null; + return value && value.length <= maximum ? value : null; +} + +export async function readBoundedJson( + req: IncomingMessage, + maxBytes: number, + fail: ParserFailure, + messages: ParserMessages, +): Promise> { + const declared = req.headers['content-length']; + const declaredValue = Array.isArray(declared) ? declared[0] : declared; + if (declaredValue && /^\d+$/.test(declaredValue) && Number(declaredValue) > maxBytes) { + return fail(413, 'request_too_large', messages.requestTooLarge); + } + const chunks: Buffer[] = []; + let size = 0; + for await (const chunk of req) { + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + size += buffer.byteLength; + if (size > maxBytes) { + return fail(413, 'request_too_large', messages.requestTooLarge); + } + chunks.push(buffer); + } + try { + const value = JSON.parse(Buffer.concat(chunks).toString('utf8')) as unknown; + if (!isRecord(value)) throw new Error('object required'); + return value; + } catch { + return fail(422, 'invalid_request', messages.invalidRequest); + } +} + +export function requireExactKeys( + value: Record, + keys: readonly string[], + fail: ParserFailure, + message: string, +): void { + const expected = new Set(keys); + if (Object.keys(value).some((key) => !expected.has(key)) + || keys.some((key) => !Object.prototype.hasOwnProperty.call(value, key))) { + fail(422, 'invalid_request', message); + } +} + +function decodeSegment( + value: string, + maximum: number, + pattern: RegExp, + fail: ParserFailure, + message: string, +): string { + let decoded: string; + try { + decoded = decodeURIComponent(value); + } catch { + return fail(422, 'invalid_request', message); + } + if (decoded.length > maximum || !pattern.test(decoded)) { + return fail(422, 'invalid_request', message); + } + return decoded; +} + +export function parseCollection( + value: string, + fail: ParserFailure, + message: string, +): string { + return decodeSegment(value, 48, COLLECTION_PATTERN, fail, message); +} + +export function parseDocumentId( + value: string, + fail: ParserFailure, + message: string, +): string { + const documentId = decodeSegment(value, 128, DOCUMENT_ID_PATTERN, fail, message); + if (documentId === '.' || documentId === '..') { + return fail(422, 'invalid_request', message); + } + return documentId; +} + +export function requireQueryKeys( + url: URL, + allowed: readonly string[], + fail: ParserFailure, + message: string, +): void { + const accepted = new Set(allowed); + if ([...url.searchParams.keys()].some((key) => !accepted.has(key))) { + fail(422, 'invalid_request', message); + } +} + +export function parseLimit( + url: URL, + fail: ParserFailure, + message: string, +): number | undefined { + const values = url.searchParams.getAll('limit'); + if (values.length === 0) return undefined; + if (values.length !== 1 || !/^[1-9]\d*$/.test(values[0])) { + return fail(422, 'invalid_request', message); + } + const limit = Number(values[0]); + if (!Number.isSafeInteger(limit) || limit < 1 || limit > 100) { + return fail(422, 'invalid_request', message); + } + return limit; +} + +export function parseCursor( + url: URL, + fail: ParserFailure, + message: string, +): string | undefined { + const values = url.searchParams.getAll('cursor'); + if (values.length === 0) return undefined; + const cursor = values.length === 1 ? boundedString(values[0], MAX_CURSOR_LENGTH) : null; + if (!cursor) return fail(422, 'invalid_request', message); + return cursor; +} + +export function parseIfMatch( + req: IncomingMessage, + fail: ParserFailure, + message: string, +): number | undefined { + const value = req.headers['if-match']; + if (value === undefined) return undefined; + if (Array.isArray(value) || !/^"[1-9]\d*"$/.test(value)) { + return fail(422, 'invalid_revision', message); + } + const revision = Number(value.slice(1, -1)); + if (!Number.isSafeInteger(revision) || revision < 1) { + return fail(422, 'invalid_revision', message); + } + return revision; +} + +export function parseData( + value: unknown, + fail: ParserFailure, + message: string, +): Record { + if (!isRecord(value)) { + return fail(422, 'invalid_request', message); + } + return value; +} diff --git a/electron/api/routes/data-service.ts b/electron/api/routes/data-service.ts index 0968c88..488335f 100644 --- a/electron/api/routes/data-service.ts +++ b/electron/api/routes/data-service.ts @@ -5,12 +5,27 @@ import { type DataServiceErrorContext, type DataServiceHostResult, } from '../../../shared/data-service'; +import { + COLLECTION_PATTERN, + parseCollection as parseCollectionSegment, + parseCursor as parseCursorValue, + parseData as parseDataValue, + parseDocumentId as parseDocumentIdValue, + parseIfMatch as parseIfMatchValue, + parseLimit as parseLimitValue, + readBoundedJson as readBoundedJsonBody, + requireExactKeys as requireExactKeysValue, + requireQueryKeys as requireQueryKeysValue, + type ParserMessages, +} from './data-service-parsers'; const LOCAL_ROOT = '/api/works/data-service'; const MAX_REQUEST_BYTES = 98_304; -const MAX_CURSOR_LENGTH = 1_024; -const COLLECTION_PATTERN = /^[a-z][a-z0-9_-]{0,47}$/; -const DOCUMENT_ID_PATTERN = /^[A-Za-z0-9._~-]{1,128}$/; +const PARSER_MESSAGES: ParserMessages = { + invalidRequest: 'Data Service request is invalid', + invalidRevision: 'Data Service document revision is invalid', + requestTooLarge: 'Data Service request is too large', +}; class DataServiceRouteError extends Error { constructor( @@ -23,15 +38,6 @@ class DataServiceRouteError extends Error { } } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - -function boundedString(value: unknown, maximum: number): string | null { - if (typeof value !== 'string') return null; - return value && value.length <= maximum ? value : null; -} - function routeFailure( status: number, code: string, @@ -59,69 +65,28 @@ function methodNotAllowed(): DataServiceHostResult { return routeFailure(405, 'method_not_allowed', 'Data Service method is not allowed'); } +function parserFailure(status: number, code: string, message: string): never { + throw new DataServiceRouteError(status, code, message); +} + async function readBoundedJson(req: IncomingMessage): Promise> { - const declared = req.headers['content-length']; - const declaredValue = Array.isArray(declared) ? declared[0] : declared; - if (declaredValue && /^\d+$/.test(declaredValue) && Number(declaredValue) > MAX_REQUEST_BYTES) { - throw new DataServiceRouteError(413, 'request_too_large', 'Data Service request is too large'); - } - const chunks: Buffer[] = []; - let size = 0; - for await (const chunk of req) { - const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); - size += buffer.byteLength; - if (size > MAX_REQUEST_BYTES) { - throw new DataServiceRouteError(413, 'request_too_large', 'Data Service request is too large'); - } - chunks.push(buffer); - } - try { - const value = JSON.parse(Buffer.concat(chunks).toString('utf8')) as unknown; - if (!isRecord(value)) throw new Error('object required'); - return value; - } catch { - throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - } + return readBoundedJsonBody(req, MAX_REQUEST_BYTES, parserFailure, PARSER_MESSAGES); } function requireExactKeys(value: Record, keys: readonly string[]): void { - const expected = new Set(keys); - if (Object.keys(value).some((key) => !expected.has(key)) - || keys.some((key) => !Object.prototype.hasOwnProperty.call(value, key))) { - throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - } -} - -function decodeSegment(value: string, maximum: number, pattern: RegExp): string { - let decoded: string; - try { - decoded = decodeURIComponent(value); - } catch { - throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - } - if (decoded.length > maximum || !pattern.test(decoded)) { - throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - } - return decoded; + requireExactKeysValue(value, keys, parserFailure, PARSER_MESSAGES.invalidRequest); } function parseCollection(value: string): string { - return decodeSegment(value, 48, COLLECTION_PATTERN); + return parseCollectionSegment(value, parserFailure, PARSER_MESSAGES.invalidRequest); } function parseDocumentId(value: string): string { - const documentId = decodeSegment(value, 128, DOCUMENT_ID_PATTERN); - if (documentId === '.' || documentId === '..') { - throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - } - return documentId; + return parseDocumentIdValue(value, parserFailure, PARSER_MESSAGES.invalidRequest); } function requireQueryKeys(url: URL, allowed: readonly string[]): void { - const accepted = new Set(allowed); - if ([...url.searchParams.keys()].some((key) => !accepted.has(key))) { - throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - } + requireQueryKeysValue(url, allowed, parserFailure, PARSER_MESSAGES.invalidRequest); } function requireConfirmed(url: URL): void { @@ -137,37 +102,15 @@ function requireConfirmed(url: URL): void { } function parseLimit(url: URL): number | undefined { - const values = url.searchParams.getAll('limit'); - if (values.length === 0) return undefined; - if (values.length !== 1 || !/^[1-9]\d*$/.test(values[0])) { - throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - } - const limit = Number(values[0]); - if (!Number.isSafeInteger(limit) || limit < 1 || limit > 100) { - throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - } - return limit; + return parseLimitValue(url, parserFailure, PARSER_MESSAGES.invalidRequest); } function parseCursor(url: URL): string | undefined { - const values = url.searchParams.getAll('cursor'); - if (values.length === 0) return undefined; - const cursor = values.length === 1 ? boundedString(values[0], MAX_CURSOR_LENGTH) : null; - if (!cursor) throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - return cursor; + return parseCursorValue(url, parserFailure, PARSER_MESSAGES.invalidRequest); } function parseIfMatch(req: IncomingMessage): number | undefined { - const value = req.headers['if-match']; - if (value === undefined) return undefined; - if (Array.isArray(value) || !/^"[1-9]\d*"$/.test(value)) { - throw new DataServiceRouteError(422, 'invalid_revision', 'Data Service document revision is invalid'); - } - const revision = Number(value.slice(1, -1)); - if (!Number.isSafeInteger(revision) || revision < 1) { - throw new DataServiceRouteError(422, 'invalid_revision', 'Data Service document revision is invalid'); - } - return revision; + return parseIfMatchValue(req, parserFailure, PARSER_MESSAGES.invalidRevision); } function parseCollections(value: unknown): string[] { @@ -179,10 +122,7 @@ function parseCollections(value: unknown): string[] { } function parseData(value: unknown): Record { - if (!isRecord(value)) { - throw new DataServiceRouteError(422, 'invalid_request', 'Data Service request is invalid'); - } - return value; + return parseDataValue(value, parserFailure, PARSER_MESSAGES.invalidRequest); } function sendResult(res: ServerResponse, result: DataServiceHostResult): void { @@ -331,5 +271,3 @@ export async function handleDataServiceRoutes( return true; } } - -export const handleDataServiceRoute = handleDataServiceRoutes; diff --git a/electron/api/routes/runtime-data.ts b/electron/api/routes/runtime-data.ts index 46cfbcf..70dbb71 100644 --- a/electron/api/routes/runtime-data.ts +++ b/electron/api/routes/runtime-data.ts @@ -6,12 +6,27 @@ import type { DataServiceErrorContext, DataServiceHostResult, } from '../../../shared/data-service'; +import { + isRecord, + parseCollection as parseCollectionSegment, + parseCursor as parseCursorValue, + parseData as parseDataValue, + parseDocumentId as parseDocumentIdValue, + parseIfMatch as parseIfMatchValue, + parseLimit as parseLimitValue, + readBoundedJson as readBoundedJsonBody, + requireExactKeys as requireExactKeysValue, + requireQueryKeys as requireQueryKeysValue, + type ParserMessages, +} from './data-service-parsers'; -const MAX_CURSOR_LENGTH = 1_024; -const COLLECTION_PATTERN = /^[a-z][a-z0-9_-]{0,47}$/; -const DOCUMENT_ID_PATTERN = /^[A-Za-z0-9._~-]{1,128}$/; const CORS_METHODS = 'GET, PUT, DELETE, OPTIONS'; const CORS_HEADERS = 'Authorization, Content-Type, If-Match'; +const PARSER_MESSAGES: ParserMessages = { + invalidRequest: 'Preview data request is invalid', + invalidRevision: 'Data Service document revision is invalid', + requestTooLarge: 'Preview data request is too large', +}; class PreviewDataRouteError extends Error { constructor( @@ -25,15 +40,6 @@ class PreviewDataRouteError extends Error { } } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - -function boundedString(value: unknown, maximum: number): string | null { - if (typeof value !== 'string') return null; - return value && value.length <= maximum ? value : null; -} - function routeFailure( status: number, code: string, @@ -144,110 +150,44 @@ function sendResult(res: ServerResponse, result: DataServiceHostResult): v } } +function parserFailure(status: number, code: string, message: string): never { + throw new PreviewDataRouteError(status, code, message); +} + async function readBoundedJson(req: IncomingMessage): Promise> { - const declared = req.headers['content-length']; - const declaredValue = Array.isArray(declared) ? declared[0] : declared; - if (declaredValue && /^\d+$/.test(declaredValue) && Number(declaredValue) > PREVIEW_DATA_MAX_REQUEST_BYTES) { - throw new PreviewDataRouteError(413, 'request_too_large', 'Preview data request is too large'); - } - const chunks: Buffer[] = []; - let size = 0; - for await (const chunk of req) { - const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); - size += buffer.byteLength; - if (size > PREVIEW_DATA_MAX_REQUEST_BYTES) { - throw new PreviewDataRouteError(413, 'request_too_large', 'Preview data request is too large'); - } - chunks.push(buffer); - } - try { - const value = JSON.parse(Buffer.concat(chunks).toString('utf8')) as unknown; - if (!isRecord(value)) throw new Error('object required'); - return value; - } catch { - throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - } + return readBoundedJsonBody(req, PREVIEW_DATA_MAX_REQUEST_BYTES, parserFailure, PARSER_MESSAGES); } function requireExactKeys(value: Record, keys: readonly string[]): void { - const expected = new Set(keys); - if (Object.keys(value).some((key) => !expected.has(key)) - || keys.some((key) => !Object.prototype.hasOwnProperty.call(value, key))) { - throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - } -} - -function decodeSegment(value: string, maximum: number, pattern: RegExp): string { - let decoded: string; - try { - decoded = decodeURIComponent(value); - } catch { - throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - } - if (decoded.length > maximum || !pattern.test(decoded)) { - throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - } - return decoded; + requireExactKeysValue(value, keys, parserFailure, PARSER_MESSAGES.invalidRequest); } function parseCollection(value: string): string { - return decodeSegment(value, 48, COLLECTION_PATTERN); + return parseCollectionSegment(value, parserFailure, PARSER_MESSAGES.invalidRequest); } function parseDocumentId(value: string): string { - const documentId = decodeSegment(value, 128, DOCUMENT_ID_PATTERN); - if (documentId === '.' || documentId === '..') { - throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - } - return documentId; + return parseDocumentIdValue(value, parserFailure, PARSER_MESSAGES.invalidRequest); } function requireQueryKeys(url: URL, allowed: readonly string[]): void { - const accepted = new Set(allowed); - if ([...url.searchParams.keys()].some((key) => !accepted.has(key))) { - throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - } + requireQueryKeysValue(url, allowed, parserFailure, PARSER_MESSAGES.invalidRequest); } function parseLimit(url: URL): number | undefined { - const values = url.searchParams.getAll('limit'); - if (values.length === 0) return undefined; - if (values.length !== 1 || !/^[1-9]\d*$/.test(values[0])) { - throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - } - const limit = Number(values[0]); - if (!Number.isSafeInteger(limit) || limit < 1 || limit > 100) { - throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - } - return limit; + return parseLimitValue(url, parserFailure, PARSER_MESSAGES.invalidRequest); } function parseCursor(url: URL): string | undefined { - const values = url.searchParams.getAll('cursor'); - if (values.length === 0) return undefined; - const cursor = values.length === 1 ? boundedString(values[0], MAX_CURSOR_LENGTH) : null; - if (!cursor) throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - return cursor; + return parseCursorValue(url, parserFailure, PARSER_MESSAGES.invalidRequest); } function parseIfMatch(req: IncomingMessage): number | undefined { - const value = req.headers['if-match']; - if (value === undefined) return undefined; - if (Array.isArray(value) || !/^"[1-9]\d*"$/.test(value)) { - throw new PreviewDataRouteError(422, 'invalid_revision', 'Data Service document revision is invalid'); - } - const revision = Number(value.slice(1, -1)); - if (!Number.isSafeInteger(revision) || revision < 1) { - throw new PreviewDataRouteError(422, 'invalid_revision', 'Data Service document revision is invalid'); - } - return revision; + return parseIfMatchValue(req, parserFailure, PARSER_MESSAGES.invalidRevision); } function parseData(value: unknown): Record { - if (!isRecord(value)) { - throw new PreviewDataRouteError(422, 'invalid_request', 'Preview data request is invalid'); - } - return value; + return parseDataValue(value, parserFailure, PARSER_MESSAGES.invalidRequest); } function requireJsonContentType(req: IncomingMessage): void { diff --git a/electron/coding-runtime/pi/release-proof.ts b/electron/coding-runtime/pi/release-proof.ts index 23bd3e0..34a0af7 100644 --- a/electron/coding-runtime/pi/release-proof.ts +++ b/electron/coding-runtime/pi/release-proof.ts @@ -1347,7 +1347,14 @@ export async function runFinalAsarExtensionProof(): Promise ( - ['ask_user', 'agent_browser', 'game_asset_browser', 'game_asset_review', 'task_state', 'changed_file', 'runtime_context'] + [ + 'ask_user', 'agent_browser', 'game_asset_browser', 'game_asset_review', + 'task_state', 'changed_file', 'runtime_context', + 'data_service_configure', 'data_service_inspect', 'data_service_list_projects', + 'data_service_get_document', 'data_service_list_documents', 'data_service_put_document', + 'data_service_delete_document', 'data_service_remove_collection', 'data_service_reset', + 'data_service_remove_project', + ] .includes(name) ))) { throw new Error('Final packaged Pi child exposed parent-only product tools'); diff --git a/electron/coding-runtime/pi/worker-process.ts b/electron/coding-runtime/pi/worker-process.ts index 14abde7..ac2b2e2 100644 --- a/electron/coding-runtime/pi/worker-process.ts +++ b/electron/coding-runtime/pi/worker-process.ts @@ -4,6 +4,7 @@ import { type ChildProcessWithoutNullStreams, } from 'node:child_process'; import { platform } from 'node:os'; +import { DATA_SERVICE_PI_TOOL_NAMES } from '../../../shared/data-service'; import { logger } from '../../utils/logger'; import type { CodingRuntimeDisposeReason } from '../contracts'; import { PiProcessError, type PiProcessErrorCode } from './process-errors'; @@ -128,6 +129,7 @@ export function buildPiRpcArgs( 'read', 'bash', 'edit', 'write', 'grep', 'find', 'ls', 'ask_user', 'subagent', 'agent_browser', 'game_asset_browser', 'game_asset_review', 'task_state', 'changed_file', 'runtime_context', + ...DATA_SERVICE_PI_TOOL_NAMES, ], ): string[] { return [ diff --git a/electron/services/preview-data-session.ts b/electron/services/preview-data-session.ts index 856454c..3901886 100644 --- a/electron/services/preview-data-session.ts +++ b/electron/services/preview-data-session.ts @@ -156,8 +156,7 @@ function normalizeOrigin(value: string): string { || hostname === '[::1]'; if ( !isLoopback - || parsed.protocol !== 'http:' - || !parsed.port + || (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') || parsed.username || parsed.password || parsed.pathname !== '/' @@ -402,6 +401,7 @@ export class PreviewDataSessionManager { handleAgentBrowserLifecycle(event: AgentBrowserLifecycleEvent): void { const session = this.session; if (!session || !samePath(session.snapshot.projectPath, event.projectPath)) return; + if (event.generation !== session.snapshot.browserGeneration) return; switch (event.type) { case 'closed': this.invalidate('preview_closed'); diff --git a/resources/coding-skills/data-service/SKILL.md b/resources/coding-skills/data-service/SKILL.md index d896e4b..06e6b07 100644 --- a/resources/coding-skills/data-service/SKILL.md +++ b/resources/coding-skills/data-service/SKILL.md @@ -63,8 +63,8 @@ JavaScript。 ## 5. Preview verification -用 `agent_browser` 打开当前项目的 data-enabled preview，要求其使用 -`inject_project_data: true` 的本地预览能力；不要在外部浏览器、发布运行时或 +用 `agent_browser` 的 `open` 输入 `{ action: "open", url: "<本地预览地址>", injectProjectData: true }` +打开当前项目的 data-enabled preview；不要在外部浏览器、发布运行时或 通用 Host 路径中寻找替代能力。实际运行最小示例，先执行一次真实 `put`，再 用返回的文档标识执行 `get`，读取并直接比较返回的文档数据与刚写入的数据。 diff --git a/tests/unit/data-service-sdk-assets.test.ts b/tests/unit/data-service-sdk-assets.test.ts index 40b828a..274d616 100644 --- a/tests/unit/data-service-sdk-assets.test.ts +++ b/tests/unit/data-service-sdk-assets.test.ts @@ -237,6 +237,7 @@ describe.each(ASSETS)('generated Data Service SDK (%s)', (assetName) => { describe('bundled Data Service Skill packaging', () => { it('ships both canonical assets and an ordered workflow with explicit completion criteria', async () => { const skill = await readFile(path.resolve('resources/coding-skills/data-service/SKILL.md'), 'utf8'); + const readme = await readFile(path.resolve('README.md'), 'utf8'); const tsAsset = await readFile(path.join(ASSET_ROOT, 'makelore-data.ts'), 'utf8'); const jsAsset = await readFile(path.join(ASSET_ROOT, 'makelore-data.js'), 'utf8'); @@ -254,9 +255,14 @@ describe('bundled Data Service Skill packaging', () => { expect(skill.indexOf('read-back')).toBeLessThan(skill.indexOf('report')); expect(skill).toContain('src/lib/makelore-data.js'); expect(skill).toContain('makelore-data.js'); + expect(skill).toContain('injectProjectData: true'); + expect(skill).not.toContain('inject_project_data'); expect(skill).toContain('逐字复制'); expect(skill).toContain('no-op'); expect(skill).toContain('冲突'); + expect(readme).toContain('data-service'); + expect(readme).toContain('显式请求'); + expect(readme).toContain('预览'); const packaged = await listProductCodingSkills(path.resolve('resources/coding-skills')); const dataService = packaged.find(({ id }) => id === 'data-service'); expect(dataService).toMatchObject({ id: 'data-service', name: 'data-service' }); diff --git a/tests/unit/data-service-server-registration.test.ts b/tests/unit/data-service-server-registration.test.ts index 399e548..fdafc6a 100644 --- a/tests/unit/data-service-server-registration.test.ts +++ b/tests/unit/data-service-server-registration.test.ts @@ -14,5 +14,6 @@ describe('Data Service Host API registration', () => { const worksIndex = routeList?.[1].indexOf('handleWorksRoutes') ?? -1; expect(dataServiceIndex).toBeGreaterThanOrEqual(0); expect(worksIndex).toBeGreaterThan(dataServiceIndex); + expect(source).not.toContain('handleDataServiceRoute = handleDataServiceRoutes'); }); }); diff --git a/tests/unit/pi-rpc-foundation.test.ts b/tests/unit/pi-rpc-foundation.test.ts index d6837da..6ef8cbb 100644 --- a/tests/unit/pi-rpc-foundation.test.ts +++ b/tests/unit/pi-rpc-foundation.test.ts @@ -244,7 +244,7 @@ describe('Pi worker process', () => { '--no-context-files', '--no-approve', '--tools', - 'read,bash,edit,write,grep,find,ls,ask_user,subagent,agent_browser,game_asset_browser,game_asset_review,task_state,changed_file,runtime_context', + 'read,bash,edit,write,grep,find,ls,ask_user,subagent,agent_browser,game_asset_browser,game_asset_review,task_state,changed_file,runtime_context,data_service_configure,data_service_inspect,data_service_list_projects,data_service_get_document,data_service_list_documents,data_service_put_document,data_service_delete_document,data_service_remove_collection,data_service_reset,data_service_remove_project', '--model', 'model-a', ]); expect(buildPiRpcArgs('sessions', ['--no-session'], ['read', 'grep', 'find', 'ls'])) diff --git a/tests/unit/pi-worker-process-real.test.ts b/tests/unit/pi-worker-process-real.test.ts index 897b598..637db21 100644 --- a/tests/unit/pi-worker-process-real.test.ts +++ b/tests/unit/pi-worker-process-real.test.ts @@ -292,6 +292,16 @@ describe('locked Pi worker process smoke', () => { 'task_state', 'changed_file', 'runtime_context', + 'data_service_configure', + 'data_service_inspect', + 'data_service_list_projects', + 'data_service_get_document', + 'data_service_list_documents', + 'data_service_put_document', + 'data_service_delete_document', + 'data_service_remove_collection', + 'data_service_reset', + 'data_service_remove_project', ])); expect(worker.stderrDiagnostic).not.toContain('Failed to load extension'); await expect(worker.stop('test_injection')).resolves.toMatchObject({ mode: 'stdin-close', code: 0 }); diff --git a/tests/unit/preview-data-session.test.ts b/tests/unit/preview-data-session.test.ts index 8f5855d..ab27e35 100644 --- a/tests/unit/preview-data-session.test.ts +++ b/tests/unit/preview-data-session.test.ts @@ -127,13 +127,58 @@ describe('preview data session manager', () => { expect(manager.getSnapshot()).toBeNull(); }); - it('requires an HTTP loopback Origin and invalidates on browser lifecycle events', async () => { + it('accepts HTTP/HTTPS loopback Origins, including their default ports', async () => { const { manager, projectPath } = await createManager(); + for (const origin of [ + 'http://localhost', + 'http://localhost:80', + 'https://localhost', + 'https://localhost:443', + 'http://127.0.0.1', + 'https://127.0.0.1:443', + 'http://[::1]', + 'https://[::1]:443', + ]) { + await expect(manager.open({ projectPath, origin, browserGeneration: 1 })) + .resolves.toMatchObject({ origin: new URL(origin).origin }); + } await expect(manager.open({ projectPath, origin: 'https://example.com:443', browserGeneration: 1, })).rejects.toMatchObject({ code: 'invalid_origin' }); + await expect(manager.open({ + projectPath, + origin: 'http://localhost:80/path', + browserGeneration: 1, + })).rejects.toMatchObject({ code: 'invalid_origin' }); + }); + + it('requires matching browser generation before invalidating lifecycle events', async () => { + const { manager, projectPath } = await createManager(); + await manager.open({ projectPath, origin: 'http://127.0.0.1:13210', browserGeneration: 2 }); + + manager.handleAgentBrowserLifecycle({ + type: 'closed', + projectId: '11111111-1111-4111-8111-111111111111', + projectPath, + generation: 1, + url: 'http://127.0.0.1:13210/', + }); + expect(manager.getSnapshot()).not.toBeNull(); + + manager.handleAgentBrowserLifecycle({ + type: 'closed', + projectId: '11111111-1111-4111-8111-111111111111', + projectPath, + generation: 2, + url: 'http://127.0.0.1:13210/', + }); + expect(manager.getSnapshot()).toBeNull(); + }); + + it('invalidates on browser lifecycle events from the current generation', async () => { + const { manager, projectPath } = await createManager(); const lifecycleTypes: AgentBrowserLifecycleEvent['type'][] = [ 'cross-origin-navigation',