feat(context): unify Context.create() signature per apcore #66#28
Merged
Conversation
Per apcore/CLAUDE.md, implementation repos contain only code and a README — feature specs, test-case matrices, and design notes live in the apcore protocol-spec repo. Removes: - docs/features/async-task-evolution.md - docs/features/middleware-architecture-hardening.md - docs/async-task-evolution/test-cases.md These files were also stale (referenced the deprecated TaskStore.put method and the removed TaskStatus.RETRYING enum value); the canonical sources are the implementation in src/apcore/async_task.py and the upstream spec at apcore/docs/features/async-tasks.md. Signed-off-by: tercel <tercel.yi@gmail.com>
…nager surface
Implements four sync-audit findings against v0.22.0:
* A-D-AT-04 / D-17 — TaskStore Protocol methods (save, get, delete,
list, list_expired) are now async on the Protocol and on the default
InMemoryTaskStore. AsyncTaskManager awaits all store calls; a
transitional shim still drives sync stores so external custom stores
get one deprecation window to migrate. cleanup() is async because
list/delete now are.
* A-D-AT-03 / D-11 — ReaperHandle.stop and AsyncTaskManager.stop_reaper
are async and drain the reaper task before returning. Callers no
longer need a manual `await asyncio.sleep(0)` after `handle.stop()`.
* A-D-AT-06 — get_status / list_tasks return shallow copies via
dataclasses.replace so callers cannot mutate the live store record.
New get_status_async / list_tasks_async surfaces for I/O-backed
stores that genuinely suspend.
* A-D-AT-09 / D-14 — Legacy RetryPolicy now defaults max_retries to 0
(retries are strictly opt-in across all SDKs) and emits a
DeprecationWarning on instantiation steering callers to RetryConfig.
Adds tests/test_async_task_sync_audit_v022.py pinning each invariant
plus an async-only custom-store smoke test. Updates existing tests to
the new async surface (await store ops, await stop_reaper, await
cleanup) and registers an existing sync test as `@pytest.mark.asyncio`
where required.
Signed-off-by: tercel <tercel.yi@gmail.com>
…wrap
Two normative-decision fixes for the executor pipeline:
* A-D-EXEC-002 / D-21 — BuiltinCallChainGuard now observes
cancel_token.is_cancelled before any guard work and short-circuits
with ExecutionCancelledError. Combined with the existing Step 8
check this satisfies the two-point cancel-token invariant; the
earlier single-check implementation leaked compute through
ACL/middleware/validation even when the caller had already
cancelled.
* A-D-EXEC-005 / D-22 — Executor._recover_from_call_error now unwraps
MiddlewareChainError and propagates its `.original` typed cause
unchanged. Previously the wrapper was collapsed to a generic
ModuleExecuteError, breaking callers that dispatch on typed errors
(notably MCP/A2A bridges keying on APPROVAL_DENIED vs
MODULE_EXECUTE_ERROR). Mirrors the TypeScript and Rust SDKs.
Adds tests/test_executor_sync_audit_v022.py with regression coverage
for both decisions. Drops the now-unused ModuleExecuteError import
from executor.py.
Signed-off-by: tercel <tercel.yi@gmail.com>
…ll paths
Four sync-audit findings clamp the Issue #65 invariants so they hold
uniformly across every registration path, not just the public
register() API:
* A-D-REG-002 — register_internal's ephemeral-namespace check now
uses the shared _is_ephemeral helper instead of bare .startswith,
so the bare ID "ephemeral" is rejected too (it previously slipped
through the .startswith("ephemeral.") guard).
* A-D-REG-003 — _register_in_order (discover path) now follows the
same three-phase deferred-publish protocol as register(): reserve
in-flight slot → run on_load outside the lock → publish on
success. Previously the discover path published into _modules
BEFORE invoking on_load, leaving a window in which registry.get()
callers could observe a module whose on_load-installed state was
incomplete.
* A-D-REG-004 — register_internal now uses the same three-phase
protocol. The Issue #65 invariant ("modules invisible until
on_load completes") now holds for sys-modules too.
* A-D-REG-005 — _invoke_on_load now emits
apcore.registry.module_load_failed when on_load fails, mirroring
the public register() path so subscribers have a single hook for
partial-init detection regardless of which registration route was
used.
Adds tests/test_registry_sync_audit_v022.py with regression coverage
for all four findings, including a probe module whose on_load asserts
the module is NOT yet visible in the registry while the callback runs.
Signed-off-by: tercel <tercel.yi@gmail.com>
Implement the v0.22.0 unified Context.create() factory signature defined in the apcore PROTOCOL_SPEC §"Contract: Context.create" and §"Contract: Executor binding to Context" (Issue #66). The factory now accepts exactly six caller-supplied fields in this order: identity, trace_parent, cancel_token, data, services, global_deadline. The previous `executor=` parameter is removed; the Executor binds itself to the Context at pipeline entry via the new SDK-internal `Context._bind_executor` helper. `cancel_token` and `global_deadline` are now first-class parameters, eliminating the post-hoc `ctx.cancel_token = token` anti-pattern. Same-instance rebinds are idempotent noops; cross-Executor rebinds raise the new `ContextBindingError` (code `CONTEXT_BINDING_ERROR`). Auto-binding is applied at every Executor entry point: call, call_async, stream, call_async_with_trace, and the _validate_async dry-run path. `BuiltinContextStep` now computes `global_deadline` for any root call (empty call_chain) based on local executor.global_timeout, matching the PROTOCOL_SPEC requirement that deserialized Contexts arriving at a remote node MUST recompute global_deadline from local config. Adds a new conformance harness (test_context_create_unified_signature) driven by apcore/conformance/fixtures/context_create.json, covering all 15 canonical cases including create defaults, cancel_token / global_deadline plumbing, executor binding rules, child propagation, deserialization behavior, distributed cancel/deadline semantics, and tracestate inside TraceParent. Updates examples/cancel_token.py to the new shape and migrates ~25 test call sites. Breaking for callers that previously passed `executor=` to `Context.create()`; acceptable pre-release for v0.22.0. Refs apcore#66 Signed-off-by: tercel <tercel.yi@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the Python SDK side of apcore Issue #66.
Summary
Context.create()signature:identity, trace_parent, cancel_token, data, services, global_deadline(kwargs-only, in spec order).executor=andcaller_id=parameters removed.Context._bind_executor()private helper. NewContextBindingError(codeCONTEXT_BINDING_ERROR).call,call_async,stream,call_async_with_trace,_validate_async) — covers local construction, deserialized contexts, and hot-reload survivors uniformly per apcore §"Contract: Executor binding to Context".apcore/conformance/fixtures/context_create.json.ctx.cancel_token = tokenpattern across examples and tests.Sub-finding:
global_deadlinedistributed semantics fixWhile removing the
Context.create(executor=...)path, found thatBuiltinContextSteponly computedglobal_deadlinein the "context is None" branch. Caller-supplied contexts (including deserialized ones from remote nodes) lost their deadline. Fixed by detecting root calls viaempty call_chainand computing deadline in both branches — incidentally satisfying the new apcore §"Contract:global_deadlinedistributed semantics" MUST.Test plan
pytest -q→ 2999 passed / 0 failed / 2 skipped / 44 warnings on commit HEADContext.create()no longer acceptsexecutor=(signature-level removal)Context.create()no longer acceptscaller_id=(signature-level removal)ContextBindingErrorContext.child()propagates executor + cancel_tokenCross-references
context-create-unify-issue-66branches)