Repository navigation
feat(runtime): durable Work Ledger rebased onto current master - #2270
jasondeng1997 wants to merge 16 commits into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
hireview001
left a comment
There was a problem hiding this comment.
- CI failed: Upgrade release gate (failure).
- CI failed: upgrade-acceptance / Prepare upgrade artifacts and matrix (failure).
- CI failed: Atomic Memory clean migration (SeekDB) (failure).
- CI failed: tests (3.12, e2e) (failure).
- CI failed: tests (3.14, e2e) (failure).
- CI failed: tests (3.11, unit) (failure).
- CI failed: tests (3.13, unit) (failure).
- CI failed: tests (3.13, e2e) (failure).
- CI failed: tests (3.14, unit) (failure).
- CI failed: tests (3.12, unit) (failure).
- Additional CI issues: PR checks.
| handoff_report: HandoffReportConfig = Field(default_factory=HandoffReportConfig) | ||
| inference: InferenceConfig = Field(default_factory=InferenceConfig) | ||
| external_skills: ExternalSkillsConfig = Field(default_factory=ExternalSkillsConfig) | ||
| deployment: DeploymentConfig = Field(default_factory=DeploymentConfig) |
There was a problem hiding this comment.
[P1] Wire deployment roles into server and runtime composition
create_server_app reconstructs BuiltinConfig without the new deployment settings, and open_builtin_runtime never starts DurableScheduler, DurableWorker, or RuntimeMembership, so distributed roles retain the existing process-local execution behavior and workers expose data-plane routes. Propagate the configuration and compose the promised role-specific services and routes.
There was a problem hiding this comment.
Fixed in b311e14a. The wiring now exists end to end:
ServerSettingsacceptsdeployment,coordination,worker,operationsandrate_limit(src/powercontext/server/settings.py:238-242). It previously had none of them, so no environment variable could reach the ledger.create_server_apppasses all five intoBuiltinConfig(server/factory.py:169-182), and so do the background runner and the offline migration command (server/cli.py).open_builtin_runtimeopens the ledger (composition.py:900), implemented by_open_work_background(composition.py:1241) with role gating:
scheduler = DurableScheduler(...) if role in ("all", "scheduler") else None
worker = DurableWorker(...) if role in ("all", "worker") else Noneapi therefore never scans or claims, scheduler never claims, and worker never scans. RuntimeMembership starts for every role that runs something, and shutdown stops the worker, then the scheduler, then membership, so an in-flight claim still commits under a valid lease.
One deliberate scope decision: a single_node/all deployment starts the ledger only when a background schedule is configured (_work_background_requested, composition.py:1145). An unconfigured install keeps exactly today's behaviour instead of acquiring an ownership lease and polling an empty queue. OperationManager is exposed as runtime.operations in every role, because reading, cancelling and retrying history is not role-specific.
Covered by tests/builtin/runtime/test_work_composition.py (role gating, discoverer and handler registration, a second runtime rejected with DuplicateSingleNodeError, clean shutdown).
There was a problem hiding this comment.
Rechecked at 886a0cfb: configuration propagation and ledger component construction are present, but the route-isolation part of this finding is still reproducible. create_server_app() with deployment.mode="distributed" and either role="scheduler" or role="worker" still registers /v1/memory/flush and /v1/scopes/{scope_id}/operations, even with Dashboard and MCP disabled. The factory unconditionally builds the full data-plane app. This conflicts with the health/metrics-only role boundary in RFC 1430 and leaves a worker's provider-backed business routes reachable on its listener. Please gate the application/routes by deployment role and cover the actual factory output, rather than only testing _work_role_components() in isolation.
| "x-powercontext-scope-mode": "selection", | ||
| } | ||
| }, | ||
| "/v1/operations": { |
There was a problem hiding this comment.
[P1] Implement the advertised durable operation endpoints
The generated schema advertises operation listing, retrieval, cancellation, and retry, but the canonical OpenAPI contract and server route registration contain none of them, so these requests return 404. Add the canonical contract and handlers, connect OperationManager to memory flush, and regenerate the transport definitions.
There was a problem hiding this comment.
Still open — this is the one substantive item I have not closed yet, and I would rather say so than claim it.
State at b311e14a: the runtime side is now ready. OperationManager is composed for every role and exposed as runtime.operations (builtin/runtime/composition.py:1121), and MemoryWorkHandler drives durable Memory work through the ledger, so there is finally something to inspect, cancel and retry. What is still missing is the transport: openapi/powercontext.yaml has no /v1/operations paths, so scripts/generate_api.py --check regenerates nothing and server/app.py registers no handler.
The work I plan, in this order:
- Add the four paths to the canonical contract with
Operation,OperationPageand anexpected_versionmutation body, plus the202+Location+Retry-Afterresponse onPOST /v1/memory/flushthat the docs already describe. - Regenerate
_generated/{models,operations,schema}.pyand keep--checkclean. - Register the handlers in
server/app.pythrough the same_add_routepath as the other operations, mappingStoredWorkto the response and keeping the_PUBLIC_KINDSwhitelist so maintenance work stays private. - Enqueue from the flush handler when
Prefer: respond-asyncis requested, and notify the local worker.
One design question I do not want to guess on: these paths address work by operation_id, while the Scope lives on the stored row, so access cannot be resolved from the request alone. I plan to add a resolver that loads the operation and authorizes against its scope_id (scope.read for get/list, scope.contribute for cancel/retry). If you would rather keep the existing resolver vocabulary and require scope_id as a query parameter instead, say so and I will do that — it changes the contract shape, which is exactly the kind of thing I should not decide unilaterally.
There was a problem hiding this comment.
Implemented in 11f4319c. The four endpoints are now canonical contract, generated transport, and live routes.
| Route | Access |
|---|---|
GET /v1/scopes/{scope_id}/operations |
scope.read |
GET /v1/scopes/{scope_id}/operations/{operation_id} |
scope.read |
POST /v1/scopes/{scope_id}/operations/{operation_id}/cancel |
scope.contribute |
POST /v1/scopes/{scope_id}/operations/{operation_id}/retry |
scope.contribute |
The paths are Scope-scoped, and that was the open design question from my last comment. You did not answer it, so I picked the shape that does not weaken authorization: a durable row carries its own scope_id, and every access resolver in server/app.py is a pure function of path/query/body. Authorizing /v1/operations/{id} from the request alone is therefore impossible today — it would need a database read inside _authorization_dependency, or an unauthenticated identifier space. Under the owning Scope, the existing resolver vocabulary works unchanged and the path Scope becomes an ownership assertion in the handler.
What that buys, concretely:
- another Scope's operation and internal maintenance work both read as
404, so ids cannot be probed for existence; _PUBLIC_KINDS(builtin/runtime/operations.py) now covers Memory and Profile source-window families in addition to Experience, so a Scope owner can act on the work its own Sources produced while maintenance work stays invisible;- the response projects a safe subset of
StoredWork— payload, logical key and lease internals do not leave the ledger; - cancel and retry take
expected_version; a lost optimistic race is409 operation_conflictand leaves the original state untouched (_map_governance_errornow mapsWorkStateConflictErrorandStaleWorkClaimError).
On "connect OperationManager to memory flush": I did not do that, and I corrected the docs instead of claiming it. The runtime side is ready — runtime.operations is composed for every role — but wiring flush to it means changing POST /v1/memory/flush from a synchronous 200 FlushMemoryResponse to a 202 handle with Location/Retry-After, which is a client-visible response-model change (the generator only accepts multiple success codes when they share one model). That is a contract decision I did not want to slip into a rebase PR. So:
docs/{en,zh}/docs/develop/http-api.mdnow documents exactly these four endpoints and states that flush still answers200synchronously;docs/{en,zh}/rfcs/1430_distributed_server_workers.mdkeeps the proposal but now carries an implementation-status paragraph naming what is not shipped:202 OperationAccepted,Prefer: respond-async,Prefer: wait=N,409 operation_blocked, and the clientsubmit_memory_flush()/ polling helpers.
Evidence at this head:
tests/test_operation_routes.py(new, 2 tests): list paging and filters, kind/status scoping, cross-Scope404, maintenance404, stale-version409, cancel →cancelled, cancelled-is-not-retryable409, and retry of a genuinely failed window returning toqueuedwith the error fields cleared;make contract-test→ 56 passed (contract, generated Python, JS operations, Desktop digest all consistent);tests/test_server.py+tests/e2e/test_operation_api.py+ docs contract tests → 70 passed;- DSH plugin
pnpm testgreen after the regenerated catalog.
Two generated-artifact notes, in case they look surprising in the diff: the schema is named DurableOperation rather than Operation because a model named Operation collides with the generator's own route generic (Operation[Request, Response]) and silently pointed response_type at the generic; and OperationStatus is a named schema so the generator cannot invent a colliding Status2 — the first attempt did exactly that and renamed an unrelated model's enum.
| ) | ||
| _require_complete_window(rows, payload) | ||
| _, source_catalog = services.sources() | ||
| memory = services.memory(source_catalog) |
There was a problem hiding this comment.
[P1] Adapt durable memory work to the current Atomic Memory service
For an unconsumed source window, MemoryWorkHandler calls services.memory, which does not exist on _ScopedServices; its subsequent memory_artifact_id and collection planning assumptions also belong to the previous memory implementation. Prepare and commit through the current Atomic Memory service so durable memory work can complete instead of exhausting retries.
There was a problem hiding this comment.
Restored in b311e14a. MemoryWorkDiscoverer (work_handlers.py:110), MemoryWorkHandler (work_handlers.py:267) and memory_work_spec (work_handlers.py:540) are back, and they no longer touch _ScopedServices.memory, memory_artifact_id or the old collection planning.
The claimed window now runs through RelationalContexts.process_memory (runtime/relational.py:1544), which is the current Atomic Memory path: it takes the per-scope activation lock and calls AtomicMemorySourceWindowProcessor.flush, so the model calls stay outside the transaction and the processor keeps the single transaction that commits the cursor and the artifacts. MemoryWorkHandler.prepare only decides the outcome:
already_committedwhen the cursor already passed the claimed window,supersededwhen a racing window consumed the prefix (the key is released instead of retried),processed/idlefrom theMemoryFlushResult,memory_conflictas a retryableWorkExecutionErrorforAtomicMemoryConflictError/GenerationConflictError/RevisionConflictError.
A retried attempt re-reads the cursor, so a window that already committed retires rather than repeating the domain write — at-least-once model calls, at-most-once commit, as the RFC states.
Covered by tests/builtin/runtime/test_memory_work.py: the spec tracks the unconsumed window, the handler drives the flush once, and a second attempt reports already_committed after the cursor advanced.
| self._payload_version = payload_version | ||
|
|
||
| async def page(self, continuation: str | None, limit: int, /) -> DiscoveryPage: | ||
| scopes = await self._contexts.scope_ids_page(continuation, limit) |
There was a problem hiding this comment.
[P2] Provide the scope pagination method used by discovery
MemoryWorkDiscoverer and ExperienceWorkDiscoverer call scope_ids_page, but RelationalContexts has no such method, so their first discovery page fails before enqueueing any work. Implement bounded scope pagination or adapt both discoverers to a supported interface.
There was a problem hiding this comment.
Already satisfied at this head: RelationalContexts.scope_ids_page exists (runtime/relational.py:1402) and both MemoryWorkDiscoverer (work_handlers.py:131) and ExperienceWorkDiscoverer (work_handlers.py:170) use it for their first page. tests/builtin/runtime/test_work_composition.py now asserts that both discoverers register, so an interface regression fails a unit test rather than the first discovery page.
| coordination: CoordinationConfig = Field(default_factory=CoordinationConfig) | ||
| worker: WorkerConfig = Field(default_factory=WorkerConfig) | ||
| operations: OperationsConfig = Field(default_factory=OperationsConfig) | ||
| rate_limit: RateLimitConfig = Field(default_factory=RateLimitConfig) |
There was a problem hiding this comment.
[P2] Install the limiter when rate limiting is enabled
ServerSettings accepts rate_limit.enabled=True, but create_server_app neither installs SharedRateLimitMiddleware nor binds SharedRateLimiter, leaving requests unlimited regardless of the configured budget. Connect the limiter to the application lifecycle and run it after principal binding.
There was a problem hiding this comment.
Fixed in b311e14a. SharedRateLimitMiddleware is new (server/rate_limit.py:43) and create_server_app installs it when rate_limit.enabled is set (server/factory.py:368), appended after the authentication middleware so current_principal() is already bound.
Behaviour: the fixed window lives in the deployment database (RateLimitRepository.consume), so replicas share one budget; the key is a SHA-256 of principal.type:principal.id, falling back to the client host when no principal is bound; /health/live, /health/ready and /metrics are exempt; a rejected request returns 429 rate_limited with Retry-After.
tests/test_server_rate_limit.py drives a real app: requests=1 allows the first request and rejects the second with a positive Retry-After, the readiness probe still returns 200, and with enabled=False three requests are never rejected.
|
|
||
| def _start_configured_server(settings: ServerSettings, scheduler_path: Path) -> RunningServer: | ||
| app = create_server_app(settings=settings, scheduler_path=scheduler_path) | ||
| def _start_configured_server(settings: ServerSettings) -> RunningServer: |
There was a problem hiding this comment.
[P2] Update the remaining caller of the server helper
The helper now accepts one positional argument, but test_context_text_assembly.py still passes settings and a scheduler path, preventing that configured acceptance scenario from starting its server. Update the remaining caller or preserve the compatible signature.
There was a problem hiding this comment.
This one does not reproduce at this head. _start_configured_server still takes two positional parameters — (settings: ServerSettings, scheduler_path: Path) (tests/e2e/real_experience_skill/harness.py:2027) — and every caller passes both, including test_context_text_assembly.py:111, test_profile_context_assembly.py:83 and test_native_code.py:376,483. The rebase kept the signature and the call sites consistent, so no acceptance scenario is blocked here. If you see the failure again, the job log would help me find the path I missed.
hidb4ai
left a comment
There was a problem hiding this comment.
Reviewed 1ee3cdc47b055f734ca001830f44cb5a9e07d6f5. This is not ready to merge.
I reproduced the existing role composition issue, missing Operation routes, obsolete Memory service calls, missing scope pagination, and inactive rate limiting. Operation GET/list return 404; discovery and Memory preparation raise AttributeError; a one-request rate limit still permits the second request. I am keeping those findings in their existing threads.
The inline comment adds a separate migration-test collection failure. scripts/generate_api.py --check also fails because the checked-in generated API does not match the canonical contract. Please complete the runtime/transport wiring and migration acceptance before seeking approval. Validation here used isolated SQLite and ASGI; it does not establish real OceanBase multiprocess acceptance.
|
|
||
| from typer.testing import CliRunner | ||
|
|
||
| from powercontext.builtin.persistence.migration import CURRENT_SCHEMA_REVISION |
There was a problem hiding this comment.
[P2] Use a migration module that exists in the package
python -m pytest --collect-only tests/test_server_migration_cli.py fails at this import with ModuleNotFoundError: No module named 'powercontext.builtin.persistence.migration'. This PR adds the test but no such module, so a normal test-suite collection aborts before migration behavior can be checked. Update the test to use the shipped migration API and revision contract, or include the missing implementation, so the acceptance scenario can actually run. The new Alembic migrations/env.py and versions/0001_baseline.py also import this absent module.
The invocation on line 39 also exits with code 2: No such command 'migrate'. The CLI registers server db-migrate, so updating only the import would still leave this scenario unable to exercise a migration.
There was a problem hiding this comment.
Fixed by the rebase. tests/test_server_migration_cli.py no longer imports powercontext.builtin.persistence.migration; it uses the shipped API (powercontext.cli.app.create_cli plus powercontext.server.cli.app) and invokes the registered command, server db-migrate plan / apply (tests/test_server_migration_cli.py:15-16,39). There is no 0001_baseline.py in the tree, and neither server_resources/env.py nor mysql/env.py imports the absent module — they only configure Alembic against the executor's pinned connection.
python -m pytest tests/test_server_migration_cli.py passes here (it is part of the 46-test migration run), and scripts/generate_api.py --check is also clean at this head.
There was a problem hiding this comment.
The test import and its db-migrate plan/apply invocation are fixed and pass at 886a0cfb. One shipped caller still needs the same correction: docker/compose.distributed.yaml:18 runs ["server", "migrate"], and both deployment guides tell operators to run that command. The exact-head CLI exits 2 with No such command 'migrate'. Every API/Scheduler/Worker service depends on that migrator completing successfully, so the documented Compose topology cannot start. Please adapt the migrator to the supported explicit db-migrate plan/apply workflow, including its acceptance requirements, and update both guides and the Docker contract test that currently asserts the invalid command.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Re-apply the Work Ledger onto master's migration framework, which replaced Alembic revision discovery with frozen bundles (detached DDL, fingerprints and a single linear `down_revision` chain). The ledger keeps its own tables and semantics; only the registration changes. Registration * The revision is registered as `v1_4_1_005` and follows `v1_4_1_004` instead of `s1402`. Master's owner-recovery revision already occupies the slot after `s1402`, and the bundle rejects a second head, so the ledger appends to the single chain. The release-based name follows the naming rule that revision introduced, which keeps `server db-migrate status` able to describe the target. * Both manifests (`server_resources`, `server_sources/1.4.0`) register the revision resources, its fingerprint, its readiness entry and the two MySQL state layouts, keeping master's `v1_4_1_004` entries intact. * The SQLite fingerprint is measured by replaying `s1300` through the new head; it changed only because the recorded layout now includes `pc_access_owner_recovery`. The persistence review digest in `release/upgrade-support.json` is recomputed and the rationale states what the revision adds. Tests * Migration tests assert the extended chain (`s1400, s1401, s1402, v1_4_1_004, v1_4_1_005`) and the new head revision. * `previous_bundle` and the owner-recovery test now drop later revisions through one shared `drop_revision` helper, in head-first order, so the synthetic "before" bundle keeps a linear chain. Verified: `make check` (prek, ruff, ty, integration manifest), the full `tests/builtin/persistence` suite (691 passed, 110 skipped) and the migration CLI, revision-name and maintenance CLI suites.
595f18b to
dd80249
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Scope update after the rebase: this PR is now the runtime Work LedgerRebasing onto current master forced two structural calls, both recorded in the latest commit: 1. The memory work kind is removed. Master now owns Source-to-Memory activation in the atomic memory pipeline ( 2. The operations HTTP layer is deferred. The What the ledger itself gains from master's rework: 3. The revision is Local verification on the rebased commit: |
| "schemas/sqlite-v1_4_1_004.json", | ||
| "schemas/mysql-v1_4_1_004.json" | ||
| ], | ||
| "v1_4_1_005": [ |
There was a problem hiding this comment.
[P1] 迁移版本号 v1_4_1_005 与并行 PR #2192 撞号,两者是内容不同的 revision
本行注册的 v1_4_1_005(work ledger)与 #2192(head 27d499e0)注册的 v1_4_1_005(atomic memory description)同号不同内容:
| 项 | 本 PR(head dd80249b) |
#2192(head 27d499e0) |
|---|---|---|
| 版本模块 | versions/v1_4_1_005_work_ledger.py |
versions/v1_4_1_005_atomic_memory_description.py |
sqlite_fingerprints["v1_4_1_005"] |
92eaad28512ba8a03917acdd9a0996523a67450092e03161eca68e4e0f9d4cb6 |
2e24be9f699c202cb9d7985690b2be947bdc98f8f64c1c4e4b8e16e35a580fc4 |
release/upgrade-support.json 的 persistence_sha256 |
581696bc…(本 head 复算一致) |
cbbf2858…(同 head 复算一致) |
两个 PR 还同时改 migrations/server_sources/1.4.0/manifest.json 的同一处追加,以及 release/upgrade-support.json 的同一个摘要值。
影响:任一方先合,另一方 rebase 时这三处必然文本冲突;若按“两边都保留”解决,同一个 revision 名会对应两份不同 DDL 与两个不同 fingerprint,离线迁移校验按 revision 名取 fingerprint,升级链基线取到哪一份取决于合并顺序。两边的 CI 各自为绿,缺陷只在合并后出现。
建议:本 PR 或 #2192 任一方改用下一个未占用编号(如 v1_4_1_006),同步 server_sources/1.4.0/manifest.json 与 sqlite_fingerprints,并按 scripts/upgrade_release_gate.py::managed_digest 重算 persistence_sha256;合入顺序确定后另一方 rebase 复算。
本次已核对的部分(不构成 Finding):本 head 的 managed_digest 与 release/upgrade-support.json 的声明值相等;tests/test_migration_revision_names.py、test_full_server_migrations.py、test_portable_storage_migration.py 在 exact head 副本上 27 passed,迁移资源自身自洽。
There was a problem hiding this comment.
Renamed to v1_4_1_006 (9683d25e). #2192 opened on 2026-10-22 and is 29/30 green, so it keeps 005; this PR takes the next free number.
| Item | Now |
|---|---|
| Version module | versions/v1_4_1_006_work_ledger.py |
| MySQL fragment | mysql/versions/v1_4_1_006.py |
| Frozen schemas | schemas/{sqlite,mysql}-v1_4_1_006.json |
down_revision |
unchanged: v1_4_1_004 |
sqlite_fingerprints["v1_4_1_006"] |
92eaad28… (unchanged, see below) |
persistence_sha256 |
fcc35b91… → 96fc678e14fa0cc85ee6668169b860bd345d0bc40f7f31e11c82291d5fdfaf56 |
Two details worth recording:
- The SQLite fingerprint did not change.
sqlite_inventoryexcludespc_schema_revisionand the retained objects, so the revision name never reaches the digest. I re-verified by replayings1300→ head through the real bundle and comparing every recorded value: all six match, includingv1_4_1_006. down_revisiondeliberately stays atv1_4_1_004. If feat(recall): add bounded atomic memory manifest #2192 lands first, rebasing will conflict here and I will point it at thatv1_4_1_005instead of merging both into a two-child graph.
tests/test_migration_revision_names.py (target now sequence: 6), test_full_server_migrations.py, test_portable_storage_migration.py, test_owner_recovery_migration.py, test_release140_server_migrations.py and test_server_migration_cli.py pass (46 passed), and scripts/generate_api.py --check is clean.
There was a problem hiding this comment.
[P1] 改号到 v1_4_1_006 后,该编号已被 master 占用:合并结果是 add/add 冲突
回复里的判断在 2026-10-29 成立,但 master 随后合入了 source-write-plugins,v1_4_1_006 已被占用。同一根因在当前 head 上仍然存在。
| 项 | 本 PR(head 886a0cfb) |
master 176b3f92 |
|---|---|---|
| 版本模块 | versions/v1_4_1_006_work_ledger.py |
versions/v1_4_1_006_source_write_plugins.py |
down_revision |
v1_4_1_004 |
v1_4_1_005 |
sqlite_fingerprints["v1_4_1_006"] |
92eaad28512ba8a03917acdd9a0996523a67450092e03161eca68e4e0f9d4cb6 |
cacce80bdfb14aff889b7babebc9ab8487b7f0a11d2dbbd3d77a3717a23df74c |
revision_resources["v1_4_1_006"] |
schemas/{sqlite,mysql}-v1_4_1_006.json |
同名同键 |
| 链尾 | 停在 v1_4_1_006 |
另有 v1_5_0_001(down_revision = v1_4_1_006) |
证据按固定 OID 取得,未用工作树判断行尾:
git merge-tree --write-tree master refs/pr/2270(master176b3f92,head886a0cfb,merge-basebdf99789)报 31 处冲突,其中CONFLICT (add/add)三处正是mysql/versions/v1_4_1_006.py、schemas/mysql-v1_4_1_006.json、schemas/sqlite-v1_4_1_006.json;manifest.json与release/upgrade-support.json为 content 冲突。- 两边
revision_resources与sqlite_fingerprints都以v1_4_1_006为键。按「两边都保留」解冲突后,后写覆盖前写,同一 revision 名会对应两份不同 DDL 与两个不同 fingerprint——正是原 Finding 里的第 ① ② 条失效机制,不会因为改名一次而消失。 - 链形也有断点:master 为
004 → 005 → 006 → v1_5_0_001,本 PR 为004 → 006。若只解文本冲突而不改链,本 PR 的v1_4_1_006不在 master 那条链上。
本 head 自身仍然自洽,所以缺陷只在合并后出现,与本 PR 的 CI 结果无关:tests/test_migration_revision_names.py + tests/builtin/persistence/test_full_server_migrations.py 在本 head 的 exact 副本上 23 passed;scripts/upgrade_release_gate.py::managed_digest 复算 = 96fc678e14fa0cc85ee6668169b860bd345d0bc40f7f31e11c82291d5fdfaf56,与 release/upgrade-support.json 声明值相等。
修复方向:改号到当前未占用的 id(v1_4_1 系列下一个空号 v1_4_1_007,或按发布语义取 v1_5_0_002),down_revision 指向 master tip v1_5_0_001;同步 versions/、mysql/versions/、两个 schemas/*、manifest.json 的 revision_resources / sqlite_fingerprints / affected_objects,按 managed_digest 重算 persistence_sha256,并同步 tests/test_migration_revision_names.py 里 sequence 与待应用列表的期望值。
There was a problem hiding this comment.
本条状态更新(head 5fa749d8):d9f33be3 把 ledger 从 v1_4_1_006 改成 v1_4_1_007,与本条指出的“编号已被 master 占用 → add/add 冲突”这一半已经解除,manifest.json 里也不再有重名注册。
但改号只解决了“名字撞车”,没有解决“位置”:v1_5_0_001 是 master 当前的已登记 head,而本 PR 把它的 down_revision 从 v1_4_1_006 改成 v1_4_1_007,等于在已发布 head 之前插入一个新 revision。已经 stamp 成 v1_5_0_001 的库仍是 head,v1_4_1_007 永远不会执行。这条的完整证据与建议写在 v1_5_0_001_artifact_storage.py:23 的线程里(Copilot 提出、我补充了 master/head 对照与 #2395 的发布目标)。
本条按“重名已解”记录,不再单独挂 P1;剩下的链位置问题以那条线程为准。
MySQL reflects `BOOL` as `tinyint(1)`, but the frozen state layout for the new revision declared the column as `bool`. `MySQLSchema.recovery()` compares the live inventory against every registered statement prefix after each DDL statement, so once `pc_work_items` was created the schema looked like an uncontrolled state and every MySQL-family upgrade aborted with `incompatible_schema: Actual MySQL schema is not a controlled next-revision DDL state`. SQLite was unaffected, which is why only the seekDB and OceanBase jobs failed. Register the column the way the rest of the frozen chain does, `TINYINT(1)`, and declare the matching reflected type in the layout. Verified by replaying the revision against a live MySQL server: every one of the sixteen intermediate prefixes now matches the reflected inventory exactly, and `recovery()` returns the expected pending statements from the previous head. `make check` and the persistence suite stay green.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
… upgrade The service lifecycle and OceanBase engine-recovery suites asserted that a completed upgrade reports `v1_4_1_004`. The rebased ledger appends `v1_4_1_005`, so those assertions now compare against the actual head. Both jobs failed only on that comparison; the database was already `ready`.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
PR oceanbase#2192 registers `v1_4_1_005` for the atomic memory description revision and opened six days earlier, so it keeps that number. Two open pull requests claiming one revision name would ship two different DDL sets and two different offline fingerprints under a single name, and the winner would depend on merge order. This PR moves to the next free number instead. The chain is otherwise unchanged: `down_revision` stays `v1_4_1_004`, the SQLite fingerprint is unchanged because inventory never contains the revision name, and `release/upgrade-support.json` is re-digested with `scripts/upgrade_release_gate.py::managed_digest`. When oceanbase#2192 lands first this revision will rebase onto it and point `down_revision` at its `v1_4_1_005`. Verified with `tests/test_migration_revision_names.py`, `test_full_server_migrations.py`, `test_portable_storage_migration.py`, `test_owner_recovery_migration.py`, `test_release140_server_migrations.py` and `test_server_migration_cli.py` (46 passed), plus a full `s1300`-to-head replay that reproduces every recorded fingerprint.
The ledger modules, the Work handlers and the OperationManager all existed, but nothing in `src/` ever constructed them: `create_server_app` rebuilt `BuiltinConfig` without the deployment, coordination, worker and operations sections, `open_builtin_runtime` never started a scheduler, worker or member, and `ServerSettings` did not accept those sections at all, so a distributed deployment silently kept the single-process behaviour. Composition now owns the lifecycle: - `ServerSettings` accepts `deployment`, `coordination`, `worker`, `operations` and `rate_limit`, and the Server, the background runner and the offline migration command pass them into `BuiltinConfig` instead of taking defaults. - `_open_work_background` starts `RuntimeMembership` plus the role's components: `all` runs the scheduler and the worker, `scheduler` and `worker` run only their own, and `api` never scans or claims work. A single-node deployment starts the ledger only when a background schedule is configured, so an unconfigured install keeps its current behaviour. Shutdown drains the worker before leadership and membership disappear. - `runtime.operations` exposes the `OperationManager` so the HTTP layer can inspect, cancel and retry history regardless of role. `MemoryWorkHandler` and `MemoryWorkDiscoverer` are restored against the current Atomic Memory service: a claimed window runs through `RelationalContexts.process_memory`, which keeps the model calls outside the lock and lets the processor own the single transaction that commits the cursor and the artifacts. A retried attempt re-reads the cursor, so a window that already committed retires instead of repeating the domain write. `SharedRateLimitMiddleware` closes the other half of the advertised behaviour: `rate_limit.enabled=True` used to leave requests unlimited. The window lives in the deployment database, runs after principal binding, keys a bound principal by type and id (falling back to the client host), and exempts liveness, readiness and metrics. Verified with `tests/builtin/runtime/test_work_composition.py`, `test_memory_work.py` and `tests/test_server_rate_limit.py` (8 passed) plus the existing `tests/builtin/runtime` and Server suites.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
…clusive The ledger was started for any `single_node` install that configured a background schedule. The artifact-processing supervisor starts for exactly the same configurations (`runtime/artifact_processing_role` must be `all`), so both paths claimed the same source window in one process: the ledger advanced the cursor, and the supervisor's next tick then saw an empty window and recorded a noop instead of doing the work. That is what pushed `tests/e2e/test_worker_otlp.py` and `tests/e2e/test_worker_model_usage.py` into `outcome=noop` with no model spans. RFC 1430 already fixes the ordering: the bridge release moves `single_node/all` onto the ledger only once the APScheduler path has fully retired. Until then: * `_work_background_requested` is true only for an explicit `distributed` deployment, so a `single_node` install keeps the supervisors it has today and only exposes the `OperationManager` facade. * `_ledger_families` reports the Families the ledger drives, and `_supervisor_bindings` drops exactly those, so a distributed run never lets the supervisor compete for a window the ledger already owns. * `_work_role_components` states the role split as a pure function: `api` runs neither component, `scheduler` only discovers, `worker` only claims, `all` does both. The role wiring, membership, rate limiting and Memory ledger handler added earlier are unchanged; only the activation boundary moved. Verified locally: the four `test_worker_otlp` and six `test_worker_model_usage` cases that failed on `b311e14a` now pass (10 passed), and `tests/builtin/runtime/test_work_composition.py` covers the new boundary.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
…s 800 ms budget `stop.mjs` bounds the boundary flush with `AbortSignal.timeout(800)`, so one Stop attempt is allowed to record `unknown` when the server round trip does not fit. The chain test asserted `cursor_reached` after a single attempt, which encodes machine speed into the assertion and failed on a loaded Python 3.14 runner. The flush is idempotent and belongs to a bounded-attempt contract, so the test now re-runs Stop until the recorded observation reaches the cursor, with the same 30 s ceiling as the surrounding harness. The assertion itself is unchanged.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
The runtime facade has been able to inspect, cancel and retry durable work
since the ledger landed, but nothing was reachable over HTTP: `docs/.../http-api.md`
advertised `/v1/operations/*` while `openapi/powercontext.yaml` had no such paths,
so every request returned 404. This closes that gap.
The endpoints hang off the owning Scope:
| Route | Access |
| --- | --- |
| `GET /v1/scopes/{scope_id}/operations` | `scope.read` |
| `GET /v1/scopes/{scope_id}/operations/{operation_id}` | `scope.read` |
| `POST /v1/scopes/{scope_id}/operations/{operation_id}/cancel` | `scope.contribute` |
| `POST /v1/scopes/{scope_id}/operations/{operation_id}/retry` | `scope.contribute` |
Scope-scoped paths are the point, not a detail. A durable row carries its own
`scope_id`, so authorizing from the path keeps the existing resolver vocabulary
and needs no database lookup inside access resolution. The path Scope is then an
ownership assertion in the handler: another Scope's row, and internal
maintenance work, both read as `404`, so identifiers cannot be probed.
Cancel and retry take `expected_version`, map a lost optimistic race to
`409 operation_conflict`, and leave the original state untouched. The response
projects a safe subset of `StoredWork`; payload, logical key and lease internals
stay private.
`_PUBLIC_KINDS` now covers the Memory and Profile source-window families in
addition to Experience, so a Scope owner can act on the work its own Sources
produced while maintenance work stays invisible.
Docs are corrected to match: the async `Prefer: respond-async` handle, the
`202`, `409 operation_blocked` and the client polling helpers are still proposal
material, and the RFC now says so instead of describing them as shipped.
Contract, generated Python and JS operations, the DSH bundle and the Desktop
contract digest are regenerated; `OperationStatus` is a named schema so the
generator cannot invent a colliding `Status2`.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
|
||
| // Generated from openapi/powercontext.yaml. Do not edit. | ||
| export const contractSha256 = "c12901b88b8e958c9cd63469298b9a04b9d5e2b0b4564d83b50f7ae95542519a"; | ||
| export const contractSha256 = "c7f00e81a12848249c7105ba99c444b6233f4b2a4849d1d4443c419f53ac764f"; |
There was a problem hiding this comment.
[P1] 手改了生成物里的契约摘要,api.d.ts 没有重生成,四个 desktop job 在本 head 同签名红
本行只把 contractSha256 从 c12901b8… 换成 c7f00e81…,但同一提交在 openapi/powercontext.yaml 新增了 4 个 path(/v1/scopes/{scope_id}/operations、.../operations/{operation_id}、.../cancel、.../retry,共 +282 行),生成物里只有 desktop/src-tauri/src/transport/operations.json 与 desktop/ui/src/generated/operations.ts 的摘要跟着变,desktop/ui/src/generated/api.d.ts 完全没有改:本 PR 的 desktop/ 文件集只有前两个文件;head 11f4319c 的 api.d.ts(9344 行)全文不含 /v1/scopes/{scope_id}/operations。
证据(head 11f4319c,run 40995611177):
linux(job 123868189387)→$ oxlint ui scripts && pnpm generate:check && …→Error: Contract drift: ui/src/generated/api.d.ts. Run 'pnpm --dir desktop generate'.(desktop/scripts/generate.mjs:265)windows(123868189660)、macos (macos-15)(123868189664)、macos (macos-15-intel)(123868189587)同一行错误。- 归因排除基线:父提交
feb9e7f2与更早的dd80249b上这四个 job 都是 success;同期 feat(recall): add bounded atomic memory manifest #2192(同样新增了 OpenAPI operation、head4028045f)的linux/windows是 success。
为什么 Python 侧没兜住:desktop/scripts/generate.mjs 的 outputs 定义为 "ui/src/generated/api.d.ts": header + astToString(await openapiTS(contract)),--check 逐文件做文本全等比较;而 tests/test_desktop_contract.py 只比对 contractSha256 在两处生成物里的常量值。手工改摘要正好让 Python 门禁通过(在本 head 的 exact 副本上 pytest tests/test_desktop_contract.py -q → 2 passed),make contract-test 绿,但 desktop 生成器检查红。
建议:在该 head 上跑一次 pnpm --dir desktop generate,把 ui/src/generated/api.d.ts(以及若 wire.rs 也变化的生成结果)一起提交;不要手改 contractSha256,因为它只是生成器输出的一部分,不是独立的校验开关。如果这 4 个 operation 本就不打算让 Desktop 使用,需要在 PR 说明里写清取舍——api.d.ts 按完整 contract 生成,无法只排除某个 path。附带一点:operations.json 的 reviewed 目录改动前后都是 11 条,说明新 operation 没有进入 reviewed 集合,这与 api.d.ts 的漂移是两件事,前者可以是设计选择,后者是生成器硬性要求。
There was a problem hiding this comment.
Fixed in cf81c1a8 (pushed 03:54Z, three minutes after your comment — I hit the same failure in run 40995611177 and had already started on it). Your diagnosis is exactly right, including the part that stings: hand-patching contractSha256 made tests/test_desktop_contract.py pass while the generator check stayed red. That was my mistake, and the reason matters for anyone else who tries it.
Why I hand-patched at all. pnpm --dir desktop install --frozen-lockfile fails on this machine:
Error: × no @pnpm/exe.darwin-arm64 native binary was found for this host
desktop/package.json pins packageManager: pnpm@11.13.1, so pnpm tries to self-manage a version whose native binary is not present; --config.manage-package-manager-versions=false did not bypass it either. Without node_modules the generator cannot import yaml / openapi-typescript. Patching the two digests was me working around a local tooling problem instead of solving it, and it hid the real drift. Not repeating that: a digest-only patch is not a valid state for this repo.
What cf81c1a8 does. api.d.ts is header + astToString(await openapiTS(contract)) — no prettier step, no banner. So I generated the body with the pinned version (npx openapi-typescript@7.13.0 openapi/powercontext.yaml) and reattached the existing license header plus the // Generated from openapi/powercontext.yaml. Do not edit. line. Diff against the previous file is purely additive, 269 lines, and contains the four new paths and the four new schemas:
137a138,217
> "/v1/scopes/{scope_id}/operations": {
...
> "/v1/scopes/{scope_id}/operations/{operation_id}/cancel": {
On wire.rs. It should not change, and if it does the job will say so at cf81c1a8. desktop/scripts/generate.mjs collects schemaNames by walking $refs only from the operations in the wanted list, then feeds that graph to the Rust generator. My four operations are not in wanted, and no wanted operation references DurableOperation, DurableOperationPage, OperationStatus, ListOperationsRequest or MutateOperationRequest, so the Rust wire graph is byte-identical.
On the reviewed set staying at 11. That is deliberate and unchanged by this PR: wanted is an explicit review list for the Desktop client, not "everything in the contract". The two facts you separated are indeed separate — the reviewed catalogue legitimately stays at 11, while api.d.ts covers the whole contract and therefore must move. I added that distinction to the PR description so the next reader does not have to rediscover it.
One thing I would add to your suggestion, because it is what let this slip past a green make contract-test: tests/test_desktop_contract.py only compares the contractSha256 constant, so the Python gate cannot catch a stale api.d.ts. If you want that class of mistake closed permanently, the check should either compare all four generator outputs or contract-test should shell out to pnpm --dir desktop generate:check. I did not change it in this PR — it is a test-infrastructure decision, not part of the ledger — but I am happy to follow up if you want it here.
There was a problem hiding this comment.
本轮复核(head 已由 11f4319c 前移到 cf81c1a8,增量仅 desktop/ui/src/generated/api.d.ts +266):本条 api.d.ts 未重生成的问题已闭合——run 40999962235 的 linux/windows/macos×2 日志中 Desktop contract matches OpenAPI 均通过,Contract drift 不再出现。
但四个 desktop job 仍然全红,失败签名换成了 tests/session.rs:307 的 the shipped preview needs a qualified Server for its current contract:operations.json 的 contractSha256 已改成 c7f00e81…,compatibility.json 里却没有同摘要记录。该缺口另开在 desktop/src-tauri/src/transport/operations.json:2(r4420552939),本线程仅记录「摘要与生成物已对齐、资格记录仍缺」这一步的边界,避免把两处混成一条。
…aths `desktop/ui/src/generated/api.d.ts` is `license header + openapi-typescript(contract)`, and the new operation paths and schemas are part of the contract now. Regenerated with the pinned openapi-typescript 7.13.0; the diff is purely additive. Wire models are unchanged because they only cover schemas reachable from the reviewed Desktop operations.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "contractSha256": "c12901b88b8e958c9cd63469298b9a04b9d5e2b0b4564d83b50f7ae95542519a", | |||
| "contractSha256": "c7f00e81a12848249c7105ba99c444b6233f4b2a4849d1d4443c419f53ac764f", | |||
There was a problem hiding this comment.
[P1] 契约摘要前移后没有对应的 qualification 记录,四个 desktop job 换签名继续红
operations.json 的 contractSha256 从 c12901b8… 改到 c7f00e81…,但 desktop/src-tauri/src/connections/compatibility.json 里没有任何 contractSha256 等于 c7f00e81… 的记录,desktop/VALIDATION.md 也没有对应小节。
证据(本 head cf81c1a8,run 40999962235):
- linux(job 123881806924)、windows(123881807243)、macos-15(123881807209)、macos-15-intel(123881807075)四个 job 同签名失败:
thread 'bundled_qualification_allows_explicit_scope_browsing' panicked at tests/session.rs:307:10: the shipped preview needs a qualified Server for its current contract。 desktop/src-tauri/tests/session.rs该用例从include_str!("../src/transport/operations.json")取contractSha256,再在state.compatibility_profiles里找同摘要记录,找不到就在expect处 panic —— 所以摘要前移就必须同时落一条记录,只改摘要等于让桌面端在当前契约上无资格可选。- 父提交
11f4319c的失败签名是Contract drift: ui/src/generated/api.d.ts;本次 api.d.ts 已重生成,日志里Desktop contract matches OpenAPI已通过,因此这不是同一处红,是修完生成物后暴露的第二个缺口。 - 对照同日 fix(access): support ownership migration when enabling enforcement #2304(
a4dc0b11,77 checks 全 success):同样把 openapi 摘要前移,同时追加了sqlite-atomic-15c9d932-v1记录(contractSha256: e37820ef…+serverCommit/artifactSha256/operations/evidence)以及desktop/VALIDATION.md的 qualification 小节。
影响:desktop 端在当前契约下没有可匹配的协议资格,bundled_qualification_allows_explicit_scope_browsing 四平台确定性失败,任何依赖「已认证连接」的 UI 流程都无法建立;重跑无法消除。
建议:按 #2304 的形态补一条记录(serverCommit 取实际构建该 wheel 的提交、artifactSha256 取 wheel 摘要、operations 列全桌面端实际评审的操作、evidence 指向 desktop/VALIDATION.md 新小节),并同步更新 VALIDATION.md 中 “The current contract has the source-specific qualification …” 那句所指向的 id。建议补一条校验:摘要变更时必须存在同摘要记录,避免只改摘要仍能进入评审。
There was a problem hiding this comment.
Fixed in 886a0cfb. You are right on both halves: the digest moved and no record followed it, and a check that only lives in cargo test is not a check the author sees before asking for review.
The record is produced by the harness, not written by hand. sqlite-atomic-cf81c1a8-v1 came out of the existing fixture_qualification.py + real_server.py pair run from the clean cf81c1a8 tree:
| Field | Value | How it was obtained |
|---|---|---|
serverCommit |
cf81c1a8a9c40c1fe186944ba6e334b54aa5da70 |
git rev-parse HEAD, tree clean |
contractSha256 |
c7f00e81a12848249c7105ba99c444b6233f4b2a4849d1d4443c419f53ac764f |
operations.json, cross-checked against the recomputed digest |
artifactSha256 |
0530ed69045ed45be3048aafd9256710d1e1ef4b234858b6314513556ff2b904 |
powercontext-1.2.1.dev414+gcf81c1a8a-py3-none-any.whl |
operations |
the eleven bundled operations, incl. get_server_info |
operations.json, sorted() |
Run evidence on macOS 27.2 ARM64 with Python 3.14.7 and Rust 1.95.0:
real_server.py— all four modespassed:loopback-anonymous,loopback-bearer,https-bearer-base-path,loopback-provider. The provider mode additionally reportsproviderIdentityChangeInvalidatesContext=true,sameIdentityRevocationDeniesExactMemoryRead=true,sameTitleScopePagination51=true, and every mode reportsserverAliveAfterClientExit=true.- The report records
desktopWorkingTreeDirty=falseandcontractSha256=c7f00e81…. - All 632 packaged runtime files in
powercontext/andpowercontext_service_bootstrap/matchgit show HEAD:src/<path>byte for byte; the sorted manifest digest is8f591bfb56ed302481297a2b02f06d1cb4f381eb15090a603a1322c6f72d1e93.
desktop/VALIDATION.md gains #### Server cf81c1a8 protocol qualification and the "current contract has …" sentence now names sqlite-atomic-cf81c1a8-v1. The dd62723d / 4ffdf4e9 records keep their original c12901b8… digest, wheel and operation set — I did not rewrite history to make the diff smaller.
One deliberate omission: the four new endpoints are not in the record's operations list. They are Server-side contract; Desktop never calls them, and adding them would claim a qualification nobody ran. They are covered by tests/test_operation_routes.py / tests/e2e/test_operation_api.py instead, and the VALIDATION.md section says so explicitly.
The guard you suggested. tests/test_desktop_contract.py::test_desktop_bundle_qualifies_the_current_contract now asserts that some profile carries the bundled digest, that its operations equal the bundled operation set, and that serverCommit / artifactSha256 are well-formed. It fails in ~0.2 s in the Python contract job instead of four platform jobs twenty minutes later:
# before the record
AssertionError: No compatibility record matches the bundled Desktop contract c7f00e81…; build the wheel from a clean commit, run '… fixture_qualification.py --server-commit <sha>' and '… real_server.py', then record the profile in …
# after
3 passed in 0.19s
Local verification at 886a0cfb:
cargo test --locked --test session: 7 passed, 0 failed, includingbundled_qualification_allows_explicit_scope_browsing.uv run --no-sync pytest tests/test_desktop_contract.py -q: 3 passed.ruff check/ruff format --checkclean.- Honest caveat on scope: this repository requires Rust 1.95 and my host only had 1.68, so I installed 1.95 to build
server_probeand to run the suite.cargo test --libstill fails twodiagnostic_processcases here withOs { code: 1, kind: PermissionDenied }atdiagnostic_process_macos.rs:349/420— the sandbox denies process enumeration/signalling, it is independent of this change (the diff touches no Rust file), and those two need the real CI runners to clear.
Pushed as 886a0cfb; the four desktop jobs are re-running against it.
`openapi/powercontext.yaml` gained four paths in `11f4319c`, so the bundled contract digest moved from `c12901b8…` to `c7f00e81…`. The Desktop preview resolves its compatibility profile by that digest, and `bundled_qualification_allows_explicit_scope_browsing` therefore panicked on four platforms: the shipped preview had no qualified Server for its own contract. Only the digest had moved; no qualification record followed it. `sqlite-atomic-cf81c1a8-v1` is that record. It was produced by the existing harness rather than written by hand: | Field | Value | | --- | --- | | `serverCommit` | `cf81c1a8a9c40c1fe186944ba6e334b54aa5da70` (clean tree) | | `contractSha256` | `c7f00e81…` (bundled contract) | | `artifactSha256` | `0530ed69…` (`powercontext-1.2.1.dev414+gcf81c1a8a…whl`) | | `operations` | the eleven bundled operations, including `get_server_info` | `desktop/tests/real_server.py` passed all four modes on macOS 27.2 ARM64 with Python 3.14.7 and Rust 1.95.0 — anonymous loopback, static Bearer, HTTPS with an explicit CA and base path, and an injected provider with enforced access — and all 632 packaged runtime files in the wheel match the source bytes at that commit. The report records `desktopWorkingTreeDirty=false`. `VALIDATION.md` gains the matching section and now names this record as the current-contract qualification; the `c12901b8…` records keep their original identities. The new endpoints are deliberately not part of the Desktop operation set: the record does not claim Desktop support for them, and the Python routes tests cover them instead. The gap that let this reach review was that the requirement only lived in `cargo test`. `tests/test_desktop_contract.py` now checks it too, so a digest without a record fails the Python contract job in under a second instead of four platform jobs twenty minutes later.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
hidb4ai
left a comment
There was a problem hiding this comment.
@jasondeng1997 Re-reviewed 886a0cfb049738440dbe079a56b7d971f6b5aa52. The scoped Operation routes, shared rate limiting, and migration acceptance test now work in my focused checks. Deferring the asynchronous flush response is explicit in the updated documentation.
There is still a blocking Memory commit/cancellation failure, detailed inline: the operation becomes cancelled while its Memory result and cursor are committed. I also confirmed that the role-isolation part of the existing deployment thread remains unresolved, and that the shipped Compose migrator still invokes the nonexistent server migrate command; I am replying in those existing threads rather than duplicating them.
Please fix these paths and provide an updated commit before approval. My execution evidence uses SQLite, ASGI, and a controlled extractor; it does not establish real OceanBase multiprocess acceptance.
| ) | ||
| _require_exact_cursor(payload, current_sequence, current_generation) | ||
| try: | ||
| flushed = await self._contexts.process_memory(claim.scope_id, self._window_limit) |
There was a problem hiding this comment.
[P1] Fence the Memory publication before committing domain state
process_memory() publishes Memory artifacts and advances the cursor during prepare(), before DurableWorker checks the claim/cancellation in WorkRepository.complete(). I reproduced this with the real SQLite processor and a paused extractor, with no Scheduler running: while extraction was paused, the scoped cancel endpoint returned 200 / cancelling; after releasing the extractor, the operation became cancelled, but one Memory artifact was persisted and its Source cursor advanced from 0 to 1. The later ledger fence cannot roll that domain write back. Validate the live claim and cancellation in the same transaction as the Memory/cursor publication and Work completion, so a cancellation accepted before publication discards the prepared result. Lease takeover needs the same protection.
|
@jasondeng1997 The PR is still at
These are the existing findings, not additional duplicate findings. Please include cancellation/publication, supported Compose migration, and role-route regression evidence with the fixes. The PR also remains conflicting with master. The current cindy-package failure is also a concrete build gate: its tested merge commit |
| '422': {$ref: '#/components/responses/InvalidRequest'} | ||
| '500': {$ref: '#/components/responses/InternalError'} | ||
| '503': {$ref: '#/components/responses/Unavailable'} | ||
| /v1/scopes/{scope_id}/operations: |
There was a problem hiding this comment.
[P1] 新增的 4 个 operation 没有让已提交的 Cindy 生成物同步,cindy-package 在本 head 确定性失败
make cindy-test 先用 integrations/dsh/plugins/powercontext 的源码经 tsdown --config ../../../cindy/tsdown.config.ts 重新生成 integrations/cindy/plugins/powercontext-hooks/node/worker.cjs,再用 git diff --exit-code 与仓库里提交的那一份做逐字节比对。本行起的 4 个 path 会进入该生成物的 operations 表,而提交进仓库的 worker.cjs 没有跟着更新。
证据(head 886a0cfb,Main run 41006825999 / job 123926048295):
- 步骤
Build and test the standalone Cindy archive= failure,命令make cindy-test,日志末尾是git diff --exit-code的输出,随后Error 1。 - diff 内容是这 4 个 operation 的纯新增条目:
list_operations(GET /v1/scopes/{scope_id}/operations)、get_operation、cancel_operation、retry_operation,operationId与path与本 PR 的openapi/powercontext.yaml完全一致。 - 归因不是基线漂移:
git show master:openapi/powercontext.yaml中scopes/{scope_id}/operations命中 0 处,这 4 个 path 由本 PR 引入。 - 为什么本地看不到:本 head 的树里没有
integrations/cindy/(它在本 PR 的 merge-basebdf99789之后才进入 master)。该 run 是pull_request事件,actions/checkout检出refs/pull/2270/merge,工作树里 master 的integrations/cindy/与本 PR 的契约同时存在,因此这个红只出现在合并结果上。
修复方向:rebase 到当前 master 后重跑一次 make cindy-test 的生成流程,把重建后的 integrations/cindy/plugins/powercontext-hooks/node/worker.cjs 一起提交,不要手改。这与本 PR 已经修过的 desktop/ui/src/generated/api.d.ts 属同类要求:契约变更必须让所有已提交的生成物同步重建,而 cindy-package 这个 job 就是对应门禁。
There was a problem hiding this comment.
状态更新(head 5fa749d8,与本条发布时相同):本条指出的 cindy 生成物缺口在当前 head 上已不存在。
在 exact head 的归档副本上按仓库门禁的命令重建后比对:
integrations/cindy/plugins/powercontext-hooks/node/worker.cjs:pnpm exec tsdown --config ../../../cindy/tsdown.config.ts(工作目录integrations/dsh/plugins/powercontext,pnpm install --frozen-lockfile之后)重建结果与提交值 sha256 均为b13c51e0600157be761ed4cca8aa28e09d2a81e903315571ef6d63e47152e546。Windows 上重建文件头部 14 行为 CRLF,按 LF 归一化后与提交值相等;Linux runner 上该差异不存在。integrations/dsh/plugins/powercontext/lib/index.js:pnpm run build重建结果与提交值 sha256 均为6370af714a56d37a5ab0d34198b1cbd2fe51fc245d533ee61f11a8aa5a1ff272,未做行尾归一化即字节相等。- 两个文件里
list_operations/get_operation/cancel_operation/retry_operation的operationId与path与openapi/powercontext.yaml一致。
说明:本 head 没有任何 check run(mergeable_state=dirty,CI 未被触发),cindy-package 未在 CI 上执行,上面是本地重建结论,不是 CI 结论。
本条不再挂 P1。同一 PR 仍存在的阻塞项是迁移链位置问题,见 v1_5_0_001_artifact_storage.py:23 线程 4440855384,与生成物重建无关。
Master took the v1_4_1_005 and v1_4_1_006 revision names, so the ledger revision moves to v1_4_1_007 and v1_5_0_001 now follows it: - rename the ledger migration, both schema snapshots and both manifests - keep master's atomic-memory and source-write-plugin layouts, then chain the ledger layout on top in vector_profile - thread create_schema through OceanBase's new _connect() seam - keep both composition.py additions (WorkBackground, compose_artifact_search) - union the doc chains, obloader restore layers and upgrade rationale - regenerate the API, JS operations and Desktop contract artifacts for the merged contract digest 3a89f413c1, and rebind persistence_sha256 - repair tests/test_desktop_contract.py, where the conflict resolution had kept a diff3 base section and dropped master's business-profile test
…merge The merge renumbered the ledger revision but left four stale references: - the portable SQLite chain still carried v1_4_1_006_work_ledger.py next to master's v1_4_1_006_source_write_plugins.py, so Alembic saw two heads; rename it to v1_4_1_007 and point v1_5_0_001 at it - both manifests mapped v1_4_1_007 to the v1_4_1_006 schema resources; point them at the 007 snapshots - the recorded SQLite fingerprints for v1_4_1_007 and v1_5_0_001 predate master's 005/006 tables; re-measure them by replaying the merged chain (all earlier revisions reproduce their recorded digests unchanged) - SeekDBProfile._connect referenced create_schema without receiving it - repair the obloader Layer-1 quoting dropped during the doc merge - update the migration-name table, source chains and pending examples - align the tests that enumerate the revision chain
The merge moved the bundled contract digest to 3a89f413…, which no record covered. Build the wheel from the clean d9f33be commit, verify all 685 packaged files byte-for-byte against it, and run the real Server harness: all four modes pass with desktopWorkingTreeDirty=false. Record sqlite-atomic-d9f33be3-v1 with its section in VALIDATION.md, restore the sqlite-atomic-cf81c1a8-v1 record that the merge dropped, and point the Compatibility scope paragraph at the new record.
There was a problem hiding this comment.
🟡 Changes recommended
Critical migration defects block deployment and existing-database upgrades, while Worker and Scheduler services expose unauthenticated business APIs within the Compose network.
7 open issues
Replace unsupported migration command with the required db-migrate workflow · New Restrict Worker and Scheduler listeners to prevent unauthenticated API access · New Append a migration revision to preserve upgrades from the existing bundle head · New Expect the current migration bundle head in the fresh-migration test · New Consider both DST folds when finding the next local-time schedule · New Pass the ServerMetrics observer through ledger composition · New Add expired rate-limit window cleanup for single-node configurations · New
Review effort: Balanced
| migrate: | ||
| <<: *runtime | ||
| restart: "no" | ||
| command: ["server", "migrate"] |
There was a problem hiding this comment.
这条也独立复核成立,而且后果比“命令写错”更直接。
src/powercontext/server/cli.py 注册的迁移入口是 app.add_typer(database_migration_app, name="db-migrate")(第 113 行),另有 processing-migrate、atomic-memory-migrate、atomic-memory-rebuild-projection 等命令,没有 migrate。因此 command: ["server", "migrate"] 会以 Typer 用法错误退出(非 0)。
再叠加本文件自身的依赖关系:api-a 是 depends_on: migrate: condition: service_completed_successfully,migrate 一旦非 0 退出,api-* 与 worker 都不会启动——即 docker compose -f docker/compose.distributed.yaml up 这条本 PR 新增的分布式部署入口整体不可用。
建议改为 ["server", "db-migrate", "apply", "--yes"](是否需要 --maintenance-confirmed 取决于首次初始化路径),并补一条最小 smoke:在 CI 里对该 compose 做一次 docker compose config 之外的命令存在性检查,例如 server migrate --help 应为非 0、server db-migrate --help 应为 0。
|
|
||
| worker-a: | ||
| <<: *runtime | ||
| command: ["server", "run"] |
|
|
||
| revision = "v1_5_0_001" | ||
| down_revision = "v1_4_1_006" | ||
| down_revision = "v1_4_1_007" |
There was a problem hiding this comment.
独立复核成立,并且这个风险今天已经在计时。
- master 上该 revision 是
down_revision = "v1_4_1_006"(git show refs/heads/master:src/powercontext/builtin/persistence/migrations/server_resources/versions/v1_5_0_001_artifact_storage.py),本 PR 的 diff 只有这一行变化(SQLite 与mysql/versions/v1_5_0_001.py两份都是v1_4_1_007),新增的v1_4_1_007里down_revision = "v1_4_1_006",即把 ledger 插到了已登记 head 之前。 - 这条 head 正是下一个发行版的迁移目标:chore(release): prepare v1.5.0 and refresh product screenshots #2395(
chore(release): prepare v1.5.0)把release/upgrade-support.json的target_version定成1.5.0,两份operate/database-migrations.md与tests/release_upgrades/README.md也统一写到v1_5_0_001。
所以只要 1.5.0 先打 tag,之后再来本 PR:已经 stamp 成 v1_5_0_001 的库仍是 alembic head,v1_4_1_007 永远不会执行,pc_source_write_plugins 之外的 pc_work_* 一组表在这些库上不存在;而 1.5.0 用户拿到的文档又把目标链写成以 v1_5_0_001 结尾。
建议:把本 revision 改成排在当前 head 之后的编号(例如 v1_6_0_00x,或与 1.5.0 同批的 v1_5_0_00N),不要修改任何已登记 revision 的 down_revision;如果确定要在 1.5.0 内发布,就需要与 #2395 合并顺序绑定,并在发布说明里同步链尾。
| work_table = connection.execute( | ||
| "SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'pc_work_items'" | ||
| ).fetchone() | ||
| assert revision == [("v1_4_1_006",)] |
| candidate = datetime.combine(candidate_date, time(hour, minute), self.timezone) | ||
| if candidate < start or not _is_local_time(candidate): | ||
| continue | ||
| return candidate.astimezone(UTC).replace(tzinfo=None) |
| self.work_enqueues = Counter( | ||
| "powercontext_work_enqueues_total", | ||
| "Durable logical work enqueue decisions.", | ||
| ("kind", "outcome"), | ||
| registry=self.registry, |
| if database is None: | ||
| await self.app(scope, receive, send) | ||
| return | ||
| decision = await self._consume(database, _principal_key(scope)) |


Which issue or RFC does this PR close?
Tracks RFC 1430 — distributed Server workers /
#1430. This is the runtime Work Ledger rebased onto current
master; it is deliberately narrowed to the ledger, its roles, and the transport that makes durable work observable.Rationale for this change
Background processing had two problems. Every process that wanted to run Memory, Profile or Experience work needed its
own APScheduler sidecar and SQLite state, so two Servers could not coordinate; and a caller had no way to see what a
bounded Source window was doing, stop it, or recover it after a failure. The Work Ledger makes one logical Source window
a database row with a fenced lease, so any process can observe it and exactly one process can act on it at a time.
What changes are included in this PR?
Persistence.
builtin/persistence/work.pyadds the work item, lane and logical-key tables with an optimisticstate_version, database-time leases, fences, and the operations that move a window throughqueued → running → retry_wait → succeeded/failed/cancelled.coordination.pyadds leader election and membership heartbeats.migrations/server_resources/schemas/mysql-v1_4_1_006.jsoncarries the schema (v1_4_1_006avoids the number alreadytaken by #2192).
Runtime.
DurableSchedulerscans keyset-paginated discovery pages as a fenced leader;DurableWorkerclaims withcapacity, renews leases and recovers abandoned attempts;
work_handlers.pyimplements the Memory, Profile and Experiencesource-window handlers and the maintenance handler;
composition.pyassembles components by deployment role(
single_nodekeeps the APScheduler supervisors,distributedsplitsapi/scheduler/worker/all), with eachFamily owned by exactly one path.
operations.pyexposes theOperationManagerfacade.Server. Deployment-role validation (including the third
BuiltinConfigconstruction site incli/config.py), thescoped durable operation endpoints described below, a shared rate-limit middleware, and worker observability (OTLP spans
and model usage per attempt).
Docs. RFC 1430 (en/zh), deployment, configuration and HTTP API pages; the operation endpoints are documented with an
explicit statement of what is still proposal material.
Are there any user-facing changes?
Yes, three groups:
New endpoints — durable operations are served under the owning Scope, so authorization resolves from the request
and another Scope's row cannot be probed:
GET /v1/scopes/{scope_id}/operations— page public operations, filter bykind/status(scope.read)GET /v1/scopes/{scope_id}/operations/{operation_id}(scope.read)POST /v1/scopes/{scope_id}/operations/{operation_id}/cancel—expected_versionrequired (scope.contribute)POST /v1/scopes/{scope_id}/operations/{operation_id}/retry— failed operations only (scope.contribute)A stale
expected_versionreturns409 operation_conflictand leaves the row untouched. Another Scope's operation,and internal maintenance work, return
404.New configuration sections —
deployment,coordination,worker,operations,rate_limit. Adistributeddeployment requires OceanBase for coordination and ageneration_modelonworker; host-localexternal Skill targets are rejected because they cannot be reproduced on another replica.
Behavior — in
single_nodenothing changes: the APScheduler supervisors still own processing and the ledger isnot opened. In
distributedthe ledger owns Memory, Profile and Experience windows.POST /v1/memory/flushis stillsynchronous (
200); the202/Prefer: respond-asynchandle described in RFC 1430 is not implemented, and thedocs now say so.
How was this change tested?
make contract-test→ 56 passed (contract, generated Python transport, JS operations, Desktop digest consistent)tests/test_operation_routes.py(new) → 2 passed: paging and filters, cross-Scope404, maintenance404,stale-version
409, cancel →cancelled, cancelled-is-not-retryable, and a genuinely failed window retrying toqueuedwith cleared error fieldstests/test_server.py+tests/e2e/test_operation_api.py+ docs contract tests → 70 passedtests/builtin/runtime+tests/builtin/persistence+ server metric/tracing/health suites → 1055 passed / 1 skippedtests/test_config_cli.py→ 45 passed (role validation)tests/e2e/test_zcode_service_chain.py→ passed repeatedly; the boundary-flush assertion now re-issues the boundedStopflush instead of racing the hook's 800 ms budgetpnpm test428 passed,test:e2e9 passed; the committed bundle is rebuilt from the regenerated catalogAI usage statement
Implementation, tests and documentation were drafted with an AI coding assistant (WorkBuddy / CodeBuddy) under author
review; every command in the list above was run by the author on this head, and the contract, generated transport and
Desktop digest were regenerated and re-verified rather than hand-edited.