Skip to content

fix(mcp): authoritative status, demurrage staleness guard, terminal-local times; drop _mapped bloat - #277

Merged
dodeja merged 3 commits into
mainfrom
fix/mcp-get-container-correctness
Jun 26, 2026
Merged

fix(mcp): authoritative status, demurrage staleness guard, terminal-local times; drop _mapped bloat#277
dodeja merged 3 commits into
mainfrom
fix/mcp-get-container-correctness

Conversation

@dodeja

@dodejadodeja commented Jun 24, 2026

Copy link
Copy Markdown
Member

Summary

Production-readiness fixes for the get_container / get_shipment_details tools and the t49:container/{id} resource. Extracts three pure, unit-tested modules under packages/mcp/src/lib/ (the deep-module seam) and wires them in:

  • ContainerStatusResolver (container-status.ts) — surfaces the API current_status as the authoritative headline status in both get_container and resources/container.ts, ending the three-way status-vocabulary divergence (API status vs. derived lifecycle vs. search status). The heuristic lifecycle is retained only as clearly-labeled, non-authoritative steering metadata and never reports delivered unless delivered_at is set.
  • DemurrageUrgencyEvaluator (demurrage.ts) — removes the fabricated ~$75-150/day demurrage accruing / URGENT message. Surfaces the real fees_at_pod_terminal (amount + currency code) and suppresses urgency when terminal_checked_at is stale (>7d) / never set, or when tracking is stopped/closed.
  • TemporalFormatter (temporal.ts) — renders terminal timestamps in pod_timezone and computes day-deltas in terminal-local time, fixing the get_shipment_details "ETA in N days" UTC off-by-one.

Additional correctness fixes:

  • get_containerinclude now augments the default [shipment, pod_terminal] instead of replacing it (previously include=[transport_events] silently dropped shipment + pod_terminal).
  • Surfaces pod_timezone + per-channel LFDs (terminal / rail / line from import_deadlines) in get_container.
  • equipment.length numeric-enum guard (no "" sentinel); drops the phantom updated_at the API never returns on containers.
  • Drops the duplicate _mapped payload from get_container and get_shipment_details (no more format:'both' with no mapper); returns the curated summary only.
  • Removes the dead getContainerTool / getShipmentDetailsTool objects — the Zod input schema in server.ts is the single source of truth.

Issues

Closes DEV-10659
Closes DEV-10661
Closes DEV-10664
Closes DEV-10665

(DEV-10665 partial: include-merge, equipment shape, drop phantom updated_at, dead *Tool removal — scoped to these tool/resource files.)

Green gate

CheckResult
build @terminal49/sdkPASS
build @terminal49/mcpPASS
type-check @terminal49/sdkPASS
type-check @terminal49/mcpPASS
test @terminal49/sdk --run51 pass / 2 skip
test @terminal49/mcp --run106 pass (was 77 baseline; +29 new)
oxlint srcclean

TDD: the three pure modules have failing-first unit tests (*.test.ts alongside each module) plus expanded get_container / get_shipment_details contract assertions.

Note

AI-drafted PR for human review. Touches only the get_container / get_shipment_details / container-resource files plus the new lib/ modules and their tests; no list_* / search / server-wide code was modified.

🤖 Generated with Claude Code

Greptile Summary

This PR extracts three pure, tested modules (container-status.ts, demurrage.ts, temporal.ts) and wires them into get_container, get_shipment_details, and the t49:container/{id} resource to fix fabricated demurrage urgency messages, three-way status vocabulary divergence, UTC off-by-one in day-delta presentation, and the include parameter silently dropping the default sideloads.

  • Status resolution: surfaces the API current_status verbatim as the headline; delivered lifecycle gated on delivered_at.
  • Demurrage evaluation: replaces the invented ~$75-150/day URGENT message with real fees_at_pod_terminal data; suppresses urgency when data is stale, never checked, or tracking stopped \u2014 reading line_tracking_stopped_* from the sideloaded shipment.
  • Temporal formatting: renders timestamps in pod_timezone and computes day deltas between local calendar dates, fixing the UTC off-by-one; surfaces per-channel LFDs in both tool and resource.

