fix(#249): schedule health endpoint accessible to CanCommunicate peers - #403
Closed
HongmingWang-Rabbit wants to merge 6 commits into
Closed
fix(#249): schedule health endpoint accessible to CanCommunicate peers#403HongmingWang-Rabbit wants to merge 6 commits into
HongmingWang-Rabbit wants to merge 6 commits into
Conversation
…icationSender PushNotificationSender is an ABC with an abstract send_notification() method — instantiating it raises TypeError at runtime, crashing every workspace agent on startup (issue #204, found in Security Audit Cycle 4). Fix: use BasePushNotificationSender(httpx.AsyncClient(), config_store) which is the concrete implementation. Share the push_config_store instance between DefaultRequestHandler and the sender so they operate on the same store. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…der comment Add workspace-template/tests/test_main_startup.py with 12 regression tests guarding against re-introducing the abstract PushNotificationSender crash that broke every workspace agent on startup (issue #204). Test categories: - Source-code guards (4): verify main.py never imports or instantiates bare PushNotificationSender, that it uses BasePushNotificationSender, and that main.py compiles cleanly. - Real SDK checks (4): load the installed a2a.server.tasks and a2a.server.request_handlers bypassing conftest mocks to verify PushNotificationSender is abstract, BasePushNotificationSender is concrete, constructor signature is correct, and DefaultRequestHandler push_sender defaults to None. - Capability guards (4): AgentCapabilities has stateTransitionHistory=True, pushNotifications is read from config (not hardcoded False), push_config_store is shared between handler and sender, and cancel() is not a pass stub. Also adds an inline comment in main.py explaining why BasePushNotificationSender is used rather than the bare PushNotificationSender (issue #204). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e peers Silent cron failures were undetectable by peer agents because the only schedule health fields (last_run_at, last_status, run_count) sat behind WorkspaceAuth, which requires the workspace's own bearer token — a token peers never hold. New endpoint: GET /workspaces/:id/schedules/health - Registered outside the wsAuth group (mirrors /workspaces/:id/a2a pattern) - Auth: caller must supply X-Workspace-ID + its own bearer token (Phase 30.5 lazy-bootstrap: callers with no tokens are grandfathered through) - Gate: registry.CanCommunicate(callerID, workspaceID) must pass - Response: id, name, enabled, last_run_at, next_run_at, run_count, last_status, last_error — prompt and cron_expr intentionally omitted - System callers (webhook:*, system:*, test:*) and self-calls bypass both token validation and CanCommunicate (same as A2A proxy) 7 new tests cover: missing caller (401), self-call allowed, legacy peer grandfathered, non-peer denied (403), system caller bypass, no prompt exposure, DB error → 500. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
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.
Problem
GET /workspaces/:id/schedulesis behindWorkspaceAuth, which requires the target workspace's own bearer token. Peer agents in the org hierarchy never hold that token — so they cannot readlast_run_at,last_status, orrun_count, making silent cron failures completely undetectable from the outside. Tracked in #249.Fix
New endpoint:
GET /workspaces/:id/schedules/healthRegistered outside the
wsAuthgroup (mirrors the/workspaces/:id/a2apattern). Auth is enforced by the handler itself:X-Workspace-IDheader required (else 401)callerID == targetID) pass immediatelysystem:*,webhook:*,test:*) pass immediatelyvalidateCallerToken+CanCommunicategate (same pattern as A2A proxy)Response is a reduced
scheduleHealthResponse— exposes onlyid,name,enabled,last_run_at,next_run_at,run_count,last_status,last_error. Never exposespromptorcron_expr.Also includes (bundled since they share the branch)
PushNotificationSenderwithBasePushNotificationSender(concrete) inworkspace-template/main.py— fixes startup crash introduced in PR fix(a2a): cancel(), capabilities, push store, startup crash fix (closes #173 #174 #175 #204) #198.Test plan
TestScheduleHealth_MissingCallerID_Rejected— 401 when no X-Workspace-IDTestScheduleHealth_SelfCall_Allowed— 200 self-call, no CanCommunicate queriesTestScheduleHealth_CanCommunicatePeer_LegacyNoToken— grandfathered peer (0 tokens) + CanCommunicate allowed → 200TestScheduleHealth_AccessDenied_NonPeer— CanCommunicate denied → 403TestScheduleHealth_SystemCaller_Allowed— system:monitor bypasses all checks → 200TestScheduleHealth_NoPromptExposed— response must not contain prompt/cron_expr/timezone fieldsTestScheduleHealth_DBError_Returns500— DB error on health SELECT → 500TestHistory_IncludesErrorDetail— history endpoint surfaces error_detailBasePushNotificationSenderinstantiation tests (12 tests in test(BUG: workspace-template/main.py crashes on startup — PushNotificationSender is abstract and cannot be instantiated #204) commit)🤖 Generated with Claude Code