fix(clients): harden config writeback flows - #171
Conversation
- Narrow discovered config parse formats to the supported value set. - Reject invalid resolved candidates before raw-contract fallback. - Cover invalid format handling at the Admin adapter boundary.
- Report creation from actual SQLite insert ownership, including races. - Gate initial backup policy and auto-attach on confirmed creation. - Reuse authoritative config details and the existing attach API. - Cover eligible, ineligible, sequential, and concurrent paths.
- Stamp staged snapshots with the local creation instant.\n- Keep API display and retention ordering aligned.\n- Preserve cross-platform same-second replacement semantics.
There was a problem hiding this comment.
Pull request overview
This pull request hardens the client configuration writeback and backup publication flows. The backend now reports whether a settings update actually created a new client record, allowing the frontend to initialize the backup policy and attach only after a real creation. It also aligns backup filenames, modification times, and retention ordering with the actual backup creation instant, while tightening parse-format validation at the Admin discovery DTO boundary.
Changes:
- Add
created: booleanto/api/client/updateresponses. Concurrent insert handling ensures that at most one request reportscreated=true, and the frontend receives this signal through its API types. - Add
attachCreatedClientIfEligibleso newly created clients are attached automatically only when they satisfy the existing attach eligibility rules. - Generate backup filenames from a creation instant with an explicit timezone offset, stamp backup modification times with that instant, prune by modification time, and publish through staging plus replacement to preserve same-second overwrite behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| board/src/lib/types.ts | Extends ClientSettingsUpdateData with the backend created signal. |
| board/src/lib/client-config-sync.ts | Adds the eligibility check and execution path for automatic attach after creation. |
| board/src/lib/client-config-sync.test.ts | Covers eligible created clients, the created=false short circuit, and ineligible clients. |
| board/src/lib/api.ts | Narrows the clientsApi.update response type to ClientSettingsUpdateResp. |
| board/src/lib/admin-discovery.ts | Normalizes config parse formats to a constrained union at the DTO boundary and validates resolved candidate inputs more strictly. |
| board/src/lib/admin-discovery.test.ts | Adds rejection coverage for an invalid config format on a resolved candidate. |
| board/src/components/client-form-drawer.tsx | Uses updateResponse.data?.created in create mode to decide whether to initialize backup policy and attempt automatic attach. |
| backend/src/system/paths/service.rs | Aligns backup naming, modification time, publication, and retention semantics with the creation instant and adds regression coverage. |
| backend/src/clients/service/state.rs | Adds state creation helpers that report created and ensure only one concurrent insert returns created=true. |
| backend/src/clients/service/settings.rs | Propagates the creation signal through the settings update result. |
| backend/src/api/models/client.rs | Adds the created field and schema description to the API DTO. |
| backend/src/api/handlers/client/handlers.rs | Populates created in /api/client/update responses and updates handler assertions. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdc4d3637f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Clarify that all pull request review output must use English.\n- Separate public GitHub text from Chinese day-to-day collaboration.
Deploying mcpmate-site with
|
| Latest commit: |
07b3a8b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c2ca14d1.mcp-umate.pages.dev |
| Branch Preview URL: | https://fix-client-config-writeback.mcp-umate.pages.dev |
- Skip automatic attach when backup policy initialization fails.\n- Report invalid resolved Admin client candidates accurately.
Summary
Motivation
New writable client records required a second manual Attach action even when they already satisfied the existing attach policy. Backup filenames also used UTC while copied file metadata could preserve the previous configuration modification time on macOS, producing conflicting timestamps in the backup UI.
Configuration and migration impact
YYYYMMDDHHMMSS+ZZZZtimestamps.Validation
bun test src/lib/client-config-sync.test.ts src/lib/admin-discovery.test.ts— 28 passed.bun run lint— 0 errors; existing repository warnings remain.bun run build— passed.RUSTC_WRAPPER= cargo test system::paths::service::tests --lib— 3 passed.RUSTC_WRAPPER= cargo clippy --all-targets --all-features -- -D warnings— passed.RUSTC_WRAPPER= cargo testoutside the sandbox — 897 passed; one unrelated parallel-state test failed, then passed on exact rerun.Residual risk
durable_commit_survives_projection_failure_without_a_second_catalog_transition; its exact rerun passed.