Confidence Score: 3/5

Safe to merge for most inputs, but LFD urgency classification and the displayed day count can silently disagree by one day for LFDs near a threshold boundary in a non-UTC terminal timezone.

The urgency label is computed from a UTC millisecond ratio inside evaluateDemurrageUrgency, while get-container.ts then patches only days_until_lfd with the terminal-local calendar value from dayDeltaInZone. When these straddle a threshold — e.g. UTC rounds to 4 days (none) while the local calendar gives 3 (imminent) — the tool returns urgency: 'none' with days_until_lfd: 3, the exact inconsistency the timezone fix was meant to eliminate.

packages/mcp/src/tools/get-container.ts lines 217–232 (where rawDemurrage urgency is computed then days_until_lfd is patched independently) and packages/mcp/src/lib/demurrage.ts (where the UTC-millisecond daysUntilLfd drives urgency classification).

Important Files Changed

FilenameOverview
packages/mcp/src/lib/container-status.tsNew pure module: resolves authoritative container status from API current_status with a clearly-labeled derived lifecycle fallback; delivery gated on delivered_at. Logic is straightforward and well-tested.
packages/mcp/src/lib/demurrage.tsNew pure module: replaces fabricated urgency messages with real fees and staleness-gated LFD urgency. Minor doc nit on total_amount; urgency classification uses UTC milliseconds while the caller patches days_until_lfd with local-calendar values, leaving them potentially mismatched at thresholds.
packages/mcp/src/lib/temporal.tsNew pure module: timezone-aware timestamp formatting and calendar-date-based day-delta math. The en-CA locale trick for YYYY-MM-DD output is correct; DST-safe midnight comparison is sound.
packages/mcp/src/tools/get-container.tsMajor correctness improvements: include-merge fix, real demurrage fees, timezone-aware timestamps, authoritative status. However, urgency is classified using UTC-based millisecond rounding inside evaluateDemurrageUrgency, then days_until_lfd is patched with the local-calendar value — these two can disagree by 1 day near the 3/4-day or 0/-1-day thresholds.
packages/mcp/src/tools/get-shipment-details.tsDrops _mapped bloat, switches to format: 'raw', adds timezone-aware ETA day count via dayDeltaInZone. Clean change with no identified issues.
packages/mcp/src/resources/container.tsAdds shared status resolver, real demurrage evaluation with shipment-level tracking-stopped flag, and per-channel LFDs. Minor: trailing-blank-line issue in the markdown template when urgency is not suppressed and railSection is empty.
packages/mcp/src/tools/contracts.test.tsExpanded contract assertions covering status authority, include-merge, real fees, demurrage suppression paths, and timezone surfacing. Good regression coverage for the changed tool behaviours.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller
participant get_container
participant Terminal49Client
participant ContainerStatusResolver
participant DemurrageEvaluator
participant TemporalFormatter
Caller->>get_container: executeGetContainer(args)
get_container->>get_container: resolveIncludes(args.include)
get_container->>Terminal49Client: "containers.get(id, includes, {format:'raw'})"
Terminal49Client-->>get_container: raw JSON:API response
get_container->>ContainerStatusResolver: resolveContainerStatus(container.attributes)
ContainerStatusResolver-->>get_container: "{status, status_source, derived_lifecycle}"
get_container->>DemurrageEvaluator: "evaluateDemurrageUrgency({fees, pickup_lfd, terminal_checked_at, tracking_stopped})"
Note over get_container,DemurrageEvaluator: tracking_stopped read from sideloaded shipment
DemurrageEvaluator-->>get_container: "{urgency, fees, days_until_lfd (UTC-based)}"
get_container->>TemporalFormatter: dayDeltaInZone(pickup_lfd, pod_timezone)
TemporalFormatter-->>get_container: localDaysUntilLfd (calendar-date based)
get_container->>TemporalFormatter: formatInZone(timestamps, pod_timezone)
TemporalFormatter-->>get_container: human-readable local strings
get_container-->>Caller: ContainerStatus (curated summary, no _mapped bloat)
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 Caller
participant get_container
participant Terminal49Client
participant ContainerStatusResolver
participant DemurrageEvaluator
participant TemporalFormatter
Caller->>get_container: executeGetContainer(args)
get_container->>get_container: resolveIncludes(args.include)
get_container->>Terminal49Client: "containers.get(id, includes, {format:'raw'})"
Terminal49Client-->>get_container: raw JSON:API response
get_container->>ContainerStatusResolver: resolveContainerStatus(container.attributes)
ContainerStatusResolver-->>get_container: "{status, status_source, derived_lifecycle}"
get_container->>DemurrageEvaluator: "evaluateDemurrageUrgency({fees, pickup_lfd, terminal_checked_at, tracking_stopped})"
Note over get_container,DemurrageEvaluator: tracking_stopped read from sideloaded shipment
DemurrageEvaluator-->>get_container: "{urgency, fees, days_until_lfd (UTC-based)}"
get_container->>TemporalFormatter: dayDeltaInZone(pickup_lfd, pod_timezone)
TemporalFormatter-->>get_container: localDaysUntilLfd (calendar-date based)
get_container->>TemporalFormatter: formatInZone(timestamps, pod_timezone)
TemporalFormatter-->>get_container: human-readable local strings
get_container-->>Caller: ContainerStatus (curated summary, no _mapped bloat)
Loading

