docs(planning): #408 dissolution gated on #428, not #306 (re-audit) - #522
Merged
Conversation
Add public webhook URLs so external systems (CI/CD, CRMs, monitoring) can
trigger agent schedule executions via a simple HTTP POST with no Trinity
account required — authenticated by a 256-bit opaque token embedded in the URL.
Changes:
- New public router POST /api/webhooks/{token}: rate-limited (10/60s per
token), audit-logged, 202 Accepted, delegates to existing scheduler trigger
- JWT-auth CRUD: POST/GET/DELETE /api/agents/{name}/schedules/{id}/webhook
- DB migration: webhook_token (TEXT UNIQUE), webhook_enabled (INTEGER DEFAULT 0)
on agent_schedules; partial unique index for O(1) token lookup
- Scheduler updated to accept triggered_by param in JSON body so executions
record triggered_by="webhook" correctly
- Webhook context field framed as data to reduce prompt injection surface
- 12 integration tests in tests/test_webhook_triggers.py
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bumps two dev-only dependencies to patched versions. Production is unaffected (happy-dom is test-only; vite only runs in local dev). - src/frontend: vite ^6.0.6 → ^6.4.2 (closes Dependabot #55, CVE-2026-39363) - tests/git-sync: happy-dom ^15.11.7 → ^20.9.0 (closes #83/#84/#85: VM context escape RCE, ESM code exec, fetch cookie leakage) Verified: frontend `vite build` clean, all 10 git-sync vitest tests pass under happy-dom 20.9.0. No new critical/high alerts introduced. Closes #485 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#484 review) Add missing documentation for the webhook trigger feature: - requirements.md: WEBHOOK-001 entry with description, key features, DB changes, API endpoints, security model, and feature flow link - architecture.md: webhooks.py listed in Routers table; Schedules table expanded from 9 to 12 endpoints with the 3 webhook management endpoints; new Webhook Triggers section documenting the public POST /api/webhooks/{token} endpoint; webhook_token and webhook_enabled columns added to agent_schedules schema block Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Merges feature/291-webhook-triggers into dev.
- Public POST /api/webhooks/{token} endpoint — no JWT, 202 Accepted,
rate-limited 10 calls/60s per token, audit-logged
- JWT-auth webhook management: POST/GET/DELETE /api/agents/{name}/schedules/{id}/webhook
- Token rotation, Redis rate limiting (fail-open), context injection with
prompt-injection framing, partial unique index for O(1) lookup
- 12 integration tests in tests/test_webhook_triggers.py
- requirements.md WEBHOOK-001 entry, architecture.md updated
Closes #291
Close the gap between "PR merged to dev" and "released to main". - New GH Action `issue-status-on-merge.yml`: on PR merge to dev, parse Fixes/Closes/Resolves #N from PR body+title, add `status-in-dev`, remove `status-in-progress`. - `/release` skill: read `gh issue list --label status-in-dev` as the authoritative shipping list for release notes; include `Closes #N` in the release PR body so issues auto-close on merge to main. - `DEVELOPMENT_WORKFLOW.md`: SDLC is now Todo → In Progress → In Dev → Done, each stage mapped 1:1 to commit-graph location. Fixes #488 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) (#494) Phase 2 of #354 polishes the shared channel-agnostic file delivery path in `message_router._handle_file_uploads`. Phase 1 (#355) added Telegram extraction/download/validation; the actual workspace write path was introduced for Slack inbound (#222). This change hardens the shared path for both channels: - New `_sanitize_filename` helper: NFKC unicode normalize → basename → safe-chars regex → empty/dotfile fallback to `file_{id}` → 200-char truncation preserving extension → collision dedup with `-1`, `-2`, … - Spec injection format: `[File uploaded by {uploader}]: {name} ({size}) saved to {path}`. Uploader is the verified email when present (Issue #311), else `adapter.get_source_identifier(message)`. - All-writes-failed handling: when every workspace write attempt fails, the router replies on the channel with an explicit error and skips agent execution (#487 AC6). Validation rejections (size/MIME/download errors) still surface in the description block as before. - Audit log entries gain an `uploader` field. Per-session upload directory (`/home/developer/uploads/{session_id}/`) preserved — keeps user uploads isolated and ephemeral, matches the existing #222 model. Tests: +17 unit tests across `TestFilenameSanitization` (12), `TestFileDeliveryFormat` (2), `TestFileDeliveryFailures` (3). 28/28 passing in `tests/unit/test_file_upload.py`. Docs: `telegram-integration.md` Phase 2 section + revision row; `slack-file-sharing.md` flow / router / errors / security sections updated for the shared change; `feature-flows.md` index row. Closes #487 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The WEBHOOK-001 commits (c630931 / 8fdf736) added `request: Request` parameters to `generate_webhook` and `get_webhook_status` without importing `Request` from fastapi. Backend module import fails with NameError on startup, blocking all dev deploys. Integration tests in tests/test_webhook_triggers.py exercise these endpoints but never caught the bug because the backend never starts — test setup fails before any test runs. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#500) services/backlog_service.py:240 lazy-imported _execute_task_background from routers.chat, but #95 deleted that function. Every backlog drain attempt failed with ImportError; the exception was swallowed at backlog_service.py:218-228, so BACKLOG-001 (#260) was silently dead. Live observation: 23 drain failures / 24h on a fan-out workload, only surface signal was the per-execution `error` column. Why it shipped silently: the unit happy-path test patched sys.modules["routers.chat"] with a SimpleNamespace stub of whatever attribute name it expected, masking the production breakage. Changes: - Lazy-import _run_async_task_with_persistence (the post-#95 replacement) and adjust the call shape (drop release_slot, drop orphaned task_activity_id; the unified executor handles both). - Capture self-task fields (is_self_task, self_task_activity_id, inject_result) at enqueue time and rehydrate on drain so SELF-EXEC-001 (#264) survives backlog overflow. - Emit a stable log token `backlog_drain_spawn_failed` so log-based detection (Vector / dashboards) can catch import drift or similar spawn-time regressions at fleet scale rather than per-row. - AST-based regression guard in tests/unit/test_backlog.py: TestLazyImportTarget parses routers/chat.py and asserts the import target exists; paired test asserts the lazy-import string matches the validated allow-list. Catches both directions of drift without booting the backend. - Update happy-path test to use the new symbol and kwarg surface; add self-task enqueue+drain round-trip tests. Closes #496 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bumps announce skill to v1.6. Adds a Python helper (scripts/post_twitter.py) that reads tweet text from stdin and posts via Twitter API v2 using OAuth 1.0a User Context — same exit-0/1 + structured-JSON contract as the existing Discord/Slack/Telegram send paths so the sequential-only and no-blind-retry rules apply uniformly. Credentials live in .env (gitignored) under ANNOUNCE_TWITTER_* keys. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sync parallel `/task` calls (parallel=true, async=false) at capacity used to fail terminally with HTTP 429 — they never touched the BACKLOG-001 backlog because the spill block was nested under `if request.async_mode:`. Observed in production: ~40% terminal-failure rate from one MCP fan-out caller (214 capacity rejections / 24h, 0 enqueues from 541 dispatches). Sync calls now spill to the same backlog the async path uses and long-poll on the open HTTP connection until the queued execution reaches a terminal status, then return the result inline. True 429 only when the backlog is also full. Total connection hold capped at 2 × effective_timeout. Implementation: - New `services/sync_waiter.py` owns the in-process registry and the `signal_sync_waiter` / `wait_for_sync_terminal` primitives. Wait combines an asyncio.Future (set by the drain finally block) with a 5s DB-poll fallback that covers terminal flips routed outside the drain (corrupt-metadata, expire_stale, cleanup recovery). - `routers/chat.py` sync branch now mirrors the async branch: pre-acquires the slot, on at-capacity calls `backlog.enqueue()` then `wait_for_sync_terminal()`, returns the inline result on wake. - `_run_async_task_with_persistence` wraps its body in try/finally and signals any registered sync waiter with the rich TaskExecutionResult plus chat_session_id. No-op when no waiter is registered (the common async fire-and-forget path). Tests (`tests/unit/test_chat_sync_backlog.py`, 13 new): - Signal / wait / poll-fallback / timeout / cleanup / concurrent waiters - Regression test pins TERMINAL_TASK_STATUSES to the enum so a new TaskExecutionStatus value forces a deliberate update (caught a missing SKIPPED entry pre-merge) Trade-off (Policy B): worst-case connection hold doubles to 2 × effective_timeout when the request is queued. Honest envelope — the caller chose to wait. Documented in the architecture diagram of `persistent-task-backlog.md`. Companion issue #505 covers the orchestration-education gap (MCP tool description + platform prompt) so agents pick the right tool for the job rather than relying on the platform absorbing every misuse. Closes #498 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…iation Adds SDLC context (Todo → In Progress → In Dev → Done) so grooming respects in-flight work, and a Step 1b that reconciles status-* labels with board columns (labels are authoritative). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… failure (#517) External signal terminations of the claude subprocess (timeout SIGKILL, OOM-kill, parent SIGTERM, operator cancel) used to fall through to the auth-fallback heuristics and surface as a misleading "Subscription token may be expired" 503. Same shape as #361 (max-turns), different exit path. Adds _classify_signal_exit() consulted before the auth heuristics: matches Python-native signal exits (return_code < 0) and shell-encoded forms (130/137/143 for SIGINT/SIGKILL/SIGTERM) and raises HTTP 504 with a clear "killed by SIGKILL/SIGTERM/SIGINT — likely timeout, OOM, or operator cancel" message. Tightens the zero-token heuristic with return_code > 0 so signal exits cannot reach it. The bug became routinely reproducible after #61 (PR #326) added backend-driven terminate_execution_on_agent() — every timeout now produces a signal-killed claude subprocess on the agent side, which the old heuristic block misclassified. Also de-risks PR #508 (auth-class auto-switch): without this fix, every timeout would trigger an unnecessary subscription rotation. Backend's task_execution_service.py only flags AUTH on 503; 504 falls through to the generic FAILED path. No backend changes required. Closes #516 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four divergences between the /sprint playbook and the SDLC documented in docs/DEVELOPMENT_WORKFLOW.md: - Step 3 used `gh issue edit --add-label status-in-progress` directly, bypassing .github/workflows/claim.yml and skipping self-assignment. Now posts `/claim` as an issue comment, which is the workflow's single source of truth for the In Progress transition. - Step 8 invoked pytest directly via `cd tests && source .venv/bin/activate && python -m pytest …`. Now defers to `/test-runner [feature]`, with a documented fallback for brand-new files outside the runner's catalog. - Step 10 commit + PR body used `closes #N`. Workflow §1 specifies `Fixes #N`; both auto-close on GitHub but the doc is the contract. - Step 11 final report didn't mention the post-merge automation. Now warns that issue-status-on-merge.yml owns the status-in-progress → status-in-dev transition, so operators don't manually edit labels post-merge. Non-breaking: argument signature, automation level (gated), state dependencies, and pipeline overview unchanged. Net +19/-11. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ss (#520) (#521) * fix(agent): classify clean-exit empty-result as 502, not silent success (#520) Sibling of #516/#517 on the return_code == 0 path. When the claude subprocess exits 0 but the final {"type":"result"} JSON line is dropped before the reader thread captures it (typical cause: a child subprocess inherited stdout, kept the pipe open past claude exit, the reader thread leaked, the pgroup unwind closed the pipe), metadata.cost_usd and metadata.duration_ms stay None. The success path used to return HTTP 200 anyway — agent-server logged "completed successfully" while backend silently reaped the execution as an orphan minutes later, masking the real failure with a misleading "completed on agent but recovered by watchdog" message. Adds _classify_empty_result(metadata, raw_message_count) consulted after the return_code != 0 block (#516 + auth heuristics) and before response building. When both cost_usd and duration_ms are None, raises HTTP 502 with diagnostic context (tools, turns, raw_messages, cause hint). Backend's task_execution_service.py:542 only flags AUTH on 503, so 502 falls through to the generic FAILED path with the helpful detail preserved — no backend changes needed. The two-field check is conservative: single-field nullability could be a Claude format quirk; both-None is a strong signal that the terminal result message never arrived. Test coverage pins the scope so a future edit can't silently broaden it. Changes: - docker/base-image/agent_server/services/claude_code.py — new _classify_empty_result() helper next to _classify_signal_exit; call site between the return_code != 0 block and response building. - tests/unit/test_empty_result_classification.py — 9 new tests, all pass. Covers both-None → 502, populated metadata → None, single-field-only → None (Claude format quirk tolerance), zero-cost and zero-duration → None (is None vs falsy), missing metadata → None. - docs/memory/feature-flows/parallel-headless-execution.md — changelog entry under Recent Updates. - docs/memory/feature-flows/task-execution-service.md — row in error-translation table + new Empty-Result Pre-Check paragraph. Requires base-image rebuild after merge: ./scripts/deploy/build-base-image.sh Closes #520 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(feature-flows): index entry for agent error classification (#516, #520) Combined Recent Updates entry covering the matching pair of agent-side error-classification fixes that shipped this week — _classify_signal_exit (#516, PR #517) and _classify_empty_result (#520, PR #521). Both touch docker/base-image/agent_server/services/claude_code.py and share the "agent surfaces the right HTTP status so backend records FAILED with a useful detail" theme. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…26-04-26) Strike item 13's stale "close as dissolved" guidance. Re-audit of services/task_execution_service.py:412-419 confirms the long-running HTTP call (agent_post_with_retry awaiting full agent response with timeout = timeout_seconds + 10) is still present. #306 changed only WebSocket transport, not backend→agent dispatch, so it cannot dissolve #408 on its own. Aligns item 13 with line 32, which already noted this on 2026-04-24. Refs #408
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.
Summary
ORCHESTRATION_RELIABILITY_2026-04.mditem 13.services/task_execution_service.py:412-419—#306changed WebSocket transport, not dispatch.Context
Plan item 13 told the next sprint to verify dissolution and close
#408. Today's audit found the synchronous wait is unchanged:This is the exact pattern
#408describes: backend coroutine held open for the full task duration, GIL contention with overlapping calls, uvicorn worker recycle drops in-flight connections.#306(Redis Streams) only changed WebSocket fan-out — not this dispatch path.The plan owner already verified this on 2026-04-24 (line 32: "long-running call still present, #306 alone insufficient"). Item 13 was never updated to match. This PR makes the two consistent.
#408stays open; dissolution is gated on#428(CapacityManager + push-completion).Out of scope (noted for next grooming pass)
#291 (WEBHOOK-001)is also stale in this doc — it shows as "PAUSED" in Sprint C sequencing and "UNBLOCKED, pending" in line 107 / item 12, but actually closed COMPLETED on 2026-04-24. Not fixed here to keep the diff tight; flag for/groom.Test plan
git diffreviewed — onlydocs/planning/ORCHESTRATION_RELIABILITY_2026-04.mdtouchedRefs #408
Generated with Claude Code