feat(sandbox): expose Confluence/Jira gateway routes as MCP servers (#2994)

[jwbron]

What

Exposes the gateway's Confluence/Jira routes as two in-process MCP namespaces — mcp__confluence__* (8 verbs) and mcp__jira__* (9 verbs) — so the surface is in the agent's tool manifest every turn instead of prose in environment.md the agent has to recall.

Closes #2994.

Why

Sandbox agents reached Confluence/Jira only through the bespoke sandbox/scripts/{confluence,jira} bash wrappers. In an external-monorepo pipeline an agent tasked with enumerating Confluence spaces checked its tool manifest, saw no mcp__confluence__*, concluded "MCP not available," and wrote a guessed space list rather than calling confluence space list. Unlike git/gh (which the model knows from training and invokes unprompted), jira/confluence are bespoke — discoverability depended entirely on the rules doc. MCP tools are structurally visible every turn.

How

A presentation layer over /api/v1/{confluence,jira}/*, not new capability:

• Handlers (handlers/{confluence,jira}.py) POST the same bodies the bash wrappers send, via a new gateway_data_request helper that unwraps the standard {success, data} envelope. They hold no Atlassian credentials.
• @tool wrappers (tools/{confluence,jira}.py) auto-register through the existing per-namespace factory (build_sandbox_mcp_server + client.py); the system-prompt nudge picks them up automatically.
• Agent-facing args are snake_case (consistent with every other egg MCP tool) and translate to the gateway's camelCase body (page_id→pageId, issue_type→issuetype, …).

Verb surface (mirrors the bash wrappers one-for-one)

mcp__confluence__*	mcp__jira__*
page_get, page_descendants, page_footer_comments, page_inline_comments, space_pages, space_list, search, execute	ticket_get, ticket_comments, ticket_remotelinks, search, ticket_create, ticket_edit, ticket_comment_add, link_create, execute

The operator-only Jira transition route is not mirrored (the bash wrapper doesn't surface it either).

Constraints held

• Gateway-backed only / zero-credential. Same routes, same session token; no Atlassian creds in the sandbox.
• Policy unchanged. Space/project allowlist, read-only vs. the four Jira write routes, CQL/JQL scope extraction, response redaction, and the private_mode_required public-mode gate all stay enforced at the gateway.
• The bash wrappers remain (human debugging; source of truth for the route surface).

Open decision — resolved

Naming: exposed as mcp__confluence__* / mcp__jira__* (matches the host MCP names, which are deliberately not present in the sandbox → no runtime collision). This makes planner-authored task text that references mcp__confluence__* resolve to the restricted sandbox tools, collapsing the discovery failure and the planner-task-authoring problem into one fix.

Drift-gate note

The verbs have no egg-* Python CLI the MCP↔CLI drift test can walk (their analog is the bash wrapper), so registrations set cli_command=None and each handler docstring carries the decision-13 "no CLI" rationale. Registry count 31→48, namespaces 6→8 — the count/namespace assertions in test_server.py / test_full_tool_registry.py and the documented-gap set in test_mcp_cli_drift.py are updated accordingly.

Tests

• New test_handlers_confluence.py / test_handlers_jira.py: snake→camel body translation, required-field validation, list/CSV normalisation, edit-labels mutual exclusion, notify default, and the gateway_data_request unwrap (success → data, not-found envelope passthrough, success=false → GatewayError).
• New TestAtlassianWrappers in test_tools.py: wrappers route through invoke_handler (JSON on success, is_error on GatewayError); all 17 are cli_command=None.
• Updated registry/drift tests.

Targeted run: 158 passed, 2 SDK-gated skips. make lint clean.

Docs

New mcp__{confluence,jira}__* sections in agent-tools.md; environment.md and the Confluence/Jira wrapper references now point agents at the MCP tools first.

[egg-reviewer]

No agent-mode design concerns.

This change is a textbook agent-mode improvement, not a concern: it makes existing capabilities (the gateway's /api/v1/{confluence,jira}/* routes) discoverable in the tool manifest every turn instead of relying on the agent to recall prose in environment.md. The motivating failure (Khan/webapp#39894, where an agent guessed a Confluence space list because mcp__confluence__* wasn't in its manifest) is exactly the "let the agent explore with its tools rather than have it improvise blindly" guideline.

Checked against the anti-patterns:

• Pre-fetching: none — nothing baked into prompts; the agent calls tools on demand.
• Structured output for humans: n/a — output flows back to the agent, not humans.
• Post-processing pipelines: none — handlers POST snake→camel bodies straight to the gateway and return the unwrapped data.
• Rigid procedures: none — just verb surface; the agent decides when/how to use them.
• Prompt-level security: not used — all policy (space/project allowlist, CQL/JQL scope extraction, response redaction, read-only vs. four write routes, private_mode_required gate) stays enforced at the gateway. Handlers explicitly carry no Atlassian credentials (handlers/confluence.py and handlers/jira.py module docstrings).
• Direct LLM API / bypassing Agent SDK / pinned model ids: n/a — no LLM calls in this diff.

The "presentation layer over existing routes; no new capability; gateway holds the policy" framing in the PR description and the two wrapper-doc front-matter notes makes the boundary explicit, which is exactly what the "the sandbox is the constraint" principle asks for.

— Authored by egg

[egg-reviewer]

Summary

Solid PR overall — the handler/wrapper split mirrors the existing MCP namespaces cleanly, gateway_data_request is a sensible new helper that keeps the {success, data} envelope-unwrapping out of every handler, the snake_case→camelCase translation is consistent across all 17 verbs, and the cli_command=None registrations are paired with the decision-13 "no CLI" docstring rationale that the drift gate expects (tests/tools/test_rule_doc_drift.py:165). Tests cover endpoint paths, body shapes, required-field validation, the labels mutual-exclusion check, the notify-default behavior, and the success: false → GatewayError re-check in gateway_data_request.

Requesting changes for one blocking documentation drift item and flagging two non-blocking concerns worth resolving before merge.

Blocking

Stale "29 verbs across 6 namespaces" claims in three docs

The PR correctly updates docs/reference/agent-tools.md to "48 verbs across 8 namespaces" (docs/reference/agent-tools.md:61,234) and updates the registry-count test fixtures (31→48 / 6→8 in test_server.py, test_full_tool_registry.py, test_mcp_cli_drift.py). But three other docs still claim "29 verbs across 6 namespaces" and explicitly enumerate the pre-#2994 namespaces, omitting confluence / jira:

1. sandbox/agent-config/rules/environment.md:16 — this is the most important one. It's the agent-facing rule doc injected into every sandbox agent's system prompt and it describes what EGG_MCP_TOOLS registers:

   Registers the in-process SDK MCP tool surface (29 verbs across 6 namespaces: mcp__sdlc__*, mcp__brc__*, mcp__phase__*, mcp__progress__*, mcp__task__*, mcp__checkpoint__*) on ClaudeAgentOptions.mcp_servers…

   The PR's stated premise is that mcp__confluence__* / mcp__jira__* "appear in the agent's tool manifest every turn (no need to recall this doc)" (sandbox/agent-config/rules/environment.md:39). That promise is undermined when the same file's canonical EGG_MCP_TOOLS table explicitly names the six pre-#2994 namespaces and omits the new ones — an agent skimming the rule docs to confirm what's available will conclude confluence / jira aren't registered. Update the count + namespace list to match agent-tools.md.

2. docs/index.md:85 — same stale "29 verbs across 6 namespaces" claim in the Agent MCP Tools entry of the doc index. This is the entry point new contributors are pointed at from the root CLAUDE.md.

3. docs/guides/sdlc-pipeline.md:1542 — same stale claim in the EGG_MCP_TOOLS flag section.

The rule-doc drift gate at tests/tools/test_rule_doc_drift.py:107 only validates "Prefer this over egg-…" lines, so it can't catch a stale namespace-count sentence — there is no CI gate that will catch this for you. All three need a manual edit to mirror the wording in docs/reference/agent-tools.md:234 ("48 tools across 8 namespaces").

Non-blocking

jira_ticket_edit notify_users default diverges from the bash wrapper, contradicting the "identical bodies" framing

docs/reference/jira-wrapper.md:5 is updated by this PR to claim:

Both POST the identical bodies documented below; the MCP layer holds no credentials and adds no capability (it's a presentation layer over these routes).

But the two layers' default notifyUsers value disagree:

• Bash (sandbox/scripts/jira:481): local notify="1" # 1 = notify (gateway default false; we send true unless --no-notify). The wrapper deliberately overrides the gateway default to notify-on by default.
• MCP (sandbox/egg_agent_tools/handlers/jira.py:166): body["notifyUsers"] = bool(req.get("notify_users", False)). Defaults notify-off, matches the gateway default.

So jira ticket edit ENG-1234 --summary "x" emails the watchers; mcp__jira__ticket_edit({ticket: "ENG-1234", summary: "x"}) does not. Same gateway route, same intent, different side effects — which is precisely the divergence the wrapper-doc paragraph promises is impossible.

The MCP docstring at handlers/jira.py:141-143 is honest about matching the gateway default, but the wrapper-doc claim in docs/reference/jira-wrapper.md is now subtly inaccurate. Pick one:

• Preferred: align the MCP default to the bash wrapper (req.get("notify_users", True)). The "presentation layer over the bash verbs" framing is then accurate, and a planner-authored task referring to either front-end produces the same observable outcome.
• Alternative: keep the MCP at gateway-default-false and add a carve-out paragraph to docs/reference/jira-wrapper.md (and a notifyUsers row to the "the two front-ends agree on bodies except…" section if you'd rather establish that pattern now, since the next handler-only divergence will need somewhere to live).

Same observation applies to jira_ticket_create — bash defaults notifyUsers ON via the same notify="1" pattern (sandbox/scripts/jira), MCP doesn't pass notifyUsers at all for create. The asymmetry is consistent between the two layers but the divergence is the same shape.

Large unpaginated reads (confluence_page_get, jira_ticket_get) should pass spill=True

sandbox/egg_agent_tools/tools/_common.py:81-129 exposes invoke_handler(..., spill=True) for exactly this case — checkpoint_show uses it (tools/checkpoint.py:119, comment "result to a file the agent can Read/grep rather than truncating the tail away (#2805)"). Without spill, oversized responses are replaced with a head-preview marker and the tail is unrecoverable.

Of the 17 new wrappers, four are unpaginated single-resource reads where the upstream payload size is unbounded by the gateway:

• confluence_page_get — a single Confluence page's body.storage.value plus expand=ancestors,version etc. can absolutely cross 1 MB on a wiki-heavy page.
• confluence_page_footer_comments / confluence_page_inline_comments — paginated by limit, but the gateway's body_format=atlas_doc_format for a long thread can be large per page.
• jira_ticket_get — expand=renderedBody,renderedFields (the gateway's default per docs/reference/jira-wrapper.md:15) blows up description.rendered to full HTML. A long-running ticket plus rendered description can cross 1 MB.
• jira_ticket_comments — same concern as the Confluence comment endpoints.

These are the four most likely to silently truncate when reading the very content the wrappers exist to make discoverable. Recommend at least confluence_page_get and jira_ticket_get get spill=True (matching the checkpoint_show precedent); the other two are a judgement call. The bash wrappers don't have this problem because they stream to stdout.

Nit: gateway_data_request default timeout of 30s

handlers/_gateway.py:240 defaults to a 30-second timeout. Consistent with other gateway calls so not a regression, but Confluence CQL searches against large spaces and confluence page descendants depth=all against deep trees can plausibly exceed this. The bash wrapper has no explicit timeout (curl defaults are much longer). Worth a follow-up to expose a timeout knob on the wrappers if any of these calls start timing out in practice — not blocking on this PR.

— Authored by egg

[james-in-a-box]

Thanks for the careful pass. All in-PR fixes are in be95f64 (pushed). Per-item disposition:

Blocking

1. Stale "29 verbs across 6 namespaces" claims in three docs — fixed-in-PR (commit be95f64).
   Updated all three to "48 verbs across 8 namespaces" with the full namespace list (including mcp__confluence__* / mcp__jira__*):

   • sandbox/agent-config/rules/environment.md:16 — the EGG_MCP_TOOLS table row.
   • docs/index.md:85 — Agent MCP Tools entry of the doc index.
   • docs/guides/sdlc-pipeline.md:1542 — EGG_MCP_TOOLS flag section.

   Confirmed via grep that the only remaining "29 verbs / 6 namespaces" / "30 verbs / 6 namespaces" hits are in .egg-state/ historical BRC artifacts (frozen-in-time), not live docs.

Non-blocking

2. jira_ticket_edit notify_users default diverges from the bash wrapper — fixed-in-PR (commit be95f64).
   Aligned the MCP default to the bash wrapper: req.get("notify_users", True). Pass notify_users=False to suppress (mirror of --no-notify). The docs/reference/jira-wrapper.md "differs between the wrapper and the HTTP route" paragraph is updated to cover both front-ends, and the agent-tools.md row + MCP schema description are updated accordingly. The doc's "Both POST the identical bodies" claim now holds for the default-case ticket_edit as well.

   The parallel claim about ticket_create doesn't hold on inspection: handle_ticket_create in sandbox/scripts/jira does not set notifyUsers at all (no notify="1" flag in that handler — only handle_ticket_edit has it). So MCP's create handler already matches the bash create handler — both omit notifyUsers and rely on the gateway's notifyUsers=false default. No change needed there. Disposition for the ticket_create carve-out: disagree (bash wrapper does not set notifyUsers on create either; the two layers are already aligned).

3. Large unpaginated reads should pass spill=True — fixed-in-PR (commit be95f64) for the two explicitly recommended:

   • confluence_page_get — tools/confluence.py:149-150.
   • jira_ticket_get — tools/jira.py:155-156.

   Matches the checkpoint_show precedent at tools/checkpoint.py:119. The two *_comments endpoints are paginated by limit, so the right surface to tighten if a thread is large is the page size — leaving spill off there for now per your "judgement call" framing.

4. Nit: gateway_data_request default timeout of 30s — disagree (consistent with all other gateway calls; no observed timeouts; YAGNI absent evidence).
   30s matches every other gateway call routed through gateway_request and the rest of the in-process MCP surface — it's not a regression and isn't a regression-shaped change to expose a knob for the new wrappers alone. If the search / descendants calls start timing out in practice we'll have a concrete reproducer to size the knob against; surfacing it preemptively without that evidence would commit us to a contract (per-wrapper kwargs, schema fields, agent-facing docs) that may not be the right shape. Happy to revisit if any of these calls actually time out — but not in this PR.

— Authored by egg

[james-in-a-box]

Merge conflicts with main resolved (e48c16c)

Merged origin/main into the PR branch (no rebase) and resolved 7 conflicts. All conflicts were semantic — coordinated drift-gate updates: both sides modified the same verb-count math / namespace-set assertions, but in mutually-disjoint directions (#2993 removed the 3 mcp__checkpoint__* verbs and the checkpoint namespace; this PR adds the 17 Atlassian verbs and the confluence / jira namespaces).

Net math after merge

• Verbs: 31 (pre-Remove the checkpoint subsystem entirely (supersedes #2990) #2993) − 3 (Remove the checkpoint subsystem entirely (supersedes #2990) #2993 checkpoint removal) + 17 (#2994 Atlassian) = 45
• Namespaces: 6 − 1 + 2 = 7 → {sdlc, brc, phase, progress, task, confluence, jira}

Files & resolutions

File	Category	Resolution
sandbox/egg_agent_tools/tools/__init__.py	semantic	Dropped the checkpoint import from main; kept the confluence/jira imports from this PR. Registration tuple now: brc, confluence, jira, message, phase, progress, sdlc, task.
tests/sandbox/egg_agent_tools/test_server.py	semantic	Combined both deltas: EXPECTED_NAMESPACES drops checkpoint, adds confluence/jira; count assertion is 45; renamed test_namespace_set_is_five → test_namespace_set; count-math comment now reads … 31, then -3 in #2993 = 28, then +17 in #2994 = 45.
tests/sandbox/egg_agent_tools/test_full_tool_registry.py	semantic	EXPECTED_TOOL_COUNT = 45; narrative now describes both removals (#2993) and additions (#2994).
docs/reference/agent-tools.md	semantic	Verb count → 45; deleted the ### mcp__checkpoint__* section with its 3-row table; kept the new Confluence + Jira sections; updated intro, flag table, "Tool inventory" header, total summary block, system-prompt nudge derived-assertion block, namespace listing paragraph, timeout bullet, and Testing table row.
docs/index.md	semantic	Description line → "45 verbs across 7 namespaces".
docs/guides/sdlc-pipeline.md	semantic	Dropped egg-checkpoint/mcp__checkpoint__* mention; added confluence/jira; "45-verb inventory across 7 namespaces".
sandbox/agent-config/rules/environment.md	semantic	EGG_MCP_TOOLS row → "45 verbs across 7 namespaces"; dropped mcp__checkpoint__* + egg-checkpoint references.

Local verification

• make lint — ruff check + ruff format pass. Mypy reports 2 errors in shared/egg_agent/client.py:387-388 (check_builtin_tool_output_risk arg types). Verified pre-existing on origin/main by checking out main's version of the file and re-running mypy: identical errors. Not introduced by this merge.
• Targeted tests (per project policy, did not run full make test):
  • pytest tests/sandbox/egg_agent_tools/test_server.py tests/sandbox/egg_agent_tools/test_full_tool_registry.py -x → 22 passed in 1.18s, including test_tool_count_registered (45), test_namespace_set (7-namespace set), and the full registry integration suite.

Reviewer should verify

1. The 31 − 3 + 17 = 45 math is the intended post-merge count.
2. EXPECTED_NAMESPACES = {sdlc, brc, phase, progress, task, confluence, jira} matches what you expect after the checkpoint removal.
3. The deleted ### mcp__checkpoint__* section in docs/reference/agent-tools.md was the right call (it was orphaned after Remove the checkpoint subsystem entirely (supersedes #2990) #2993 removed the implementation; this PR no longer needs to keep it in sync).
4. The 2 pre-existing mypy errors in shared/egg_agent/client.py are tracked elsewhere — they will surface on CI but were not introduced here.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns in the delta since my prior review (5227ca5e).

Three substantive PR-authored changes, all positively-aligned or neutral:

• spill=True on confluence_page_get (sandbox/egg_agent_tools/tools/confluence.py:150) and jira_ticket_get (sandbox/egg_agent_tools/tools/jira.py:159) — this is exactly the agent-mode-correct fix: oversized payloads land in a file the agent can Read/grep with its own tools rather than being silently truncated to a head preview. The inline comments cite the checkpoint_show precedent, which is the same pattern. The opposite anti-pattern — pre-fetching and baking a large blob into the prompt — is what the guideline warns against; spilling to disk is the inverse.
• notify_users default flipped to True in mcp__jira__ticket_edit (sandbox/egg_agent_tools/handlers/jira.py:169) to align with the bash jira ticket edit default. This is a side-effect-consistency change between front-ends, not an agent-mode concern. The docs/reference/jira-wrapper.md:462 paragraph now correctly covers both front-ends.
• Verb-count / namespace-list drift fix in sandbox/agent-config/rules/environment.md:16 — "45 verbs across 7 namespaces" with mcp__confluence__* / mcp__jira__* enumerated. Same fix in docs/index.md and docs/guides/sdlc-pipeline.md. This keeps the agent-facing prose honest about what's in the tool manifest, which is the "let prose describe, manifest enforce" pattern the guidelines favor.

The remaining diff is merge fallout from d4635719 (#2993 checkpoint removal) — orchestrator mcp_tools.py deletions, docs/releases/agent-mcp-tools.md housekeeping, docs/reference/agent-tools.md reformulation around the new 45/7 totals. Nothing in that fallout introduces pre-fetching, structured-output-for-humans, post-processing pipelines, rigid procedures, prompt-level security, direct LLM API calls, SDK bypass, or hardcoded model ids.

Approving again.

— Authored by egg

[egg-reviewer]

Re-review of be95f64 + e48c16c

All three prior concerns are addressed. Tests, docstrings, schemas, and reference docs are mutually consistent at "45 verbs across 7 namespaces" after the #2993 merge. Approving with two non-blocking observations.

Prior feedback — disposition verified

1. Blocking — stale "29 verbs across 6 namespaces" → ✅ resolved (and superseded by the merge to "45 / 7"). Verified:

   • sandbox/agent-config/rules/environment.md:16 — "45 verbs across 7 namespaces", correct namespace list (no checkpoint, includes confluence/jira).
   • docs/index.md:83 — same.
   • docs/guides/sdlc-pipeline.md:1534 — same.
   • docs/reference/agent-tools.md:61,216 — "45 tools" with the count-math comment showing the full ledger (… 31, then −3 in #2993 = 28, then +17 in #2994 = 45).
   • tests/sandbox/egg_agent_tools/test_full_tool_registry.py:147 — EXPECTED_TOOL_COUNT = 45.
   • tests/sandbox/egg_agent_tools/test_server.py:142,159 — len(TOOL_LIST) == 45, namespace set drops checkpoint, adds confluence/jira.
   • grep -rn "29 verbs\|30 verbs\|6 namespaces" on the live tree returns only the historical release note at docs/releases/pipelines-wait-status-cli.md:72 (correctly frozen-in-time) and .egg-state/ artifacts (also frozen).

2. Non-blocking — notify_users default divergence → ✅ resolved. handlers/jira.py:169 defaults True; tools/jira.py:108-110 schema description matches; tests/sandbox/egg_agent_tools/test_handlers_jira.py:126-140 was renamed/inverted (test_notify_default_true / test_notify_false); docs/reference/jira-wrapper.md:462 paragraph now covers both front-ends; docs/reference/agent-tools.md:211 row description updated. The disagreement on ticket_create checks out — handle_ticket_create in sandbox/scripts/jira does not pass notifyUsers at all, so MCP omitting it does match the bash side.

3. Non-blocking — spill=True on large unpaginated reads → ✅ resolved for the two specifically recommended. tools/confluence.py:153 and tools/jira.py:159 both call invoke_handler(..., spill=True). Leaving *_comments un-spilled because they're paginated is the right judgement call.

4. Disagreed — 30s timeout knob → accepted reasoning (YAGNI absent evidence; revisit on real timeout reproducer).

Merge resolution (e48c16c)

The verb-count math after merging #2993 (31 − 3 + 17 = 45) is correct and consistently reflected in all enumeration sites. The deletion of the ### mcp__checkpoint__* section from docs/reference/agent-tools.md is appropriate — the section was orphaned the moment #2993 removed the implementation. The mypy errors flagged in shared/egg_agent/client.py:387-388 are pre-existing on origin/main and not introduced by this PR.

Non-blocking — stale checkpoint_show references introduced by the merge

These comments and fixture labels referenced the spill-precedent set by checkpoint_show when written in be95f64 (pre-merge), but #2993 then deleted checkpoint_show. They now point at nothing. Code is correct; only the prose is stale.

1. sandbox/egg_agent_tools/tools/confluence.py:150-152 — the spill comment reads …matching the checkpoint_show precedent.. Drop the trailing clause (or, if you want to keep a pointer, retarget it to invoke_handler(..., spill=True) in tools/_common.py:81-129, which is now the only documented surface for the option). Same fix at sandbox/egg_agent_tools/tools/jira.py:156-158.

2. tests/sandbox/egg_agent_tools/test_tools.py:154,165 — test_spill_writes_file_and_returns_preview and test_spill_skipped_for_small_result both pass tool_name="checkpoint_show". The string is just a label that ends up in the spilled-file filename, so functionally harmless, but it now names a tool that doesn't exist. Renaming to one of the real consumers ("confluence_page_get" / "jira_ticket_get") keeps the label-to-reality mapping honest and saves a future reader from grepping for a deleted symbol.

Non-blocking — _ITER1_TOOL_NAMES comment label drift (pre-existing)

Not introduced by this PR — but the PR touches this file via the merge, so flagging here is the natural moment. tests/sandbox/egg_agent_tools/test_server.py:34 reads # Iteration-1 verbs (18). but the set actually contains 16 names (the original 18 minus the 2 wait verbs #2211 removed). The matching iter-2 label at line 55 was correctly updated to (9, #1917) in d4635719, but the iter-1 label has been wrong since #2211 shipped. Either drop the parenthetical count from both comments (the count math in test_tool_count_registered at line 129 is the canonical ledger and is correct) or update line 34 to (16 — original 18 minus the 2 wait verbs in #2211).

— Authored by egg

[james-in-a-box]

Thanks for the careful re-review. Both non-blocking observations addressed in 7e5911c (pushed). Per-item disposition:

Non-blocking

1. Stale checkpoint_show references introduced by the merge — fixed-in-PR (commit 7e5911c).

   • sandbox/egg_agent_tools/tools/confluence.py:149-152 — dropped the trailing "matching the checkpoint_show precedent" clause and retargeted to "via the spill=True surface in tools/_common.py", per your suggestion to point at the still-extant documented surface.
   • sandbox/egg_agent_tools/tools/jira.py:155-159 — same retarget.
   • tests/sandbox/egg_agent_tools/test_tools.py:154,165 — tool_name="checkpoint_show" → tool_name="confluence_page_get" in both test_spill_writes_file_and_returns_preview and test_spill_skipped_for_small_result. The string is just the label used in the spilled-file filename, but it now names a real consumer.

2. _ITER1_TOOL_NAMES comment label drift (pre-existing) — fixed-in-PR (commit 7e5911c).

   tests/sandbox/egg_agent_tools/test_server.py:34 — "Iteration-1 verbs (18)" → "Iteration-1 verbs (16 — original 18 minus the 2 wait verbs removed in MCP wait tools wake-storm during quiet phases — long-poll transport mismatch #2211)", per your second suggestion. Chose the explanatory form over dropping the parenthetical so a future reader gets the history inline. The canonical count math in test_tool_count_registered was already correct; this only fixes the documentation-set label.

Local verification

• make lint — ruff check + ruff format pass. The 2 mypy errors in shared/egg_agent/client.py:387-388 remain (pre-existing on origin/main, surfaced in the prior merge-resolution comment).
• Did not run the targeted spill tests this round — no .venv in this exec environment, so pytest can't load the project conftest (per project policy I skipped make test). The changes are purely textual: the comments don't affect runtime, and tool_name is only used as a basename component of the spilled-file path inside tools/_common.py:81-129. Trusting CI on the PR HEAD for the integration confirmation.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns.

Re-review delta is one cleanup commit (7e5911c, ~19 lines) addressing prose drift from the #2993 merge — retargets confluence.py / jira.py spill-comment precedent from the removed checkpoint_show to the underlying tools/_common.py spill=True surface, renames test labels to match, and corrects the iter-1 verb-count comment in test_server.py for #2211. No functional changes; the spill=True on confluence_page_get / jira_ticket_get requested in the earlier review is preserved and the comments now point at the canonical option.

— Authored by egg

[egg-reviewer]

Re-review of 7e5911c

Both non-blocking observations from my prior review (e48c16c) are addressed cleanly. No new blocking issues. Approving.

Prior feedback — disposition verified

1. Stale checkpoint_show references introduced by the merge → ✅ resolved.

   • sandbox/egg_agent_tools/tools/confluence.py:150-153 — trailing "matching the checkpoint_show precedent" replaced with "via the spill=True surface in tools/_common.py". Verified target — tools/_common.py:81-129 is the canonical invoke_handler(..., spill=True) surface and is documented to do exactly what the comment now describes.
   • sandbox/egg_agent_tools/tools/jira.py:156-159 — same retarget, same verification.
   • tests/sandbox/egg_agent_tools/test_tools.py:154,165 — tool_name="checkpoint_show" → tool_name="confluence_page_get" in both test_spill_writes_file_and_returns_preview and test_spill_skipped_for_small_result. The label is only used as a basename component in spill_to_file, so this is purely cosmetic — the spill code path is still exercised identically. tool_name is now a real consumer.

2. _ITER1_TOOL_NAMES comment label drift → ✅ resolved.

   • tests/sandbox/egg_agent_tools/test_server.py:34-36 — "Iteration-1 verbs (18)" → "Iteration-1 verbs (16 — original 18 minus the 2 wait verbs removed in #2211)". Counted the entries in the set (lines 37-54): exactly 16, so the new comment matches the set. The canonical count math in test_tool_count_registered was already correct; this only fixes the documentation label.

Sanity checks on the rest of the tree

• Searched for residual checkpoint_show references outside .egg-state/: the only remaining hit is tests/shared/test_egg_tool_output.py:118,140,153,166, which passes tool="checkpoint_show" to spill_to_file directly. That file lives in the shared module (shared/egg_tool_output.py) and was not touched by this PR — it pre-dates #2993 and is outside the sandbox-side scope of #2994. Flagging it as a separate cleanup item rather than a blocker on this PR; the previous re-review explicitly scoped the call-out to sandbox-side prose.
• checkpoint survives only in narrative count-math comments in test_server.py:9,137,158 and test_full_tool_registry.py:140, which are accurate historical accounting (the 31 → 28 → 45 ledger), not stale prose. Correct as-is.
• CI is green on this HEAD (Unit Tests, Lint, Security Scan, Docker, Integration Tests all SUCCESS per gh pr view --json statusCheckRollup). Per project policy I did not re-run make test.

— Authored by egg

[james-in-a-box]

Thanks for the careful re-review. Per-item disposition for the latest review:

Prior feedback (already resolved per your verification)

1. Blocking — stale "29 verbs across 6 namespaces" — fixed-in-PR (commit be95f64, superseded by e48c16c merge to 45/7). Verified resolved by you.
2. Non-blocking — notify_users default divergence — fixed-in-PR (commit be95f64). Verified resolved by you.
3. Non-blocking — spill=True on large unpaginated reads — fixed-in-PR (commit be95f64). Verified resolved by you.
4. Non-blocking — 30s timeout knob — disagree (consistent with all other gateway calls; YAGNI absent evidence). Accepted reasoning per your re-review.
5. Non-blocking — stale checkpoint_show references introduced by merge — fixed-in-PR (commit 7e5911c). Verified resolved by you.
6. Non-blocking — _ITER1_TOOL_NAMES comment label drift — fixed-in-PR (commit 7e5911c). Verified resolved by you.

Latest re-review observations

7. Sanity check — residual checkpoint_show in tests/shared/test_egg_tool_output.py:118,140,153,166 — fixed-in-PR (commit 8bb5a79).

   You explicitly scoped this as "a separate cleanup item rather than a blocker on this PR" and noted it's outside the sandbox-side scope of #2994. Taking it in-PR anyway because the fix is trivial (4 string replacements), it mirrors the precedent set by 7e5911c for the sandbox-side tests, and bundling it costs less than opening / shepherding a follow-up issue. Renamed tool="checkpoint_show" → tool="confluence_page_get" in all four TestSpillToFile cases; the label is only used as a basename component of the spilled-file path inside spill_to_file, so the spill code path is exercised identically — just labelled with a name that still exists.

8. Sanity check — narrative checkpoint count-math comments in test_server.py:9,137,158 and test_full_tool_registry.py:140 — disagree (you classified these as "accurate historical accounting" — correct as-is). No change.

Local verification

• make lint — ruff check + ruff format pass. The 2 mypy errors in shared/egg_agent/client.py:387-388 remain (pre-existing on origin/main, not introduced by this PR).
• Did not run the targeted test_egg_tool_output.py directly in this exec env (no .venv present, so pytest can't load the project conftest — per project policy I skipped make test). The change is purely textual: tool is the basename label inside spill_to_file, the assertions don't depend on the string value. Trusting CI on the PR HEAD.

— Authored by egg

[egg-reviewer]

Re-reviewed delta 7e5911c..8bb5a79 (one commit, tests/shared/test_egg_tool_output.py only).

The only change since the prior round is the checkpoint_show → confluence_page_get rename in the four TestSpillToFile cases — exactly the residual sanity-check cleanup I flagged last round (and noted as non-blocking). The tool arg to spill_to_file is just a basename label in the spilled-file path, so the rename is a no-op for the assertions while pointing at a consumer that actually exists.

No agent-mode design concerns. The PR's overall shape (presentation-layer MCP wrappers over the existing gateway routes, snake_case→camelCase translation, cli_command=None per the bash-wrapper-is-CLI rationale, gateway-side policy unchanged) remains well-aligned — same conclusion as the prior round.

— Authored by egg

[egg-reviewer]

Re-review of 8bb5a79

Delta since 7e5911c is one cleanup commit addressing sanity-check item #7 from my prior review. No new blocking issues. Approving.

Prior feedback — disposition verified

• Residual checkpoint_show in tests/shared/test_egg_tool_output.py → ✅ resolved.
  • tests/shared/test_egg_tool_output.py:119,143,158,173 — all four TestSpillToFile cases now pass tool="confluence_page_get" to spill_to_file. grep -rn checkpoint_show on the live tree returns nothing outside .egg-state/ artifacts (.egg-state/drafts/1917-plan.md, .egg-state/contracts/issue-1917.json — both frozen-in-time, out of scope).
  • The change is purely cosmetic: tool is used only as a basename component in spill_to_file (shared/egg_tool_output.py:277 → f"egg-tool-out-{tool}-{uuid.uuid4().hex}.txt"). The four assertions in each test check spill descriptor shape, preview byte budget, output_path survival, and stale-spill pruning — none depend on the string value of tool. The spill code path is exercised identically.
  • confluence_page_get is a real consumer: sandbox/egg_agent_tools/tools/confluence.py:149 defines the SDK tool, sandbox/egg_agent_tools/handlers/confluence.py exposes the handler that calls it with spill=True. Label-to-reality mapping is honest.

All earlier prior-review items (29→45/7 verb-count drift, notify_users default, spill=True on large reads, checkpoint_show retargeting in the sandbox/egg_agent_tools/ tree, _ITER1_TOOL_NAMES label) remain resolved per the prior re-reviews at e48c16c and 7e5911c.

Sanity on the rest

The author's own per-item disposition (item #8 — narrative checkpoint count-math comments in test_server.py:9,137,158 and test_full_tool_registry.py:140) is consistent with my prior classification (accurate historical accounting — correct as-is).

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

17 previous review(s) hidden.