fix: accept compressed Robot revision ETags
This commit is contained in:
@@ -0,0 +1,99 @@
|
||||
# Task: Fix Robot configuration response validation
|
||||
|
||||
## Identity
|
||||
|
||||
- Task ID: 20260815-robot-config-schema-8e2c41
|
||||
- Mode: Feature
|
||||
- Branch: codex/20260815-robot-config-schema-8e2c41-robot-config-schema
|
||||
- Worktree: D:\Datas\OthersProjects\makelore-robot-config-schema-8e2c41
|
||||
- Base commit: a26a53a7f4b1326be3084955029a65ef1f79929a
|
||||
- Owner: codex
|
||||
- Status: Ready for Integration
|
||||
|
||||
## Scope
|
||||
|
||||
- Reproduce the production Robot configuration read failure with the exact safe response shape and field lengths reported by Works Square logs.
|
||||
- Identify the precise Main or Renderer validation rule that rejects a successful versioned configuration response.
|
||||
- Align the client contract with the already deployed Xiaozhi/Works Square public schema without weakening exact-key validation, bounds, ETag/revision equality, authentication, or error redaction.
|
||||
- Add regression coverage at the rejecting seam and the user-visible typed API/page path.
|
||||
|
||||
## Intent And Constraints
|
||||
|
||||
- Treat the upstream `200 application/json`, numeric revision, and Works `200` as confirmed evidence that the failure is downstream client validation, not device binding or authentication. The canonical strong ETag is confirmed only on the Xiaozhi-to-Works hop; the deployed compressed Works response was independently observed as canonical weak `W/"0"`.
|
||||
- Use only safe field names, types, and lengths from the supplied logs; do not capture tokens, raw prompts, agent/device identifiers, or raw response bodies in tests or task records.
|
||||
- Preserve strict DTO projection and bounded field validation. Change only rules proven inconsistent with the server contract.
|
||||
- Keep Renderer free of credentials/raw upstream responses and preserve Main ownership of Works Square access and ETag handling.
|
||||
- Work only in the isolated linked worktree; do not alter or release the existing `main` integration owner.
|
||||
|
||||
## Project Context Loaded
|
||||
|
||||
Task context:
|
||||
- Task ID: `20260815-robot-config-schema-8e2c41`
|
||||
- Mode: `feature`
|
||||
- Branch: `codex/20260815-robot-config-schema-8e2c41-robot-config-schema`
|
||||
- Worktree: `D:\Datas\OthersProjects\makelore-robot-config-schema-8e2c41`
|
||||
- Base commit: `a26a53a7f4b1326be3084955029a65ef1f79929a`
|
||||
- Other active local tasks: the existing `main` integration owner plus isolated release, packaging, AI Design, AI Programming, and completed Robot tasks returned by the registry.
|
||||
- Overlap or semantic-conflict assessment: the immediately preceding Robot loading task is completed and integrated, and establishes the relevant error/retry and timeout behavior. No active peer owns the Robot response-schema validators. Packaging tasks are read/build-only snapshots; AI Design and AI Programming scopes do not overlap this contract fix.
|
||||
|
||||
Read:
|
||||
- `.project-docs/05-agent-entry/read-before-planning.md`
|
||||
- `.project-docs/05-agent-entry/planning-gate.md`
|
||||
- `.project-docs/05-agent-entry/memory-index.md`
|
||||
- this task record
|
||||
- `.project-docs/00-brief/project-positioning.md`
|
||||
- `.project-docs/00-brief/success-criteria.md`
|
||||
- `.project-docs/30-worklog/current-state.md`
|
||||
- `.project-docs/10-decisions/decision-index.md`
|
||||
- `.project-docs/20-architecture/system-overview.md`
|
||||
- `.project-docs/20-architecture/module-map.md`
|
||||
- `.project-docs/20-architecture/data-flow.md`
|
||||
- `.project-docs/40-domain/business-rules.md`
|
||||
- evidence, reflection, commitment, and stale-item indexes
|
||||
- every registered peer task record's Scope, Intent And Constraints, and Promotion Candidates sections
|
||||
|
||||
Relevant understanding:
|
||||
- Project goal: keep a non-technical desktop workflow while Electron Main owns credentials, upstream network access, revisions, and safe response projection.
|
||||
- Current integrated focus: Robot is the single hardware module; the previous fix now exposes a terminal configuration error instead of an endless spinner.
|
||||
- Active task scope: correct the strict client response contract that rejects a server-successful agent configuration.
|
||||
- Active constraints: do not bypass validation or enable editing without a real configuration plus revision; do not expose raw prompt/response data.
|
||||
- Decisions affecting this task: one account maps to one Xiaozhi binding; agents/devices are subordinate resources; Main owns auth, idempotency, and ETag conversion.
|
||||
- Evidence, reflections, or commitments affecting this task: production logs prove both upstream and Works endpoints return 200, numeric revision, and a complete field inventory. A local Electron diagnostic using the existing OS-encrypted session fetched the deployed Works endpoint without printing secrets or configuration values and confirmed `content-encoding`, `ETag: W/"0"`, `config_revision: 0`, `chat_history_conf: 0`, and the exact expected snake_case key set. Real deployment smoke remains necessary after the client fix.
|
||||
- Files or modules likely involved: `electron/api/routes/ai-hardware.ts`, `src/lib/ai-hardware.ts`, and their focused tests.
|
||||
- Unknowns, stale docs, or conflicts: the exact edge component that weakens the origin's strong ETag while compressing the response is not represented in this repository. The client can safely recover the numeric application revision from a strictly canonical weak tag only when it matches the typed DTO revision; no canonical memory conflict was found.
|
||||
|
||||
Gate result:
|
||||
- Passed.
|
||||
|
||||
## Outcome
|
||||
|
||||
- Confirmed the deployed Works Square response was not the same as the origin-hop log: the JSON DTO and numeric revision were valid, while response compression changed the version validator to canonical weak `ETag: W/"0"`.
|
||||
- Updated the Electron Main AI hardware route to accept either canonical strong `"n"` or canonical weak `W/"n"` revision tags. Noncanonical values, negative/decimal values, leading zeroes, non-numeric values, and unsafe integers remain rejected.
|
||||
- Preserved optimistic concurrency: the parsed ETag revision must equal the strictly projected DTO revision, and PATCH/PUT continue to send a strong `If-Match: "n"` header.
|
||||
- Added regression coverage across configuration GET/PATCH and assignment GET/PUT, including weak-tag mismatch and invalid-tag rejection.
|
||||
|
||||
## Verification
|
||||
|
||||
- TDD red: the new deployed-shape cases failed before the implementation with `AI_HARDWARE_INVALID_ETAG` (`2 failed, 23 passed`).
|
||||
- TDD green: `tests/unit/ai-hardware-routes.test.ts` passed `25/25`.
|
||||
- Integrated focused suite passed `7 files / 78 tests`:
|
||||
- `tests/unit/ai-hardware-routes.test.ts`
|
||||
- `tests/unit/ai-hardware-api.test.ts`
|
||||
- `tests/unit/ai-hardware-page.test.tsx`
|
||||
- `tests/unit/host-api-proxy.test.ts`
|
||||
- `tests/unit/ai-hardware-server-registration.test.ts`
|
||||
- `tests/unit/module-navigation.test.tsx`
|
||||
- `tests/unit/main-layout-module-gate.test.tsx`
|
||||
- `corepack pnpm run typecheck` passed.
|
||||
- ESLint passed for both changed TypeScript files.
|
||||
- `git diff --check` passed; Git emitted only line-ending conversion warnings.
|
||||
- Final read-only Sol reviewer returned `PASS` after independently checking the diff and rerunning the 7-file/78-test suite, typecheck, ESLint, doc drift, and registry doctor.
|
||||
|
||||
## Follow-ups
|
||||
|
||||
- Rebuild/redeploy Makelore and repeat the real signed-in Robot configuration read/edit flow through the compressed public Works Square endpoint.
|
||||
- The exact edge component responsible for the strong-to-weak ETag transformation is outside this repository; retaining this strict compatibility prevents future compression changes from breaking the semantic revision flow.
|
||||
|
||||
## Promotion Candidates
|
||||
|
||||
- Target: AI hardware client contract documentation. Proposal: record that versioned Works responses may arrive with canonical weak numeric ETags after content transformation; Electron Main normalizes only the numeric application revision, requires DTO equality, and always emits strong `If-Match` for mutations. Evidence: deployed response `ETag: W/"0"` with `content-encoding` and matching `config_revision: 0`. Future impact: preserves Robot read/edit behavior across compression gateways without weakening optimistic concurrency. Semantic conflicts: original strict strong-response assumption. Human confirmation required: no; this is an observed transport compatibility fact.
|
||||
@@ -11,7 +11,7 @@ const MAX_RESPONSE_BYTES = 256 * 1024;
|
||||
const DEFAULT_TIMEOUT_MS = 15_000;
|
||||
const LOCAL_ROOT = '/api/works/ai-hardware';
|
||||
const UPSTREAM_ROOT = '/api/ai-hardware';
|
||||
const STRONG_REVISION = /^"(0|[1-9]\d*)"$/;
|
||||
const REVISION_ETAG = /^(?:W\/)?"(0|[1-9]\d*)"$/;
|
||||
const LOCAL_ID = /^[A-Za-z0-9][A-Za-z0-9_-]{0,35}$/;
|
||||
const OPERATION_ID = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
|
||||
const MAX_RETRY_AFTER_SECONDS = 2;
|
||||
@@ -285,9 +285,9 @@ function validateConfigUpdate(body: Record<string, unknown>): Record<string, unk
|
||||
return output;
|
||||
}
|
||||
|
||||
function strongRevisionFrom(response: Response): number {
|
||||
function revisionFromEtag(response: Response): number {
|
||||
const etag = response.headers.get('etag');
|
||||
const match = etag?.match(STRONG_REVISION);
|
||||
const match = etag?.match(REVISION_ETAG);
|
||||
if (!match) throw new SafeRouteError(502, 'AI_HARDWARE_INVALID_ETAG', 'AI hardware service returned an invalid revision', true);
|
||||
const revision = Number(match[1]);
|
||||
if (!Number.isSafeInteger(revision)) throw new SafeRouteError(502, 'AI_HARDWARE_INVALID_ETAG', 'AI hardware service returned an invalid revision', true);
|
||||
@@ -559,7 +559,7 @@ export function createAiHardwareRouteHandler(dependencies: AiHardwareRouteDepend
|
||||
}
|
||||
let revision: number | undefined;
|
||||
try {
|
||||
revision = requireEtag ? strongRevisionFrom(response) : undefined;
|
||||
revision = requireEtag ? revisionFromEtag(response) : undefined;
|
||||
} catch (error) {
|
||||
await cancelTimedResponse(activeCall);
|
||||
throw error;
|
||||
|
||||
@@ -98,10 +98,56 @@ describe('AI hardware Host API route', () => {
|
||||
expect(result.payload).toEqual({ success: true, data: config, revision: 0 });
|
||||
});
|
||||
|
||||
it('rejects weak upstream ETags without exposing upstream data', async () => {
|
||||
it('accepts canonical weak numeric ETags for versioned responses and keeps outbound If-Match strong', async () => {
|
||||
const config = {
|
||||
id: 'a-1', name: 'Desk', config_revision: 0,
|
||||
system_prompt: null, lang_code: null, language: null, asr_model_id: null,
|
||||
vad_model_id: null, llm_model_id: null, slm_model_id: null, vllm_model_id: null,
|
||||
tts_model_id: null, tts_voice_id: null, tts_language: null, tts_volume: null,
|
||||
tts_rate: null, tts_pitch: null, mem_model_id: null, intent_model_id: null,
|
||||
chat_history_conf: null,
|
||||
};
|
||||
const updatedConfig = { ...config, config_revision: 1, system_prompt: 'hello' };
|
||||
const assignment = { id: 'd-1', agent_id: 'a-1', assignment_revision: 0 };
|
||||
const updatedAssignment = { ...assignment, assignment_revision: 1 };
|
||||
const fetchImpl = vi.fn<typeof fetch>()
|
||||
.mockResolvedValueOnce(jsonResponse(config, { headers: { etag: 'W/"0"' } }))
|
||||
.mockResolvedValueOnce(jsonResponse(updatedConfig, { headers: { etag: 'W/"1"' } }))
|
||||
.mockResolvedValueOnce(jsonResponse(assignment, { headers: { etag: 'W/"0"' } }))
|
||||
.mockResolvedValueOnce(jsonResponse(updatedAssignment, { headers: { etag: 'W/"1"' } }));
|
||||
const { handler } = setup(fetchImpl);
|
||||
|
||||
const getConfig = await invoke(handler, 'GET', '/api/works/ai-hardware/agents/a-1');
|
||||
expect(getConfig.payload).toEqual({ success: true, data: config, revision: 0 });
|
||||
|
||||
const patchConfig = await invoke(handler, 'PATCH', '/api/works/ai-hardware/agents/a-1', {
|
||||
revision: 0, system_prompt: 'hello',
|
||||
});
|
||||
expect(patchConfig.payload).toEqual({ success: true, data: updatedConfig, revision: 1 });
|
||||
expect((fetchImpl.mock.calls[1][1] as RequestInit).headers).toMatchObject({ 'If-Match': '"0"' });
|
||||
|
||||
const getAssignment = await invoke(handler, 'GET', '/api/works/ai-hardware/devices/d-1/agent-assignment');
|
||||
expect(getAssignment.payload).toEqual({ success: true, data: assignment, revision: 0 });
|
||||
|
||||
const putAssignment = await invoke(handler, 'PUT', '/api/works/ai-hardware/devices/d-1/agent-assignment', {
|
||||
revision: 0, agent_id: 'a-1',
|
||||
});
|
||||
expect(putAssignment.payload).toEqual({ success: true, data: updatedAssignment, revision: 1 });
|
||||
expect((fetchImpl.mock.calls[3][1] as RequestInit).headers).toMatchObject({ 'If-Match': '"0"' });
|
||||
});
|
||||
|
||||
it.each([
|
||||
'w/"0"',
|
||||
'W/ "0"',
|
||||
'W/"00"',
|
||||
'W/"-1"',
|
||||
'W/"1.0"',
|
||||
'W/"revision"',
|
||||
'W/"9007199254740992"',
|
||||
])('rejects non-canonical weak ETag %s without exposing upstream data', async (etag) => {
|
||||
const fetchImpl = vi.fn<typeof fetch>().mockResolvedValue(jsonResponse({
|
||||
id: 'd-1', agent_id: 'a-1', assignment_revision: 0, token: 'secret',
|
||||
}, { headers: { etag: 'W/"0"' } }));
|
||||
}, { headers: { etag } }));
|
||||
const { handler } = setup(fetchImpl);
|
||||
const result = await invoke(handler, 'GET', '/api/works/ai-hardware/devices/d-1/agent-assignment');
|
||||
expect(result.status).toBe(200);
|
||||
@@ -109,7 +155,7 @@ describe('AI hardware Host API route', () => {
|
||||
expect(JSON.stringify(result.payload)).not.toContain('secret');
|
||||
});
|
||||
|
||||
it('rejects config and assignment DTO revisions that disagree with the strong ETag', async () => {
|
||||
it('rejects config and assignment DTO revisions that disagree with the canonical ETag', async () => {
|
||||
const config = {
|
||||
id: 'a-1', name: 'Desk', config_revision: 2,
|
||||
system_prompt: null, lang_code: null, language: null, asr_model_id: null,
|
||||
@@ -118,13 +164,13 @@ describe('AI hardware Host API route', () => {
|
||||
tts_rate: null, tts_pitch: null, mem_model_id: null, intent_model_id: null,
|
||||
chat_history_conf: null,
|
||||
};
|
||||
const configHandler = setup(vi.fn<typeof fetch>().mockResolvedValue(jsonResponse(config, { headers: { etag: '"1"' } })));
|
||||
const configHandler = setup(vi.fn<typeof fetch>().mockResolvedValue(jsonResponse(config, { headers: { etag: 'W/"1"' } })));
|
||||
const configResult = await invoke(configHandler.handler, 'GET', '/api/works/ai-hardware/agents/a-1');
|
||||
expect(configResult.payload).toMatchObject({ success: false, status: 502, code: 'AI_HARDWARE_INVALID_RESPONSE' });
|
||||
|
||||
const deviceHandler = setup(vi.fn<typeof fetch>().mockResolvedValue(jsonResponse(
|
||||
{ id: 'd-1', agent_id: 'a-1', assignment_revision: 3 },
|
||||
{ headers: { etag: '"2"' } },
|
||||
{ headers: { etag: 'W/"2"' } },
|
||||
)));
|
||||
const deviceResult = await invoke(deviceHandler.handler, 'GET', '/api/works/ai-hardware/devices/d-1/agent-assignment');
|
||||
expect(deviceResult.payload).toMatchObject({ success: false, status: 502, code: 'AI_HARDWARE_INVALID_RESPONSE' });
|
||||
|
||||
Reference in New Issue
Block a user