fix(coding): remediate data service review findings

2026-08-27 00:49:59 +08:00
parent 239e20d2cf
commit 38843e091f
15 changed files with 449 additions and 194 deletions
--- 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
@@ -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.
--- 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`，不访问云部署服务。
--- 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);
  }

--- a/electron/api/routes/data-service-parsers.ts
+++ 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<string, unknown> {
+  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<Record<string, unknown>> {
+  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<string, unknown>,
+  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<string, unknown> {
+  if (!isRecord(value)) {
+    return fail(422, 'invalid_request', message);
+  }
+  return value;
+}
--- 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<string, unknown> {
-  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<T>(
  status: number,
  code: string,
@@ -59,69 +65,28 @@ function methodNotAllowed<T>(): DataServiceHostResult<T> {
  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<Record<string, unknown>> {
-  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<string, unknown>, 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<string, unknown> {
-  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<T>(res: ServerResponse, result: DataServiceHostResult<T>): void {
@@ -331,5 +271,3 @@ export async function handleDataServiceRoutes(
    return true;
  }
 }
-
-export const handleDataServiceRoute = handleDataServiceRoutes;
--- 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<string, unknown> {
-  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<T>(
  status: number,
  code: string,
@@ -144,110 +150,44 @@ function sendResult<T>(res: ServerResponse, result: DataServiceHostResult<T>): v
  }
 }

+function parserFailure(status: number, code: string, message: string): never {
+  throw new PreviewDataRouteError(status, code, message);
+}
+
 async function readBoundedJson(req: IncomingMessage): Promise<Record<string, unknown>> {
-  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<string, unknown>, 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<string, unknown> {
-  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 {
--- 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<PiReleaseExtensionPr
      throw new Error('Final packaged Pi parent did not load the materialized subagent tool');
    }
    if (childToolNames.includes('subagent') || childToolNames.some((name) => (
-      ['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');
--- 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 [
--- 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');
--- 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`，读取并直接比较返回的文档数据与刚写入的数据。

--- 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' });
--- 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');
  });
 });
--- 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']))
--- 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 });
--- 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',