feat(jobs): per-filter event cursors, multi-replica tests, lease docs (closes #1252) - #1254
Merged
Conversation
…, and lease-semantics docs Each distinct recvEvent type-filter now tracks its own cursor — independent streams, exactly-once within a stream — so interleaved gates can no longer skip each other's events; per-filter locks let a long-poll on one type run concurrently with another. The mock and registry share one verified page contract. New uc33 integration tests prove poll-liveness (a quietly-gating job is never re-claimed) and supersession fencing (exactly one owner survives a genuine reclaim) on 2-replica consumers. Registration waits in the cold-JVM-sensitive suites poll until registered instead of sleeping a fixed 25s. Jobs man pages document lease/liveness rules, multi-replica fencing, cursor semantics, and the stale-timeout-vs-lease distinction. Closes #1252 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughUpdates job event cursoring and lease/fencing docs, adds UC33 multi-replica integration fixtures and scenarios, and replaces several integration-test waits with a shared agent-registration routine. ChangesPer-filter cursors, fencing docs, and UC33 tests
Estimated code review effort: 5 (Critical) | ~120 minutes Registration wait routine and test timeout updates
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/runtime/core/src/jobs.rs (1)
3714-3781: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftForce the concurrency tests to overlap.
These tests can pass without proving the lock behavior: the spawned receiver is not synchronized to enter
recv_event, andMockBackend::list_job_eventsreturns immediately. Use a small blocking/Notify test backend so the first call is known to be pending before the peer call starts; then assert different filters proceed while same-filter calls serialize.🤖 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 `@src/runtime/core/src/jobs.rs` around lines 3714 - 3781, The concurrency tests in recv_event_different_filters_do_not_block_each_other and recv_event_same_filter_monotonic_under_concurrency do not guarantee the calls actually overlap. Update them to use a small blocking/Notify-based test backend or synchronization point so the first recv_event call is definitely pending before starting the second, and then assert that different filters can proceed concurrently while same-filter calls serialize and observe distinct seq values.tests/integration/suites/uc06_observability/tc03_java_llm_tracing/test.yaml (1)
77-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood dependency-aware readiness check.
Unlike the sibling uc10 consumer-wait blocks (tc19/tc20/tc46), this analyst-wait loop verifies actual dependency resolution (
analyst.*1/1/analyst.*healthy), not just registration presence — this is the more robust pattern.Separately, this exact polling idiom (seq/sleep/grep/error-tail) is now duplicated verbatim across this file (x2), tc19/tc20/tc46, and uc20 tc07 (x2) — 7 occurrences total. Consider extracting it into a shared routine (the repo already uses
routines.yamlfor reusable setup elsewhere) to avoid drift as timeouts/messages get tuned independently over time.🤖 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 `@tests/integration/suites/uc06_observability/tc03_java_llm_tracing/test.yaml` around lines 77 - 124, This analyst readiness loop is correct, but the same polling pattern is duplicated in multiple test steps, which will drift over time. Extract the repeated seq/sleep/grep/error-tail logic into a shared reusable routine (following the existing routines.yaml pattern) and have the analyst check in this test call that shared helper while keeping the dependency-aware 1/1 or healthy condition intact. Reference the wait_provider and wait_analyst shell handlers, plus the similar tc19/tc20/tc46 and uc20 tc07 waits, when consolidating the logic.tests/integration/suites/uc10_toolcalls/tc22_consumer_java_provider_openai_py_tool_py/test.yaml (1)
94-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPolling-based registration wait looks correct.
Loop bound (120×2s=240s) matches the comment and stays within the 260s step timeout; failure path dumps logs before exiting non-zero, which aids debugging.
Note: this identical polling block is duplicated verbatim across many test.yaml files in this cohort (tc19, tc20, tc22, tc23, tc25, tc26, tc46, uc06/tc03, uc20/tc07). Consider extracting it into a shared routine (similar to existing
global.setup_for_python_agentroutines) parameterized by agent name, to avoid maintaining N copies of the same wait logic.🤖 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 `@tests/integration/suites/uc10_toolcalls/tc22_consumer_java_provider_openai_py_tool_py/test.yaml` around lines 94 - 114, The polling logic for waiting on consumer registration is duplicated verbatim across multiple test.yaml suites, which will be hard to maintain. Extract the repeated shell-based wait loop from this scenario into a shared routine like the existing global.setup_for_python_agent helpers, parameterized by the agent name (for example, the consumer identifier used by the wait step). Update the current step to call that shared helper instead of inlining the registration polling and log-dump logic.
🤖 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 `@src/runtime/core/src/jobs.rs`:
- Around line 756-766: The per-filter lock acquisition in jobs.rs currently
waits on filter_lock.lock() before the timeout budget starts, so queued calls
can exceed their intended limit. Update the locking flow around
Self::filter_key, recv_locks, and the filter_lock/_guard section so the deadline
is computed before awaiting the mutex, then acquire the lock by racing it
against the remaining timeout budget and fail early if the budget is exhausted.
Ensure the timeout used for the poll accounts for time spent waiting on the
same-filter lock.
In
`@tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yaml`:
- Around line 94-114: The wait step in the consumer registration shell command
only checks that analyst-java appears in meshctl list, which is insufficient for
readiness. Update the wait logic to use the same provider-ready signal as tc03,
and keep polling until analyst-java reports a healthy/ready state such as 1/1
rather than just registration. Use the existing wait-for-consumer step and
meshctl list/logs checks to locate the change, and ensure the analyze step only
runs after full readiness is confirmed.
---
Nitpick comments:
In `@src/runtime/core/src/jobs.rs`:
- Around line 3714-3781: The concurrency tests in
recv_event_different_filters_do_not_block_each_other and
recv_event_same_filter_monotonic_under_concurrency do not guarantee the calls
actually overlap. Update them to use a small blocking/Notify-based test backend
or synchronization point so the first recv_event call is definitely pending
before starting the second, and then assert that different filters can proceed
concurrently while same-filter calls serialize and observe distinct seq values.
In `@tests/integration/suites/uc06_observability/tc03_java_llm_tracing/test.yaml`:
- Around line 77-124: This analyst readiness loop is correct, but the same
polling pattern is duplicated in multiple test steps, which will drift over
time. Extract the repeated seq/sleep/grep/error-tail logic into a shared
reusable routine (following the existing routines.yaml pattern) and have the
analyst check in this test call that shared helper while keeping the
dependency-aware 1/1 or healthy condition intact. Reference the wait_provider
and wait_analyst shell handlers, plus the similar tc19/tc20/tc46 and uc20 tc07
waits, when consolidating the logic.
In
`@tests/integration/suites/uc10_toolcalls/tc22_consumer_java_provider_openai_py_tool_py/test.yaml`:
- Around line 94-114: The polling logic for waiting on consumer registration is
duplicated verbatim across multiple test.yaml suites, which will be hard to
maintain. Extract the repeated shell-based wait loop from this scenario into a
shared routine like the existing global.setup_for_python_agent helpers,
parameterized by the agent name (for example, the consumer identifier used by
the wait step). Update the current step to call that shared helper instead of
inlining the registration polling and log-dump logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a02bc62-fbd9-487b-a605-27d9a8ddc31d
📒 Files selected for processing (24)
docs/concepts/jobs.mdsrc/core/cli/man/content/environment.mdsrc/core/cli/man/content/jobs.mdsrc/core/cli/man/content/jobs_java.mdsrc/core/cli/man/content/jobs_typescript.mdsrc/runtime/core/src/jobs.rstests/integration/suites/uc06_observability/tc03_java_llm_tracing/test.yamltests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yamltests/integration/suites/uc10_toolcalls/tc20_consumer_java_provider_claude_py_tool_ts/test.yamltests/integration/suites/uc10_toolcalls/tc22_consumer_java_provider_openai_py_tool_py/test.yamltests/integration/suites/uc10_toolcalls/tc23_consumer_java_provider_openai_py_tool_ts/test.yamltests/integration/suites/uc10_toolcalls/tc25_consumer_java_provider_gemini_py_tool_py/test.yamltests/integration/suites/uc10_toolcalls/tc26_consumer_java_provider_gemini_py_tool_ts/test.yamltests/integration/suites/uc10_toolcalls/tc46_consumer_java_provider_claude_ts_tool_py/test.yamltests/integration/suites/uc20_tutorial/tc07_day07_committee/test.yamltests/integration/suites/uc33_meshjob_replicas/artifacts/gate-drivertests/integration/suites/uc33_meshjob_replicas/artifacts/gated-worker-atests/integration/suites/uc33_meshjob_replicas/artifacts/gated-worker-btests/integration/suites/uc33_meshjob_replicas/fixtures/gate-driver/main.pytests/integration/suites/uc33_meshjob_replicas/fixtures/gated-worker-a/main.pytests/integration/suites/uc33_meshjob_replicas/fixtures/gated-worker-b/main.pytests/integration/suites/uc33_meshjob_replicas/routines.yamltests/integration/suites/uc33_meshjob_replicas/tc01_quiet_gate_single_owner/test.yamltests/integration/suites/uc33_meshjob_replicas/tc02_supersession_fences_stale_owner/test.yaml
Comment on lines
+94
to
+114
| # Liveness only: poll until the Java consumer REGISTERS instead of a fixed | ||
| # sleep — cold-JVM starts (mvn compile + Spring Boot startup) routinely | ||
| # outran the old 25s wait. Generous 240s deadline; dependency resolution | ||
| # stays covered by the runtime's settle window (no dep-resolution wait). | ||
| - name: "Wait for consumer registration" | ||
| handler: shell | ||
| workdir: /workspace | ||
| command: | | ||
| echo "Waiting for analyst-java to register..." | ||
| for i in $(seq 1 120); do | ||
| if meshctl list 2>/dev/null | grep -q "analyst-java"; then | ||
| echo "analyst-java registered after ~$((i*2))s" | ||
| exit 0 | ||
| fi | ||
| sleep 2 | ||
| done | ||
| echo "ERROR: analyst-java did not register within 240s" | ||
| meshctl logs analyst-java 2>/dev/null | tail -50 || true | ||
| exit 1 | ||
| capture: wait_consumer_registration | ||
| timeout: 260 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target test and the analogous uc06 tc03 check.
git ls-files 'tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yaml' \
'tests/integration/suites/uc06_toolcalls/tc03_*/test.yaml' \
'tests/integration/suites/**/test.yaml' | sed -n '1,120p'
echo '--- target file excerpt ---'
cat -n tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yaml | sed -n '70,140p'
echo '--- search for registration checks and meshctl list usage ---'
rg -n --hidden -S 'meshctl list|1/1|healthy|analyst-java|claude-provider-py|Non-tool query|analyze' tests/integration/suites
echo '--- analogous tc03 file excerpt if found ---'
python3 - <<'PY'
import pathlib, re
matches = list(pathlib.Path('tests/integration/suites').rglob('test.yaml'))
for p in matches:
txt = p.read_text(errors='ignore')
if '1/1' in txt and 'healthy' in txt and 'meshctl list' in txt:
print(f'FILE: {p}')
lines = txt.splitlines()
for i,l in enumerate(lines,1):
if any(k in l for k in ['meshctl list','1/1','healthy','analyst','claude-provider-py','analyze','Non-tool query']):
start=max(1,i-3); end=min(len(lines),i+5)
print(f'--- lines {start}-{end} ---')
for j in range(start,end+1):
print(f'{j}: {lines[j-1]}')
print()
break
PYRepository: dhyansraj/mcp-mesh
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- target file: tc19 excerpt ---'
cat -n tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yaml | sed -n '1,220p'
echo '--- similar Java-consumer toolcall tests with registration polling ---'
for f in \
tests/integration/suites/uc10_toolcalls/tc23_consumer_java_provider_openai_py_tool_ts/test.yaml \
tests/integration/suites/uc10_toolcalls/tc27_consumer_java_provider_gemini_py_tool_java/test.yaml \
tests/integration/suites/uc10_toolcalls/tc02_consumer_py_provider_claude_py_tool_ts/test.yaml \
tests/integration/suites/uc10_toolcalls/tc04_consumer_py_provider_openai_py_tool_py/test.yaml
do
if [ -f "$f" ]; then
echo "FILE: $f"
cat -n "$f" | sed -n '80,180p'
echo
fi
done
echo '--- search for any explicit "healthy" or dependency-resolution assertion near analyze ---'
rg -n -C 3 -S 'analyze|healthy|1/1|register|meshctl list' \
tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yaml \
tests/integration/suites/uc10_toolcalls/tc23_consumer_java_provider_openai_py_tool_ts/test.yaml \
tests/integration/suites/uc10_toolcalls/tc27_consumer_java_provider_gemini_py_tool_java/test.yaml \
tests/integration/suites/uc10_toolcalls/tc02_consumer_py_provider_claude_py_tool_ts/test.yaml \
tests/integration/suites/uc10_toolcalls/tc04_consumer_py_provider_openai_py_tool_py/test.yamlRepository: dhyansraj/mcp-mesh
Length of output: 46340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- find meshctl list implementation / docs ---'
rg -n -S 'meshctl list|list agents|healthy|1/1|status' . \
-g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' -g '!**/.git/**' \
| sed -n '1,250p'
echo '--- locate likely CLI source files ---'
git ls-files | rg -n '(^|/)(meshctl|cli|cmd|commands?).*\.(go|ts|js|py)$|(^|/)main\.(go|ts|js|py)$'
echo '--- inspect files with meshctl list formatting, if found ---'
python3 - <<'PY'
import pathlib, re
cands=[]
for p in pathlib.Path('.').rglob('*'):
if p.is_file() and p.suffix in {'.go','.ts','.js','.py'}:
try:
txt=p.read_text(errors='ignore')
except Exception:
continue
if 'meshctl' in txt and ('list' in txt or 'healthy' in txt or '1/1' in txt):
cands.append(str(p))
for p in cands[:20]:
print(p)
PYRepository: dhyansraj/mcp-mesh
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/core/cli/list.go ---'
cat -n src/core/cli/list.go | sed -n '1,260p'
echo '--- src/core/cli/status.go ---'
cat -n src/core/cli/status.go | sed -n '1,260p'
echo '--- src/core/cli/list_test.go excerpts ---'
rg -n -C 3 -S 'healthy|unresolved|dependency|meshctl list|superseded|down' src/core/cli/list_test.go
echo '--- docs/meshctl-cli.md list semantics excerpt ---'
rg -n -C 3 -S 'meshctl list|healthy agents by default|unhealthy|dependencies|status' docs/meshctl-cli.mdRepository: dhyansraj/mcp-mesh
Length of output: 49715
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- tc19 artifacts / wiring references ---'
rg -n -S 'analyst-java|claude-provider-py|weather-tool-py|dependency_resolutions|Mesh provider not resolved|`@MeshLlm`|analyze' \
tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py \
examples/toolcalls \
tests/integration/suites/uc10_toolcalls/tc23_consumer_java_provider_openai_py_tool_ts \
tests/integration/suites/uc10_toolcalls/tc27_consumer_java_provider_gemini_py_tool_java \
src/core/cli \
docs \
| sed -n '1,260p'
echo '--- inspect any tc19 artifact/main files if present ---'
git ls-files 'tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/**' 'examples/toolcalls/**' | sed -n '1,200p'Repository: dhyansraj/mcp-mesh
Length of output: 34759
Wait for provider readiness, not just registration.
meshctl list | grep -q "analyst-java" only proves the consumer name appeared. A consumer can still be healthy while its dependency_resolutions are unresolved, so this can race the following analyze call and flake under cold JVM startup. Match the same readiness condition used in tc03 (e.g. 1/1/healthy) before proceeding.
🤖 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
`@tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yaml`
around lines 94 - 114, The wait step in the consumer registration shell command
only checks that analyst-java appears in meshctl list, which is insufficient for
readiness. Update the wait logic to use the same provider-ready signal as tc03,
and keep polling until analyst-java reports a healthy/ready state such as 1/1
rather than just registration. Use the existing wait-for-consumer step and
meshctl list/logs checks to locate the change, and ensure the analyze step only
runs after full readiness is confirmed.
… routines recv_event computes its deadline before queueing on the per-filter lock and races acquisition against the budget and this execution's cancel token, so queued calls honor their own timeout and cancelled executions never park on the lock. Concurrency tests now prove overlap via a gated mock backend. The nine registration waits share one global liveness-only routine parameterized by agent list; the single dep-aware wait condition is normalized to liveness-only per the settle-grace convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Review comments addressed in afc1a27:
Full suite on the refactored tree: 520 passed; the 2 failures are pre-existing uc04 structured-output tests in an untouched use case (re-run in progress to confirm). |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yaml (1)
18-18: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRaise the suite timeout
tests/integration/config.yamltreatsexecution.timeoutas the per-test wall-clock cap, so this suite can still hit the 600s limit:maven-installalone allows 600s,wait_for_agents_registeredruns up to ~240s, and the twoanalyzecalls add 240s more.tc03_java_llm_tracingalready uses 1200 for the same stacked-wait pattern.🤖 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 `@tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yaml` at line 18, Increase the suite timeout for this test so it can accommodate the combined wall-clock time of `maven-install`, `wait_for_agents_registered`, and both `analyze` calls under `execution.timeout`; update the `timeout` value in `tc19_consumer_java_provider_claude_py_tool_py/test.yaml` to match the higher limit already used by `tc03_java_llm_tracing` for the same stacked-wait pattern.tests/integration/suites/uc10_toolcalls/tc20_consumer_java_provider_claude_py_tool_ts/test.yaml (1)
19-19: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRaise the TC20 suite timeout
timeout: 600leaves no room for the rest of this flow: the Java consumer install is capped at 600s, then the suite still runs 15s/20s waits, the 260s registration poll, and two 120sanalyzecalls. TC20 also adds TypeScript agent setup, so this path needs a larger suite budget than tc19.🤖 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 `@tests/integration/suites/uc10_toolcalls/tc20_consumer_java_provider_claude_py_tool_ts/test.yaml` at line 19, The TC20 suite timeout is too low for the full flow in this test; update the suite budget in the test configuration for tc20 so it accounts for the Java consumer install cap, the registration polling, the two analyze calls, and the added TypeScript agent setup. Locate the timeout setting in the TC20 test YAML and increase it to a value larger than the current 600s so the suite can complete reliably.tests/integration/suites/uc10_toolcalls/tc22_consumer_java_provider_openai_py_tool_py/test.yaml (1)
18-18: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winIncrease the suite timeout above the 600s Maven install budget
timeout: 600at the suite level leaves no room for the later waits and 120s query calls ifmaven-installuses most of its own 600s budget. Raise the suite timeout so the whole scenario has headroom.🤖 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 `@tests/integration/suites/uc10_toolcalls/tc22_consumer_java_provider_openai_py_tool_py/test.yaml` at line 18, The suite-level timeout in the test configuration is too tight relative to the 600s Maven install budget. Update the timeout in the uc10_toolcalls tc22 consumer/provider YAML so the full scenario, including later waits and query calls, has extra headroom; use the suite timeout setting in the test definition as the target to increase.
🧹 Nitpick comments (1)
tests/integration/global/routines.yaml (1)
293-320: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDiagnostic dump on timeout doesn't scale with agent count vs. remaining time budget.
The loop consumes up to 240s (120 × 2s) of the step's 260s
timeout, leaving only ~20s for the failure path to runmeshctl listplus onemeshctl logs "$agent" | tail -30per expected agent (Line 315-318). Callers liketc07_day07_committeepass 12 agent names in a single call — if registration genuinely times out, the ~20s remainder is unlikely to be enough to dump logs for all 12 agents before the step's own 260s timeout kills the process, truncating exactly the diagnostics needed to debug the failure.Consider reserving diagnostic time proportional to the agent count (e.g., scale the loop bound/timeout by
TOTAL, or cap/parallelize the log dumps).♻️ Possible adjustment
- steps: - - handler: shell - workdir: /workspace - command: | - EXPECTED="${params.agents}" - TOTAL=$(echo "$EXPECTED" | wc -w | tr -d ' ') - echo "Waiting for $TOTAL agent(s) to register: $EXPECTED" - for i in $(seq 1 120); do + steps: + - handler: shell + workdir: /workspace + command: | + EXPECTED="${params.agents}" + TOTAL=$(echo "$EXPECTED" | wc -w | tr -d ' ') + echo "Waiting for $TOTAL agent(s) to register: $EXPECTED" + for i in $(seq 1 110); doTrim the poll loop bound (or bump the step
timeout) to leave headroom that grows withTOTALfor the diagnostics block.🤖 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 `@tests/integration/global/routines.yaml` around lines 293 - 320, The timeout handling in the shell step that waits for agents to register does not leave enough room for the diagnostic dump when many agents are expected. In the wait loop inside the wait_agents_registered handler, reduce the polling budget or otherwise make it scale with params.agents/TOTAL so the failure path still has time to run meshctl list and the per-agent meshctl logs checks. Keep the existing diagnostics, but ensure the remaining time budget grows with the number of expected agents.
🤖 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.
Outside diff comments:
In
`@tests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yaml`:
- Line 18: Increase the suite timeout for this test so it can accommodate the
combined wall-clock time of `maven-install`, `wait_for_agents_registered`, and
both `analyze` calls under `execution.timeout`; update the `timeout` value in
`tc19_consumer_java_provider_claude_py_tool_py/test.yaml` to match the higher
limit already used by `tc03_java_llm_tracing` for the same stacked-wait pattern.
In
`@tests/integration/suites/uc10_toolcalls/tc20_consumer_java_provider_claude_py_tool_ts/test.yaml`:
- Line 19: The TC20 suite timeout is too low for the full flow in this test;
update the suite budget in the test configuration for tc20 so it accounts for
the Java consumer install cap, the registration polling, the two analyze calls,
and the added TypeScript agent setup. Locate the timeout setting in the TC20
test YAML and increase it to a value larger than the current 600s so the suite
can complete reliably.
In
`@tests/integration/suites/uc10_toolcalls/tc22_consumer_java_provider_openai_py_tool_py/test.yaml`:
- Line 18: The suite-level timeout in the test configuration is too tight
relative to the 600s Maven install budget. Update the timeout in the
uc10_toolcalls tc22 consumer/provider YAML so the full scenario, including later
waits and query calls, has extra headroom; use the suite timeout setting in the
test definition as the target to increase.
---
Nitpick comments:
In `@tests/integration/global/routines.yaml`:
- Around line 293-320: The timeout handling in the shell step that waits for
agents to register does not leave enough room for the diagnostic dump when many
agents are expected. In the wait loop inside the wait_agents_registered handler,
reduce the polling budget or otherwise make it scale with params.agents/TOTAL so
the failure path still has time to run meshctl list and the per-agent meshctl
logs checks. Keep the existing diagnostics, but ensure the remaining time budget
grows with the number of expected agents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9729fd5c-2047-48b4-9cb1-d340e63bbb07
📒 Files selected for processing (11)
src/runtime/core/src/jobs.rstests/integration/global/routines.yamltests/integration/suites/uc06_observability/tc03_java_llm_tracing/test.yamltests/integration/suites/uc10_toolcalls/tc19_consumer_java_provider_claude_py_tool_py/test.yamltests/integration/suites/uc10_toolcalls/tc20_consumer_java_provider_claude_py_tool_ts/test.yamltests/integration/suites/uc10_toolcalls/tc22_consumer_java_provider_openai_py_tool_py/test.yamltests/integration/suites/uc10_toolcalls/tc23_consumer_java_provider_openai_py_tool_ts/test.yamltests/integration/suites/uc10_toolcalls/tc25_consumer_java_provider_gemini_py_tool_py/test.yamltests/integration/suites/uc10_toolcalls/tc26_consumer_java_provider_gemini_py_tool_ts/test.yamltests/integration/suites/uc10_toolcalls/tc46_consumer_java_provider_claude_ts_tool_py/test.yamltests/integration/suites/uc20_tutorial/tc07_day07_committee/test.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/runtime/core/src/jobs.rs
Merged
dhyansraj
added a commit
that referenced
this pull request
Jul 3, 2026
## v2.8.0 release ### Bump scope - `scripts/bump_version.py 2.7.0 2.8.0` — 387 files across runtimes, examples, tests, docs - Helm: all nine `Chart.yaml` (chart version + mesh-tracking appVersions + core subchart pins) — done by hand; script gap backlogged on #1260 — with `Chart.lock` regenerated via `helm dependency update` - `Cargo.lock` regenerated for the core crate version ### Release highlights (see RELEASE_NOTES.md) - **`required=true`** — availability-aware dependency graph: transitive capability availability with full constraint matching, route 503 perimeters in all three runtimes, cycle rejection, meshctl/UI observability, job claim-gating (#1249: #1255/#1257/#1258; #1256 lost-event fix) - **MeshJob execution integrity** — claim-epoch fencing, poll-liveness, per-filter event cursors, multi-replica integration suite (#1252: #1253/#1254) - **Data fidelity** — empty tool returns round-trip exactly across runtimes (#1251); LLM reply envelope always carries the answer as a string (#1248) - Breaking/behavior changes and mixed-version upgrade notes are called out in RELEASE_NOTES.md ### Validation - Full release gate (src-tests + integration at 2.8.0) running; result will be posted as a comment before merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.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.
Summary
Completes #1252 (Phases 3-5; Phases 1+2 landed in #1253).
JobControllertracks an independent cursor per canonical type-filter (sorted/deduped, trimmed, empty types dropped to match registry semantics; unfiltered is its own stream). Exactly-once within a stream, documented at-least-once across streams; interleaved gates with different filters can no longer permanently skip each other's events. Per-filter locks replace the global recv lock, so a 60s long-poll on one type doesn't block another — while same-filter calls stay serialized and monotonic. The epoch/supersession logic from feat(jobs): claim-epoch fencing and poll-liveness (Phases 1+2 of #1252) #1253 is untouched (verified byte-intact in review). The mock-vs-registry page contract is pinned by a dual-backend scenario table with exact path/query matching (the registry's own behavior is pinned Go-side), and the in-process mock now applies the registry's trim/drop-empties filter rule.uc33_meshjob_replicas): two 2-replica scenarios from the field report —tc01proves a quietly-gating handler pollingrecvEventthrough 2× its lease window keeps a single claim (poll-liveness), andtc02proves a genuinely-wedged handler is re-claimed with the stale owner fenced (epochs[1,2], exactly one surviving owner, job completes once). Assertions run against the registry job API with the epoch/attempt/transition evidence; timing margins sized for CI load.environment.md, anddocs/concepts/jobs.mdnow document the lease window derivation (max_duration, 300s default), what renews a lease (progress deltas AND executorrecvEventpolls), multi-replica claiming and epoch fencing, per-runtime supersession surfaces, per-filter cursor semantics with checkpoint guidance,claimEpochaccessors, and theMCP_MESH_JOB_STALE_TIMEOUTreap-ceiling vs lease-window distinction. Every claim was verified against the implementation before writing.Review Notes
Zero-context review applied (0 HIGH); all findings fixed: contract-test circularity (now matches
after/typesper scenario), tc02 lease-margin hardening (2s finish-gate rounds), retention raised to 60s with the 10s sweep tick kept, fail-fast re-claim wait, filter-key input hygiene + honest edge-case docs. Backlog note from testing:MCP_MESH_AGENT_NAMEseeds only the instance-id prefix — there is no env knob for a shared registered name across replicas.Closes #1252
Test plan
tsuite run --suite-path tests/src-tests— 12/12🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
recv_eventto treat each event type filter as its own independent stream with separate cursors, preventing cross-filter skipping.Bug Fixes
Documentation
Tests