Skip to content

feat: serve durable inbound SMS history - #233

Draft
Justinabox wants to merge 1 commit into
mainfrom
agent/store-backed-messages-20260727
Draft

feat: serve durable inbound SMS history#233
Justinabox wants to merge 1 commit into
mainfrom
agent/store-backed-messages-20260727

Conversation

@Justinabox

Copy link
Copy Markdown
Owner

Summary

  • Route GET /sms/messages through bounded, durable inbound SMSStore history rather than the process-global event list.
  • Preserve the existing inbound response contract (sender, body, received_at), exclude outbound stored SMS, and keep legacy-service fallback behavior.
  • Keep rare direct +CMT persistence failures observable through a bounded fallback without duplicating successfully persisted messages.
  • Normalize ordering safely across persisted and fallback records; direct SMS persistence now records UTC-aware timestamps.

Validation

  • git diff --check
  • PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q — 881 passed

Risks

  • The process-global fallback remains only for legacy SMS services and rare local-persistence failures; it is deliberately bounded at read time. No hardware interaction was used.

Part of #19.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release-captain review: needs-fix before landing.

The durable-store route and auth preservation are sound, but the fallback path still appends raw inbound sender/body records to the process-global received_messages list without any write-time retention bound (server.py, event handler). The endpoint response limit does not bound the retained sensitive data. Please replace that fallback with a fixed-capacity buffer (or trim on each append) and add a regression that ingests more than the cap and proves only the newest records remain. Re-run the required full suite on the rebased PR head and post the exact result.

This is a focused, no-product-decision blocker.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review: needs-human.

This is a broad public HTTP/SMS-history behavior change (durable persistence, fallback retention, ordering, and response semantics) rather than a narrow isolated bug fix. It needs an explicit product/privacy decision on retention and exposure semantics before autonomous landing. After that decision, rebase on current main, rerun the full required gate, and request a fresh security/API review.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review: needs-human (with a required retention fix).

This broadens SMS-history persistence/exposure and retains raw inbound sender/body data in an unbounded process-global fallback whenever the service is legacy or persistence fails. The limit query only bounds response size, not retained sensitive data.

An explicit product/privacy decision is needed for retention cap/TTL, durable raw-body policy, fallback behavior, and access/authentication expectations. After that decision, add real callback-path tests proving bounded fallback retention and no duplication of durably persisted events, implement the selected policy, rebase on current main, and rerun required gates. It will not be autonomously merged before the decision.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review: needs-fix. The fallback received_messages path retains raw sender/body data without a write-time cap (legacy service and direct +CMT persistence failure); response limiting is not retention limiting. Also SMSStore.list_incoming(limit) limits insertion order before the HTTP route sorts, so a late-saved older timestamp can hide the actual newest item. Please add a fixed-capacity fallback buffer/trim-on-append and chronological ordering before limiting (including SQLite reload/mixed fallback coverage), preserving exactly-once persisted direct-message behavior. Rebase on current main and rerun the required full gate.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review classification: needs-human. This introduces or documents a product-level capability/data-retention/deployment decision beyond a narrow behavior-preserving fix. It will not be auto-merged in this lane. Please retain the branch for Korgo’s product/operational decision, then request a focused follow-up review against current main.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release-lane classification: needs human decision.

This introduces or documents a material product/operational behavior. It requires Korgo's decision on public semantics, retention/operations expectations, and rollout scope before an autonomous merge; local test success alone is not authorization to land it.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release classification: needs-human.

This is a material product/API/data-lifecycle feature rather than a narrow corrective maintenance patch. It needs Korgo’s decision on product scope, compatibility, and operational/privacy semantics before autonomous landing. Keep it draft; after a decision, rebase/repackage over current main and run the prescribed gates.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review: needs-human. Replacing process-local SMS event history with durable /sms/messages behavior has retention, privacy, and external API semantics. Korgo approval is required before merge.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release classification: needs-human decision.

This is a broader externally visible product/API capability (durable SMS history). The scheduled release lane requires product confirmation of the HTTP contract, persistence lifecycle, retention/ordering semantics, and rollout priority before landing.

Sign up for free to 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