fix(mcp): get_container_transport_events false-empty timeline (fall back to include path)

[dodeja]

Summary

get_container_transport_events reads the dedicated /containers/{id}/transport_events sub-resource. That sub-resource can return 404 even when the container exists and has events (it isn't enabled/populated for every container). The old tool swallowed that NotFound into a success-shaped empty timeline whose _metadata.error secretly carried "Not Found". The result: an LLM could not distinguish a genuinely empty timeline from a failed fetch, and a bad container id looked identical to a valid container with no events.

This PR makes the failure modes unambiguous:

• Fallback on NotFound — when the dedicated sub-resource 404s, the tool falls back to GET /containers/{id}?include=transport_events and maps the includedtransport_event records, so it returns the real events instead of a false-empty.
• Real error for a bad id — if the fallback container fetch also 404s (the container genuinely does not exist), the error propagates as a real tool error rather than a fake-empty success.
• Empty-but-valid is distinct — a container that exists with zero events returns an empty timeline tagged _metadata.container_found: true with noerror, clearly separating "nothing happened yet" from "bad id".
• No more secret error metadata — the timeline is never presented as success while carrying an internal _metadata.error: "Not Found".
• Dropped the duplicate mapped payload — the tool now returns a single flat timeline summary (total_events, event_categories, timeline, milestones, _metadata) instead of { mapped, summary }. Non-NotFound errors (e.g. upstream 5xx) still propagate unchanged — the fallback only triggers on NotFound.

_metadata.source is set to transport_events_subresource or container_include_fallback so callers/operators can see which path served the data.

Issues

Closes DEV-10660

Green gate

Whole-repo gate is green (baseline was SDK 51 pass / 2 skip, MCP 77 pass):

• npm run build --workspace @terminal49/sdk — clean
• npm run build --workspace @terminal49/mcp — clean
• npm run type-check --workspace @terminal49/sdk — clean
• npm run type-check --workspace @terminal49/mcp — clean
• npm test --workspace @terminal49/sdk -- --run — 51 pass / 2 skip
• npm run test --workspace @terminal49/mcp -- --run — 80 pass (added: fallback returns real events; genuinely-missing container raises a real error; empty-but-valid timeline; non-NotFound errors do not fall back; updated the happy-path to assert the flat, no-duplicate shape)

Notes

• TDD: the new/updated tests in packages/mcp/src/tools/contracts.test.ts were written first (red), then the tool was implemented to green.
• Scope: only packages/mcp/src/tools/get-container-transport-events.ts and the events region of contracts.test.ts were touched. The repo baseline is not oxfmt-clean (the pristine HEAD versions of these files already fail oxfmt --check), so a repo-wide oxfmt --write was intentionally avoided — it rewrites files owned by other PRs and even mangles an import. The diff matches the surrounding committed style and stays confined to the owned files.

This is an AI-drafted PR for human review.

🤖 Generated with Claude Code

Greptile Summary

This PR fixes a subtle but impactful bug where get_container_transport_events would silently convert a 404 from the dedicated sub-resource into a success-shaped empty timeline — making a missing sub-resource look identical to "no events" to an LLM consumer. The fix adds a two-step fallback: a NotFound from the primary endpoint triggers a GET /containers/{id}?include=transport_events fetch, a second 404 (genuine missing container) is re-thrown as a real error, and a new _metadata.source field tells callers which path served the data.

• Fallback on NotFound: The dedicated sub-resource 404 now triggers a container-level include fetch instead of returning a fake-empty success.
• Unambiguous error semantics: Non-404 errors and genuinely missing containers both throw rather than return a success-shaped payload with a secret _metadata.error field.
• Flat response shape: The { mapped, summary } wrapper is removed; the tool now returns a single flat object (total_events, timeline, milestones, _metadata).

Confidence Score: 4/5

The change is safe to merge; it replaces a proven-buggy silent-swallow pattern with a well-tested two-step fallback, and the new tests cover all four distinct outcomes.

The fallback logic and error propagation are clearly correct and backed by new tests. The two observations — no log entry emitted at the moment of fallback, and the absent container_found flag on the primary zero-event path — are non-blocking quality concerns, not defects in the happy path.

get-container-transport-events.ts around the silent 404 catch block and the FormatOptions / containerFound branching logic in formatTransportEventsResponse.

Important Files Changed

Filename	Overview
packages/mcp/src/tools/get-container-transport-events.ts	Introduces a two-step fallback: primary 404 triggers GET /containers/{id}?include=transport_events; non-404 errors and genuinely missing containers are now thrown rather than swallowed into a fake-empty response. Minor observability gap: the initial 404 is consumed silently with no log entry before the fallback runs.
packages/mcp/src/tools/contracts.test.ts	Test suite updated with four new/revised cases (fallback returns real events, missing container raises NotFoundError, empty-but-valid timeline, non-NotFound errors skip fallback) and the happy-path updated to assert the flat response shape. Good coverage of the new branching logic.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Tool as get_container_transport_events
participant Sub as GET /containers/{id}/transport_events
participant Inc as GET /containers/{id}?include=transport_events
Tool->>Sub: "events(id, {format:'raw'})"
alt 200 OK
Sub-->>Tool: raw event list
Tool-->>Tool: "formatTransportEventsResponse(raw, source='transport_events_subresource')"
else 404 NotFound
Sub-->>Tool: NotFoundError
Note over Tool: silent – no log entry for the 404
Tool->>Inc: "get(id, ['transport_events'], {format:'raw'})"
alt 200 OK (container exists)
Inc-->>Tool: container + included events
Tool-->>Tool: "extractIncludedTransportEvents → format(source='container_include_fallback', containerFound=true)"
else 404 NotFound (container missing)
Inc-->>Tool: NotFoundError
Tool-->>Tool: logError + rethrow
end
else Other error
Sub-->>Tool: Error
Tool-->>Tool: logError + rethrow
end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Tool as get_container_transport_events
participant Sub as GET /containers/{id}/transport_events
participant Inc as GET /containers/{id}?include=transport_events
Tool->>Sub: "events(id, {format:'raw'})"
alt 200 OK
Sub-->>Tool: raw event list
Tool-->>Tool: "formatTransportEventsResponse(raw, source='transport_events_subresource')"
else 404 NotFound
Sub-->>Tool: NotFoundError
Note over Tool: silent – no log entry for the 404
Tool->>Inc: "get(id, ['transport_events'], {format:'raw'})"
alt 200 OK (container exists)
Inc-->>Tool: container + included events
Tool-->>Tool: "extractIncludedTransportEvents → format(source='container_include_fallback', containerFound=true)"
else 404 NotFound (container missing)
Inc-->>Tool: NotFoundError
Tool-->>Tool: logError + rethrow
end
else Other error
Sub-->>Tool: Error
Tool-->>Tool: logError + rethrow
end

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---### Issue 1 of 2
packages/mcp/src/tools/get-container-transport-events.ts:56-68
**No log entry when the primary 404 triggers the fallback**
When the primary sub-resource returns a 404, the catch block silently discards the error and jumps straight to `fallbackToContainerInclude`. The structured log stream will show `tool.execute.start` followed by `tool.execute.complete` with `source: container_include_fallback`, but nothing indicates *why* the fallback ran. Any operator investigating unexpected fallback traffic has no log signal to correlate against. Consider emitting a structured `tool.execute.fallback` event (or similar) before entering the fallback path, so the trigger is observable without requiring a debugger.
### Issue 2 of 2
packages/mcp/src/tools/get-container-transport-events.ts:185-198
**`container_found` is absent when the primary subresource succeeds with zero events**`options.containerFound` is only set on the fallback path (`containerFound: true`), so `_metadata.container_found` is never present for the primary path. If the primary `/transport_events` endpoint returns an empty list (200 OK, zero events), the caller sees `total_events: 0` with `presentation_guidance` that says "This container exists but has no transport events yet" — but no `container_found` flag. Contrast this with the fallback empty-timeline path, where `container_found: true` is set. An LLM consuming the tool output now has two different shapes for the "container exists, zero events" case depending on which path served the data. This is minor now but could become confusing as callers add logic around `container_found`.

_{Reviews (1): Last reviewed commit: "fix(mcp): get_container_transport_events..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

[linear-code]

DEV-10660

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
api	Ready	Preview, Comment	Jul 1, 2026 8:41pm

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:d23a46b744

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dodeja]

Triaged the automated review comments and pushed 9645ad6. All three inline P2s were real and in-scope, so I addressed all of them:

1. Preserve locations on the include fallback (Codex) — Correct bug. The fallback uses containers.get(id, ['transport_events']), which side-loads the transport_event resources but not their related port/metro_area locations, and canonical transport_event payloads carry location_locode (no location_name). resolveLocation previously gated the embedded branch solely on location_name, so every fallback event lost its location. Now it surfaces a location whenever location_locode/port_locode is present. The fallback test fixture was unrealistic (location_name); updated to the real location_locode shape with a guard assertion that the code survives the fallback.

2. No log entry when the primary 404 triggers the fallback (Greptile) — Added a structured tool.execute.fallback log (with reason: transport_events_subresource_not_found) before the fallback runs, so operators can correlate fallback traffic. The 404 stays non-error (expected), it's just now observable.

3. container_found absent on primary zero-event success (Greptile) — Set containerFound: true on the primary success path too. A 200 from the sub-resource means the container exists, so an empty-but-valid primary timeline is now consistent with the fallback path and never reads as a missing container.

Nothing skipped — no nitpicks, duplicates, or out-of-scope suggestions in the set.

Gate: SDK build/type-check/test (51 pass / 2 skip) and MCP build/type-check/lint/test (80 pass) all green. No SDK public surface change, so no docs regen. Did not run oxfmt --write (not clean on main; would have mass-reformatted both files — kept the diff minimal and confirmed oxlint passes).

[dodeja]

Rebased onto latest origin/main (picked up #277–#284, no conflicts) and re-verified.

All three inline automated-review comments (Codex + Greptile) were already fixed in the prior commit 9645ad6 (now 4c37fda post-rebase):

1. Fallback location loss — resolveLocation() now resolves location_locode/port_locode when the include-fallback path doesn't side-load the location relationship.
2. Missing fallback log signal — added a structured tool.execute.fallback log (reason: transport_events_subresource_not_found) before the fallback runs.
3. Inconsistent container_found — the primary sub-resource success path now sets containerFound: true too, matching the fallback path.

No new issues found on re-review; nothing skipped.

Checks after rebase:

• npm run test --workspace @terminal49/mcp -- --run: 133 passed
• npm run build --workspace @terminal49/mcp: clean
• npm run lint --workspace @terminal49/mcp: clean
• npm run test --workspace @terminal49/sdk -- --run: 61 passed / 2 skipped
• npx tsc --noEmit -p tsconfig.json (api/): clean

Force-pushed the rebase to fix/mcp-transport-events-false-empty.