Fix All in Codex

Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---### Issue 1 of 3
packages/mcp/src/tools/get-container.ts:228-232
**Urgency classification uses UTC milliseconds; displayed `days_until_lfd` uses terminal-local calendar — they can disagree at threshold boundaries**`evaluateDemurrageUrgency` classifies urgency using `Math.round(msDiff / MS_PER_DAY)` which is a raw UTC millisecond ratio. `days_until_lfd` is then overwritten with the calendar-date delta from `dayDeltaInZone`. Near a threshold (e.g. 3 vs 4 days), the two computations can disagree: an LFD that is 3 local calendar days away but ~3.5 UTC days away would yield `urgency: 'none'` (UTC rounds to 4 → `> 3`) with `days_until_lfd: 3` displayed — the exact opposite of the intended fix. The same slip can occur at the `overdue`/`imminent` boundary. The fix is to pass `localDaysUntilLfd` back into urgency classification after computing it, rather than patching `days_until_lfd` in isolation.
### Issue 2 of 3
packages/mcp/src/lib/demurrage.ts:25-26
The JSDoc on `DemurrageEvaluation.total_amount` says "null when fees are present" but should say "null when fees are absent/unknown". Slightly misleading for the empty-array case.
```suggestion /** Sum of fee amounts when fees are present and non-empty, otherwise null. */ total_amount: number | null;```### Issue 3 of 3
packages/mcp/src/resources/container.ts:134-136
When `urgency_suppressed` is false and `railSection` is empty, this template produces two consecutive blank lines at the bottom of the markdown output.
```suggestion${demurrage.urgency_suppressed ? `\n- **LFD Urgency:** Unavailable (${demurrage.suppression_reason})` : ''}${railSection ? `\n${railSection}` : ''}`;```

Reviews (1): Last reviewed commit: "fix(mcp): read line_tracking_stopped_* f..." | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

…ocal times; drop _mapped bloat
Extracts three pure, unit-tested modules under packages/mcp/src/lib/ and wires
them into get_container, the container resource, and get_shipment_details:
- ContainerStatusResolver (container-status.ts): surfaces the API `current_status`
as the headline status in get_container AND resources/container.ts, ending the
three-way status-vocabulary divergence. The heuristic lifecycle is kept only as
clearly-labeled, non-authoritative steering metadata and never reports
"delivered" unless `delivered_at` is set.
- DemurrageUrgencyEvaluator (demurrage.ts): removes the fabricated
"~$75-150/day demurrage accruing / URGENT" message. Surfaces the real
fees_at_pod_terminal (amount + currency) and suppresses urgency when
terminal_checked_at is stale, was never set, or tracking is stopped/closed.
- TemporalFormatter (temporal.ts): renders terminal timestamps in pod_timezone
and computes day-deltas in terminal-local time, fixing the get_shipment_details
"ETA in N days" UTC off-by-one.
Also:
- get_container `include` now AUGMENTS the default [shipment, pod_terminal]
instead of replacing it.
- Surfaces pod_timezone + per-channel LFDs (terminal/rail/line from
import_deadlines) in get_container.
- equipment.length numeric-enum guard (no "" sentinel); drops the phantom
`updated_at` the API never returns on containers.
- Drops the duplicate `_mapped` payload from get_container and
get_shipment_details (no more format:both with no mapper); returns the curated
summary only.
- Removes the dead getContainerTool / getShipmentDetailsTool objects; the Zod
input schema in server.ts is the single source of truth.
Closes DEV-10659
Closes DEV-10661
Closes DEV-10664
Closes DEV-10665
Green gate: SDK build/type-check/test (51 pass, 2 skip) + MCP build/type-check/
test (106 pass) all green; oxlint clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@linear-code

Copy link
Copy Markdown
Contributor

@vercel

vercelBot commented Jun 24, 2026

Copy link
Copy Markdown

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

ProjectDeploymentActionsUpdated (UTC)
apiReadyReadyPreview, CommentJun 26, 2026 10:32am

Request Review

…not the container
line_tracking_stopped_at / line_tracking_stopped_reason live on the SHIPMENT
schema (per the generated OpenAPI types), not the container. The previous code
read them off the container attributes, so on real API data the tracking-stopped
branch never fired and the "suppress demurrage urgency when tracking is
stopped/closed" acceptance criterion was unmet.
get_container now reads these fields from the shipment already resolved out of
the JSON:API included[] (relationships.shipment.data.id), and the container
resource resolves the sideloaded shipment the same way. When the shipment is not
included, both fall back to treating tracking as not-stopped rather than crash.
Adds unit tests proving suppression fires when the sideloaded shipment has
line_tracking_stopped_at set, does not fire when absent, and does not crash when
the shipment is missing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:c14098b185

ℹ️ 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".

Comment threadpackages/mcp/src/tools/get-container.ts Outdated
Comment threadpackages/mcp/src/tools/get-container.ts Outdated
Comment threadpackages/mcp/src/lib/demurrage.ts Outdated
Comment threadpackages/mcp/src/resources/container.ts Outdated
…D safe
Address PR #277 review feedback (codex P2 / greptile P1 + P2s) and known
residuals:
- evaluateDemurrageUrgency now accepts a terminal-local `days_until_lfd` and
classifies `urgency` from it, so the displayed count and the urgency band
can no longer disagree at a threshold boundary (e.g. 3 vs 4 days). get-
container.ts feeds dayDeltaInZone() in instead of patching days_until_lfd
after the fact.
- temporal: treat a date-only value (e.g. "2099-01-10") as a literal calendar
day in the terminal timezone in formatInZone/localCalendarDate/dayDeltaInZone,
fixing a one-day-early render/delta in west-of-UTC zones from the UTC-midnight
parse.
- container resource: drop the stray double blank line at the bottom of the
markdown when urgency isn't suppressed and there's no rail section.
- demurrage: correct the total_amount JSDoc wording.
- tests: add temporal date-only cases, demurrage urgency-from-local-days cases,
and container resource-level tests for status/fees/LFD-urgency rendering.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dodeja

Copy link
Copy Markdown
MemberAuthor

Triaged the automated review (Codex + Greptile) plus the known residuals, and pushed 6b705cf.

Addressed

  • Urgency / days_until_lfd disagreement at threshold boundaries (Codex P2 + Greptile P1, get-container.ts:228-232): evaluateDemurrageUrgency now takes a terminal-local days_until_lfd and classifies urgency from that same count, instead of patching days_until_lfd after the fact while urgency stayed on the raw UTC ms delta. The displayed count and the urgency band can no longer slip apart at a 3-vs-4-day (or overdue/imminent) boundary.
  • Date-only LFD off-by-one (known residual a): formatInZone / localCalendarDate / dayDeltaInZone now treat a date-only value like 2099-01-10 as a literal calendar day in the terminal tz, instead of parsing it as UTC midnight and rendering/deltaing one day early in west-of-UTC zones.
  • Resource-level tests (known residual b): added t49:container tests for authoritative-status, fees (total + None vs Not reported), LFD-urgency, and rail-section rendering.
  • Trailing double blank line (Greptile P2, resources/container.ts:134-136): collapsed so output no longer ends in blank lines when urgency isn't suppressed and there's no rail section.
  • total_amount JSDoc wording (Greptile P2, demurrage.ts:25-26): clarified to 'null when absent/unknown'.

Skipped

  • None of the four inline comments were dropped; all were either applied or subsumed by the urgency refactor (Codex and Greptile flagged the same threshold bug — one fix covers both).

Gate green from the worktree: SDK 51 pass / 2 skip, MCP 121 pass; both builds + type-checks clean. No SDK public-surface change, so no docs/sdk/reference regen needed. Only the changed MCP files were oxfmt-formatted (no mass reformat).

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:6b705cf45d

ℹ️ 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".

if (!attrs.pod_discharged_at) return 'arrived';

// Only the API's explicit delivery confirmation may yield "delivered".
if (attrs.delivered_at) return 'delivered';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Derive delivery from a real API field

For delivered containers this branch never fires because the public container attributes schema/generated types do not define delivered_at (the actual payload exposes current_status: "delivered" and full-out timestamps). That means _metadata.derived_lifecycle and presentation_guidance fall through to discharged/at_terminal, so the tool can steer the LLM toward pickup/demurrage actions instead of confirming delivery. Use current_status === 'delivered' or a real container milestone field for this gate.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flagging for a maintainer decision rather than auto-resolving — this is a legitimate open design question, not something already fixed.

Confirmed details:

  • delivered_at is real in the API payload (present in the container fixtures: containers.get.base.json, containers.get.include.json, containers.list.json) but is NOT yet in the generated OpenAPI container-attributes schema (current_status and import_deadlines are the only attributes typed in that region of generated/terminal49.ts). So the comment is right that the typed schema does not define it, but it is a field the API does emit.
  • The headline status is unaffected: resolveContainerStatus surfaces current_status verbatim, so a current_status: "delivered" container correctly reads "delivered" to the user. The delivered_at gate only governs the explicitly non-authoritative derived_lifecycle and the presentation_guidance/suggestions steering keyed off it.
  • The strict delivered_at-only gate is a deliberate, documented design rule in container-status.ts ("never label a container delivered unless the API has actually set delivered_at"). The fixtures also show current_status: "picked_up" with delivered_at: null and pod_full_out_at set, i.e. terminal-complete containers where the derived lifecycle would fall through to at_terminal/discharged and the guidance could lean toward pickup/demurrage.

So the trade-off is real: switching the gate to current_status === "delivered" (or a full-out milestone) would make the steering metadata track delivery, but it changes the deliberate conservative derivation and would need test updates (container-status.test.ts asserts the current delivered_at behavior). Not resolving — please confirm the intended steering behavior for delivered/picked_up containers and whether to broaden the gate.

Comment threadpackages/mcp/src/tools/get-container.ts
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@dodeja