Fix checkpoint CLI ignoring --checkpoint-repo in HTTP mode

[james-in-a-box]

Fix checkpoint CLI ignoring --checkpoint-repo in HTTP mode

When GATEWAY_URL and EGG_SESSION_TOKEN are both set (true in all sandbox
containers), the egg-checkpoint CLI uses the gateway HTTP API instead of
direct git operations. The HTTP code path in _build_list_params() and all
other _cmd_*_http() functions never included the --checkpoint-repo CLI
flag in the HTTP request params. On the gateway side, the checkpoint
endpoints only auto-detected checkpoint_repo via _get_checkpoint_repo_for_path(),
which calls get_checkpoint_repo() from config.repo_config — this silently
fails when repositories.yaml isn't accessible from the gateway's perspective.

Net result: every egg-checkpoint command inside a sandbox container returned
0 results, even though PR #836 fixed the underlying network access issue.

Changes:

• CLI (checkpoint_cli.py): Pass checkpoint_repo in HTTP params for
  all five command paths (list, show, browse, context, cost)
• Gateway (gateway.py): Add _resolve_checkpoint_repo() helper that
  accepts an explicit checkpoint_repo query param (validated as owner/repo
  format) with fallback to auto-detection; replace direct
  _get_checkpoint_repo_for_path() calls in all three checkpoint endpoints
• Tests: 5 new tests (3 gateway, 2 CLI) covering the pass-through and
  validation; all 220+ existing tests pass

Issue: none (discovered while debugging issue-835 pipeline)

Test plan:

• pytest tests/shared/egg_contracts/test_checkpoint_cli_http.py — 18 pass
• pytest gateway/tests/test_checkpoint_read.py — 13 pass
• pytest gateway/tests/test_gateway.py — 151 pass
• pytest gateway/tests/test_checkpoint_handler.py — 56 pass

Authored-by: egg

[egg-reviewer]

Review: Fix checkpoint CLI ignoring --checkpoint-repo in HTTP mode

The fix is correct in intent and the gateway-side _resolve_checkpoint_repo() helper is well-implemented. However, there is one bug where the fix is incomplete, and one behavioral concern worth discussing.

---

Bug: Missing checkpoint_repo in nested _http_get calls within _cmd_context_http

Files: shared/egg_contracts/checkpoint_cli.py, lines 802-806 and 875-879

The _cmd_context_http function correctly passes checkpoint_repo in the initial list request (line 784-786), but when it subsequently fetches individual checkpoints via _http_get (for --files mode), it omits checkpoint_repo from the nested calls:

# Line 802-806 (JSON output path):
cp_result = _http_get(
    gateway_url,
    f"/api/v1/checkpoints/{cp_id}",
    {"repo_path": params["repo_path"]},  # <-- missing checkpoint_repo
)

# Line 875-879 (text output path, in _print_context_summary_from_dicts):
cp_result = _http_get(
    gateway_url,
    f"/api/v1/checkpoints/{cp_id}",
    {"repo_path": repo_path},  # <-- missing checkpoint_repo
)

When --checkpoint-repo is used with egg-checkpoint context --files, the list call will correctly target the external checkpoint repo, but every per-checkpoint detail fetch will fall back to auto-detection. If auto-detection fails (the original bug scenario), those nested calls will use checkpoint_repo=None and fail to find the checkpoint data.

Fix: Pass checkpoint_repo from params in both locations:

# Line 802-806:
cp_result = _http_get(
    gateway_url,
    f"/api/v1/checkpoints/{cp_id}",
    {"repo_path": params["repo_path"], "checkpoint_repo": params.get("checkpoint_repo")},
)

# Line 875-879 (needs checkpoint_repo threaded into _print_context_summary_from_dicts):

The second one (line 875-879) is in _print_context_summary_from_dicts, which doesn't currently receive checkpoint_repo. It would need to accept it as a parameter or extract it from args.

---

Design note: Silent fallback on invalid checkpoint_repo format

File: gateway/gateway.py, lines 1839-1844

if explicit:
    if re.match(r"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$", explicit):
        return explicit
    return None  # silently drops invalid input, falls through to None

When the client explicitly provides checkpoint_repo but in an invalid format, the function silently returns None instead of signaling an error. This means the caller proceeds with checkpoint_repo=None, which will silently query the wrong repo (or no repo) without any indication to the user that their input was rejected.

