Update health check retries in Dockerfile#143
Open
mrdulasolutions wants to merge 7 commits intoverygoodplugins:mainfrom
Open
Update health check retries in Dockerfile#143mrdulasolutions wants to merge 7 commits intoverygoodplugins:mainfrom
mrdulasolutions wants to merge 7 commits intoverygoodplugins:mainfrom
Conversation
Increased the number of retries for the health check from 3 to 10.
Add a /documents API and MCP tools so agents can upload files to AutoMem. The file stays in the bucket as an opaque original; AutoMem never parses content. The uploading agent must read the file first and provide a human-quality title + summary, which become the Memory's content so existing vector/keyword recall surfaces the doc. Raw bytes are retrieved later via a short-lived presigned URL. ## Why DocDeploy-style document capability without the weight of full extraction. Works with Railway Buckets out of the box (plus any S3-compatible store: MinIO, Cloudflare R2, Wasabi, AWS S3). ## The "gate" POST /documents returns 422 if title or summary is missing — the agent-generated description IS the searchable representation. No OCR, no pdfplumber, no trafilatura. When an agent needs the raw bytes it calls get_document_url for a presigned URL (Claude reads PDFs natively). ## Surface area HTTP (automem/api/documents.py): POST /documents - multipart upload (file, title, summary, ...) GET /documents - list Document memories (filter by tags) GET /documents/:id/download - presigned URL (30s-1h, default 5min) DELETE /documents/:id - clean up graph + vector + bucket MCP (mcp-sse-server/server.js): upload_document - forces title+summary via required schema fields get_document_url - presigned URL for downstream agents list_documents delete_document ## Implementation notes - BucketStore wrapper around boto3; virtual-host vs path addressing, signature_version=s3v4, retries baked in. - Streaming upload with inline SHA-256 (no full-file buffering). - Rolls back bucket object on graph failure. - bucket_store is optional: unconfigured env => 503 with clear message, never a 500. Enables local/dev runs without S3 creds. - Document added to MEMORY_TYPES; TYPE_ALIASES maps "document" => "Document". ## Tests tests/test_documents.py: 14 cases covering the 422 gate, the 400 cases (missing file, invalid UUID, oversized title), happy-path upload, presigned download, delete, and the 503 path. Uses an in-memory FakeBucketStore to keep the suite boto3-free. Also updates tests/contracts/test_routes_contract.py to include the four new public routes. ## Deploy Bucket creds wired to the AutoMem service on Railway via S3_* env vars (see /tmp/automem-tokens.env + bucket credentials in the dashboard). Rails deploys from mrdulasolutions/automem main.
Author
|
fire |
feat: agent-driven document storage via S3-compatible bucket
The feat/documents branch was formatted with a different black version than pre-commit pins, so CI's pre-commit hook failed on merge. No behavioral changes — purely black/isort formatting to match the repo's 100-char line length. All 15 document tests + route contract test still pass locally; pre-commit run --all-files is green. Also adds .venv-test/ to .gitignore for the local virtualenv used during development.
style(ci): reformat to match repo's black 24.4.2 + line-length=100
Some MCP client transports stringify nested object/array arguments before calling tools, even when the input schema declares them as `object` or `array`. AutoMem then rejects the body with `'metadata' must be an object` because its strict JSON parse expects native types. Fix: add `coerceJsonFields(args, fields)` that JSON-parses string values where the target field expects an object (metadata) or array (embedding, tags) and the parsed shape matches. Malformed JSON and shape mismatches pass through unchanged so the upstream service returns its normal clear error. Applied in `storeMemory` and `updateMemory` before building the request body. `uploadDocument` was already tolerant (it builds a multipart form and stringifies inline). ## Tests Added 6 cases to `mcp-sse-server/test/server.test.js`: - parses stringified metadata objects - parses stringified tags arrays - leaves already-native values alone (strict identity check) - leaves malformed JSON strings as-is - rejects shape mismatches (array passed where object expected) - `AutoMemClient.storeMemory` end-to-end with stringified args 17/17 node tests pass (11 existing + 6 new). Also includes a package-lock.json sync (0.1.0 → 0.2.0 to match package.json). Co-authored-by: MRDula <mac@MRDulas-MacBook-Pro.local>
jack-arturo
requested changes
Apr 23, 2026
Member
jack-arturo
left a comment
jack-arturo
left a comment
There was a problem hiding this comment.
This PR is not reviewable in its current form.
The title/body describe a small Docker health-check retry change, but the actual diff currently includes multiple unrelated changes bundled together: the Dockerfile tweak, a new /documents API plus S3 bucket store and tests, new MCP document tools, a separate MCP JSON-coercion fix, and formatting/lockfile/merge-churn. That makes review, rollback, and release notes harder than they need to be.
Please split this up and resubmit as focused PRs instead of continuing review on this branch.
Suggested split:
chore/ops: Docker health-check retry only.feat/documents:/documentsAPI, bucket store, config wiring, tests.fix(mcp): JSON-stringified arg coercion inmcp-sse-server/server.jsplus its tests.
Please also:
- branch from current upstream
main - avoid opening from fork
main - remove merge commits and unrelated churn
- rewrite the PR title/body so they match the actual scope
If the documents feature comes back as its own PR, it also needs docs updates for the public surface it adds:
docs/API.mdfor the new/documentsendpointsdocs/ENVIRONMENT_VARIABLES.mdforS3_*andDOCUMENT_*docs/MCP_SSE.mdfor the new MCP tools- likely
README.mdand/ordocs/RAILWAY_DEPLOYMENT.mdif Railway bucket setup is now part of the supported path
For the resubmitted PRs:
- Dockerfile PR: minimal smoke validation is fine.
- MCP coercion PR: keep the existing node tests proving object/array coercion behavior.
- Documents PR: keep the unit tests, and either add integration coverage for the S3-backed path or explicitly document why that coverage is deferred.
There may be good work in here, but it needs to be separated into reviewable units first.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Increased the number of retries for the health check from 3 to 10.