fix(a2a): remove non-existent InvalidSignatureError catch - #1645
Conversation
|
Warning Review limit reached
More reviews will be available in 33 minutes and 25 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR contains two independent updates: a coordination document entry recording planned execution lanes for the SPARK-KIMI node, and a code simplification that consolidates JWT authentication error handling into a single exception path, broadening the returned error detail message. ChangesCoordination and authentication updates
Possibly related PRs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pmoves/docs/AGENTS/AGNOTE4482PHI.t1.md`:
- Around line 975-976: The Lane D entry "Lane D — SPARK Model Orchestration"
lacks a branch/PR scope; update that line to include an explicit CLAIM scope
(e.g., add "CLAIM=branch-name/PR-<id>" or "CLAIM=docs-only/no-branch") so
ownership is unambiguous; ensure the agent_signature remains
`ACK::SPARK-KIMI::MULTI-LANE-CLAIM-2026-05-27` and, if relevant, note the
branch/PR in the delivery metadata (delivery=SPARK-KIMI) and any model routing
changes referenced (tensorzero.toml) for traceability.
- Around line 971-976: The claim header says "Three parallel lanes" but the list
defines four lanes (Lane A — A2A Runtime Verification, Lane B — NATS Auth
Secondary Batch, Lane C — PMOVES.YT Summarize + Chapters, Lane D — SPARK Model
Orchestration); update the header in the timestamped claim line (the line
starting with `2026-05-27T21:40:00Z` CLAIM `SPARK-KIMI` scope:) to reflect the
correct count by changing "Three parallel lanes" to "Four parallel lanes" (or
else remove/merge a lane so the count matches), leaving the rest of the lanes
and agent_signature intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d0c78721-69ac-4320-99da-89c94123efdd
📒 Files selected for processing (2)
pmoves/docs/AGENTS/AGNOTE4482PHI.t1.mdpmoves/services/agent-zero/python/features/a2a/server.py
💤 Files with no reviewable changes (1)
- pmoves/services/agent-zero/python/features/a2a/server.py
| - `2026-05-27T21:40:00Z` CLAIM `SPARK-KIMI` scope: Three parallel lanes on SPARK node (ARM64 + NVIDIA GB10, 128GB unified): | ||
| 1. **Lane A — A2A Runtime Verification** (`infra/a2a-runtime-verification`): Verify A2A router mounts at `/a2a/v1/*`, test `/.well-known/agent-card.json` discovery, auth via `mcp_server_token`, document secure activation path. Closes Known Gap P0. | ||
| 2. **Lane B — NATS Auth Secondary Batch** (`fix/nats-auth-secondary-batch`): Migrate 21-file secondary batch (vllm-orchestrator, supaserch/app.py, gateway-agent/nats_integration.py, benchmark-runner, agent-zero/python/events/bus.py) from unauthenticated `nats://(nats|localhost):4222` to env-driven `NATS_URL`. Closes Known Gap P0. | ||
| 3. **Lane C — PMOVES.YT Summarize + Chapters** (`feat/yt-summarize-chapters`): Add `/yt/summarize` and `/yt/chapters` endpoints to `pmoves-yt` service using host Ollama (Gemma/qwen3.5, expanded model selection on SPARK). Bounded NEXT_STEPS deliverable. | ||
| 4. **Lane D — SPARK Model Orchestration**: Audit host Ollama model roster against SPARK GB10 capacity (128GB unified), pull qwen3 family updates surfaced via channel-monitor ingestion, update `tensorzero.toml` model entries for local-first routing. | ||
| - Three-body: delivery=SPARK-KIMI, control=operator approval, memory=this trail. agent_signature: `ACK::SPARK-KIMI::MULTI-LANE-CLAIM-2026-05-27`. |
There was a problem hiding this comment.
Fix lane count mismatch in the claim summary.
Line 971 says “Three parallel lanes,” but Lines 972-975 define four lanes (A-D). Please align the summary text with the actual list to avoid coordination ambiguity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/docs/AGENTS/AGNOTE4482PHI.t1.md` around lines 971 - 976, The claim
header says "Three parallel lanes" but the list defines four lanes (Lane A — A2A
Runtime Verification, Lane B — NATS Auth Secondary Batch, Lane C — PMOVES.YT
Summarize + Chapters, Lane D — SPARK Model Orchestration); update the header in
the timestamped claim line (the line starting with `2026-05-27T21:40:00Z` CLAIM
`SPARK-KIMI` scope:) to reflect the correct count by changing "Three parallel
lanes" to "Four parallel lanes" (or else remove/merge a lane so the count
matches), leaving the rest of the lanes and agent_signature intact.
| 4. **Lane D — SPARK Model Orchestration**: Audit host Ollama model roster against SPARK GB10 capacity (128GB unified), pull qwen3 family updates surfaced via channel-monitor ingestion, update `tensorzero.toml` model entries for local-first routing. | ||
| - Three-body: delivery=SPARK-KIMI, control=operator approval, memory=this trail. agent_signature: `ACK::SPARK-KIMI::MULTI-LANE-CLAIM-2026-05-27`. |
There was a problem hiding this comment.
Add explicit branch/PR scope for Lane D to keep claim traceable.
Lane D currently has no branch/PR scope while the other lanes do. Please add it (or explicitly mark as docs-only/no-branch) so claim ownership is unambiguous.
As per coding guidelines, “claim the relevant lane with branch/PR scope (CLAIM)”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/docs/AGENTS/AGNOTE4482PHI.t1.md` around lines 975 - 976, The Lane D
entry "Lane D — SPARK Model Orchestration" lacks a branch/PR scope; update that
line to include an explicit CLAIM scope (e.g., add "CLAIM=branch-name/PR-<id>"
or "CLAIM=docs-only/no-branch") so ownership is unambiguous; ensure the
agent_signature remains `ACK::SPARK-KIMI::MULTI-LANE-CLAIM-2026-05-27` and, if
relevant, note the branch/PR in the delivery metadata (delivery=SPARK-KIMI) and
any model routing changes referenced (tensorzero.toml) for traceability.
python-jose does not define InvalidSignatureError on the jwt module. When an invalid signature was encountered, the except clause itself raised AttributeError, causing a 500 instead of the intended 403. The existing jose_jwt.JWTError catch-all already returns 403, so remove the broken intermediate handler. Closes AGNOTE4482 Lane A bug.
bfb54e3 to
dac8d12
Compare
- Lane A: A2A bug fix documented, PR #1645 opened - Lane C: PMOVES.YT summarize/chapters verified on SPARK
- Lane A: A2A bug fix documented, PR #1645 opened - Lane C: PMOVES.YT summarize/chapters verified on SPARK
…1646) * feat(yt): expose OLLAMA_URL and YT_SUMMARY_OLLAMA_MODEL to pmoves-yt Wire Ollama configuration env vars into the pmoves-yt service so SPARK (and other nodes) can route summary/chapter generation to the correct Ollama endpoint with the right model. Changes: - OLLAMA_URL: points to Ollama API (default: containerized service) - YT_SUMMARY_PROVIDER: summary backend (default: ollama) - YT_SUMMARY_OLLAMA_MODEL: concrete model id (default: gemma2:9b-instruct) SPARK nodes should set OLLAMA_URL=http://host.docker.internal:11434 and YT_SUMMARY_OLLAMA_MODEL to an available local model (e.g. qwen3.5:35b-a3b-q4_K_M). Closes AGNOTE4482 Lane C wiring. * docs(agnote4482): SPARK-KIMI Lane A+C progress - Lane A: A2A bug fix documented, PR #1645 opened - Lane C: PMOVES.YT summarize/chapters verified on SPARK --------- Co-authored-by: PMOVES-AGENT-ZERO-SPARK <pmoves-spark@powerfulmoves.com>
…1649) * feat(yt): expose OLLAMA_URL and YT_SUMMARY_OLLAMA_MODEL to pmoves-yt Wire Ollama configuration env vars into the pmoves-yt service so SPARK (and other nodes) can route summary/chapter generation to the correct Ollama endpoint with the right model. Changes: - OLLAMA_URL: points to Ollama API (default: containerized service) - YT_SUMMARY_PROVIDER: summary backend (default: ollama) - YT_SUMMARY_OLLAMA_MODEL: concrete model id (default: gemma2:9b-instruct) SPARK nodes should set OLLAMA_URL=http://host.docker.internal:11434 and YT_SUMMARY_OLLAMA_MODEL to an available local model (e.g. qwen3.5:35b-a3b-q4_K_M). Closes AGNOTE4482 Lane C wiring. * docs(agnote4482): SPARK-KIMI Lane A+C progress - Lane A: A2A bug fix documented, PR #1645 opened - Lane C: PMOVES.YT summarize/chapters verified on SPARK * fix(ci): correct Docker Compose image name in gateway-agent tag step Docker Compose v2 names images as projectname-servicename (dash), not projectname/servicename (slash). The tag step was referencing pmoves/gateway-agent but the built image is pmoves-gateway-agent. --------- Co-authored-by: PMOVES-AGENT-ZERO-SPARK <pmoves-spark@powerfulmoves.com>
Problem
python-jose does not define InvalidSignatureError on the jwt module. When an invalid JWT signature was encountered, the except clause itself raised AttributeError, causing a 500 instead of the intended 403.
Fix
Remove the broken handler. The existing jose_jwt.JWTError catch-all already returns 403 for all signature failures.
Verification
Closes AGNOTE4482 Lane A finding.
Risk
Low — 5-line deletion, no behavior change (JWTError already returned 403).
Summary by CodeRabbit
Documentation
Refactor