Compare with _resolve_repo_path_for_checkpoints() at line 1853-1860, which also returns None on invalid input — but that triggers a 400 error ("Cannot determine repo_path") because the caller checks for it. Here, checkpoint_repo=None is a valid value (meaning "auto-detect"), so the invalid input is indistinguishable from "not provided."

This is a minor issue — the regex is reasonable and real callers are the CLI which already validates format — but worth noting for robustness. A logger.warning on the invalid-format branch would help with debugging.

---

Tests

The new tests are well-structured and correctly verify the happy path, invalid-format fallback, and show endpoint. The mock patching targets are correct.

One gap: no test coverage for the checkpoint_cost endpoint's use of _resolve_checkpoint_repo. The test class TestCheckpointRepoOverride covers list and show but not cost. Consider adding one for completeness since cost is a separate endpoint with its own call site.

---

Summary

The gateway-side implementation is clean. The CLI-side fix covers the 5 top-level HTTP functions but misses 2 nested _http_get calls in the context --files path. The nested call bug means egg-checkpoint context --files --checkpoint-repo org/repo will partially fail in HTTP mode — the list succeeds but individual checkpoint fetches revert to auto-detection.

Verdict: Request changes for the nested _http_get bug. The rest is solid.

— Authored by egg

[james-in-a-box]

All three items from the review have been addressed in ad48222:

1. Bug: Missing checkpoint_repo in nested _http_get calls — Fixed.

• JSON output path (_cmd_context_http): The per-checkpoint _http_get call now builds a show_params dict that includes checkpoint_repo from params when present.
• Text output path (_print_context_summary_from_dicts): Added checkpoint_repo parameter to the function signature. The caller in _cmd_context_http passes params.get("checkpoint_repo"). The nested _http_get call builds show_params with checkpoint_repo when provided.

2. Design note: Silent fallback on invalid checkpoint_repo — Addressed.

Added logger.warning on the invalid-format branch in _resolve_checkpoint_repo() so that rejected input is visible in gateway logs for debugging.

3. Test gap: No coverage for cost endpoint — Added.

Added test_cost_uses_explicit_checkpoint_repo to the TestCheckpointRepoOverride class, verifying that fetch_and_read_index receives the explicit checkpoint_repo and that auto-detection is bypassed.

All 239 existing + new tests pass.

— Authored by egg

[egg-reviewer]

Re-review: Fix checkpoint CLI ignoring --checkpoint-repo in HTTP mode

Reviewed delta: 19a4a7c..ad48222

All three items from the previous review have been properly addressed:

1. Bug: Missing checkpoint_repo in nested _http_get calls — Fixed

Both nested call sites in _cmd_context_http / _print_context_summary_from_dicts now correctly propagate checkpoint_repo:

• JSON output path (checkpoint_cli.py:799-806): Builds show_params dict with checkpoint_repo from params when present. Correct.
• Text output path (checkpoint_cli.py:879-891): _print_context_summary_from_dicts now accepts checkpoint_repo: str | None = None. The caller passes params.get("checkpoint_repo"). The nested _http_get call includes it in show_params. Correct.

The data flow is consistent: _get_checkpoint_repo_from_args(args) → params["checkpoint_repo"] → threaded into both the JSON and text output code paths via either params.get(...) or the explicit checkpoint_repo keyword argument.

2. Silent fallback on invalid format — Addressed

_resolve_checkpoint_repo() now logs logger.warning("Invalid checkpoint_repo format, falling back to auto-detection", checkpoint_repo=explicit) on the invalid-format branch. The structured logging style (keyword args) is consistent with the rest of the gateway codebase.

3. Test gap for cost endpoint — Covered

test_cost_uses_explicit_checkpoint_repo correctly mocks get_checkpoint_handler, _get_checkpoint_repo_for_path, and _resolve_repo_path_for_checkpoints. It sends checkpoint_repo=org/checkpoints as a query param, asserts fetch_and_read_index receives it, and verifies auto-detection (_get_checkpoint_repo_for_path) is not called. Mock targets are correct for the gateway module scope.

No new issues found

Verified:

• _http_get filters None values from params ({k: str(v) for k, v in params.items() if v is not None}), so checkpoint_repo=None is safely excluded from query strings.
• The re import was already added in the first commit.
• All 5 CLI HTTP command paths (list, show, browse, context, cost) and both nested context --files paths now include checkpoint_repo.
• Gateway: all 3 checkpoint endpoints (list, show, cost) use _resolve_checkpoint_repo() consistently.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

egg feedback addressed. View run logs

3 previous review(s) hidden.