forked from molecule-ai/molecule-core
9991057ad1
Resolves four of six findings from the retrospective code review of Phases 1–4 (poll-mode chat upload). Bundled because every change is in the platform's pending_uploads layer or the multi-file handler that reads it. Findings resolved: 1. Important — Sweep query lacked an index for the acked-retention OR-arm. The Phase 1 partial indexes are both `WHERE acked_at IS NULL`, so the `(acked_at IS NOT NULL AND acked_at < retention)` half of the WHERE clause seq-scanned the table on every cycle. Add a complementary partial index on `acked_at WHERE acked_at IS NOT NULL` so both arms of the disjunction are index-covered. Disjoint from the existing two indexes (no row matches both predicates), so write amplification is bounded to ~one index entry per terminal-state row. 2. Important — uploadPollMode partial-failure left orphans. The previous per-file Put loop committed rows 1..K-1 and then errored on row K with no compensation, so a client retry would double-insert the survivors. Refactor the handler into three explicit phases (pre-validate + read-into-memory, single atomic PutBatch, per-file activity row) and add Storage.PutBatch with all-or-nothing transaction semantics. 3. FYI — pendinguploads.StartSweeperWithInterval was exported only for tests. Move it to lower-case startSweeperWithInterval and expose the test seam through pendinguploads/export_test.go (Go convention; the shim file is stripped from the production binary at build time). 4. Nit — multipart Content-Type was passed verbatim into pending_uploads rows and re-served on /content. Add safeMimetype which strips parameters, rejects CR/LF/control bytes, and coerces malformed shapes to application/octet-stream. The eventual GET /content response can no longer be header-split via a crafted Content-Type on the multipart. Comprehensive tests: - 10 PutBatch unit tests (sqlmock): happy path, empty input, all four pre-validation rejection paths, BeginTx error, per-row error + Rollback (no Commit), first-row error, Commit error. - 4 new PutBatch integration tests (real Postgres): all-rows-commit happy path with COUNT(*) verification, atomic-rollback no-leak via a NUL-byte filename that lib/pq rejects mid-batch, oversize short-circuit no-Tx, idx_pending_uploads_acked existence + partial predicate via pg_indexes (planner-shape-independent). - 3 new chat_files_poll tests: atomic rollback on second-file oversize, atomic rollback on PutBatch error, mimetype CRLF/NUL/parameter sanitization (8 sub-cases). The two remaining review findings (inbox_uploads.fetch_and_stage blocks the poll loop synchronously; two httpx Clients per row) are Python-side and ship in Phase 5b once this lands on staging. Test-only export pattern via export_test.go, atomic pre-validation discipline (validate before Tx), and behavior-based (not name-based) test assertions follow the standing project conventions. |
||
|---|---|---|
| .. | ||
| 001_workspaces.sql | ||
| 002_agents.sql | ||
| 003_events.sql | ||
| 004_secrets.sql | ||
| 005_canvas_layouts.sql | ||
| 006_workspace_config_memory.sql | ||
| 007_approvals.sql | ||
| 008_agent_memories.sql | ||
| 009_activity_logs.sql | ||
| 010_workspace_awareness.sql | ||
| 011_workspace_runtime.sql | ||
| 012_global_secrets.sql | ||
| 013_workspace_dir.sql | ||
| 014_indexes.sql | ||
| 015_workspace_schedules.sql | ||
| 016_workspace_channels.sql | ||
| 017_memories_fts_namespace.down.sql | ||
| 017_memories_fts_namespace.up.sql | ||
| 018_secrets_encryption_version.down.sql | ||
| 018_secrets_encryption_version.up.sql | ||
| 019_workspace_access.down.sql | ||
| 019_workspace_access.up.sql | ||
| 020_workspace_auth_tokens.down.sql | ||
| 020_workspace_auth_tokens.up.sql | ||
| 021_delegation_idempotency.down.sql | ||
| 021_delegation_idempotency.up.sql | ||
| 022_workspace_schedules_source.down.sql | ||
| 022_workspace_schedules_source.up.sql | ||
| 023_workspace_memory_version.down.sql | ||
| 023_workspace_memory_version.up.sql | ||
| 024_channel_budget.down.sql | ||
| 024_channel_budget.up.sql | ||
| 025_workspace_token_usage.down.sql | ||
| 025_workspace_token_usage.up.sql | ||
| 026_org_plugin_allowlist.down.sql | ||
| 026_org_plugin_allowlist.up.sql | ||
| 027_workspace_budget.down.sql | ||
| 027_workspace_budget.up.sql | ||
| 028_workspace_artifacts.down.sql | ||
| 028_workspace_artifacts.up.sql | ||
| 029_workspace_hibernation.down.sql | ||
| 029_workspace_hibernation.up.sql | ||
| 030_audit_events.down.sql | ||
| 030_audit_events.up.sql | ||
| 031_memories_pgvector.down.sql | ||
| 031_memories_pgvector.up.sql | ||
| 032_schedule_consecutive_empty.down.sql | ||
| 032_schedule_consecutive_empty.up.sql | ||
| 033_strip_crlf_cron_prompts.up.sql | ||
| 034_workspaces_last_outbound_at.up.sql | ||
| 035_org_api_tokens.down.sql | ||
| 035_org_api_tokens.up.sql | ||
| 036_org_api_tokens_org_id.down.sql | ||
| 036_org_api_tokens_org_id.up.sql | ||
| 037_max_concurrent_tasks.down.sql | ||
| 037_max_concurrent_tasks.up.sql | ||
| 038_workspace_instance_id.down.sql | ||
| 038_workspace_instance_id.up.sql | ||
| 039_activity_tool_trace.down.sql | ||
| 039_activity_tool_trace.up.sql | ||
| 040_platform_instructions.down.sql | ||
| 040_platform_instructions.up.sql | ||
| 042_a2a_queue.down.sql | ||
| 042_a2a_queue.up.sql | ||
| 043_workspace_status_enum.down.sql | ||
| 043_workspace_status_enum.up.sql | ||
| 044_platform_inbound_secret.down.sql | ||
| 044_platform_inbound_secret.up.sql | ||
| 045_workspaces_delivery_mode.down.sql | ||
| 045_workspaces_delivery_mode.up.sql | ||
| 046_workspace_status_awaiting_agent.down.sql | ||
| 046_workspace_status_awaiting_agent.up.sql | ||
| 047_runtime_image_pins.down.sql | ||
| 047_runtime_image_pins.up.sql | ||
| 048_activity_logs_peer_indexes.down.sql | ||
| 048_activity_logs_peer_indexes.up.sql | ||
| 049_delegations.down.sql | ||
| 049_delegations.up.sql | ||
| 20260417000000_workflow_checkpoints.down.sql | ||
| 20260417000000_workflow_checkpoints.up.sql | ||
| 20260505100000_pending_uploads.down.sql | ||
| 20260505100000_pending_uploads.up.sql | ||
| 20260505200000_pending_uploads_acked_index.down.sql | ||
| 20260505200000_pending_uploads_acked_index.up.sql | ||