Repository navigation
Conversation
…ovider Narrow follow-up to OpenMontageInc#7 as requested in its triage: one video-only tool under the publish capability (provider upload_post), next to export_bundle. - Publishes a finished render to TikTok, Instagram Reels, YouTube, LinkedIn, Facebook, X, Threads, Pinterest and Bluesky and returns a schema-valid publish_log with one entry per platform. - The request id is a hash of the video, cover and caption being published and is sent as the Idempotency-Key. Before uploading, the tool checks whether that id already exists and resumes it, so re-running an interrupted publish never posts twice. - A dropped connection is never answered with a re-send; the tool polls the same id. - A ledger next to the export bundle blocks publishing a different version of the same render (e.g. a corrected cover) while an earlier one is live or in flight, unless allow_additional_post is set. - publish_log.metadata records the request id and the SHA-256 of the video, thumbnail and caption actually sent. - dry_run validates, fingerprints and reports connected platforms and conflicts without uploading; the publish-director only publishes after the stage's approval gate. Docs: PROVIDERS.md section + tables, .env.example, ARCHITECTURE.md, publish-director Step 5b, animated-explainer tools_available. Tests: 19 mocked tests (no network), including the interrupted-create and corrected-cover cases.
Only definitive pre-acceptance rejections (400/401/402/403/404/413/422/429) and a connect timeout may end as 'failed' and allow re-sending. A 5xx, a dropped connection, a timeout or a 2xx without a JSON body may still have been accepted: the tool polls the request id, keeps tracking it if it exists, and otherwise records an explicit 'ambiguous' state in the ledger that blocks re-sending — across runs and restarts — until the user confirms nothing went out (confirm_not_published). This replaces the old 'not_received' state, which treated an unconfirmed upload as safe to resend. Tests: 503 and empty 2xx on the first submit -> re-run -> 0 extra uploads, also from a fresh process reading the ledger; crash after 'submitting'; 503 after server-side acceptance is tracked, not failed; 4xx and connect timeout still allow a retry.
|
Pushed 4b62b6b: a hardening fix to how upload errors are classified. Only definitive pre-acceptance rejections (400/401/402/403/404/413/422/429) and a connect timeout now end as New tests (no network):
With the previous commit, the 503, empty-2xx, restart and dropped-connection tests fail (it re-sent). With this one, all 28 pass. The full suite shows 1857 passed and |
…issue OpenMontageInc#664, advance baseline Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
calesthio
left a comment
There was a problem hiding this comment.
Thank you for the PR and the work on this!
The new ambiguous-upload handling is a good improvement and all 28 supplied tests pass. A completed-request replay case still bypasses the duplicate guard.
[P1] Treat a locally completed submission as durable evidence even when the remote status endpoint later returns 404 (execute around the UNRESOLVED_STATES check). _conflicts skips the same request_id, while UNRESOLVED_STATES omits completed. I wrote a completed ledger entry, mocked _get_status to return None, and reran identical inputs: _submit was called once again, without either confirmation flag. This can create a duplicate after remote status/idempotency retention expires—the exact restart scenario the module promises to prevent. Return the recorded completion or require deliberate additional-post authorization; never infer non-publication from a missing remote record.
[P1] Fail closed on an unreadable/corrupt existing ledger. _read_ledger currently turns every OSError/ValueError into an empty history, which can discard the only evidence of an earlier accepted upload. Distinguish a genuinely absent ledger from a damaged/unreadable one and require recovery before publishing.
Please add restart tests for completed+404 and corrupt/unreadable ledger. All review reproductions used mocks; no content was published.
…corrupt ledgers never re-upload - A publish recorded as completed is answered from the ledger when Upload-Post no longer has the request (status/idempotency retention expired, GET status 404). The outcome (normalized results + publish_log) is stored with the entry on completion, so the replay returns the recorded completion instead of inferring 'never published' from the missing remote record. Only allow_additional_post=true sends the same publish again. - _read_ledger distinguishes an absent ledger (empty history) from one that exists but cannot be read or parsed (OSError other than ENOENT, invalid JSON, unexpected structure): the latter raises LedgerError and execute/dry_run fail closed with ledger_unreadable=true, leaving the file untouched. - Restart tests: completed + remote 404 (and with a platform failure), corrupt ledger, unreadable ledger, missing ledger.
|
Thanks for the two reproductions. Both are fixed in [P1] Completed submission replayed after the remote record expires. A ledger entry in state [P1] Unreadable or corrupt ledger. Tests added: |
|
@calesthio both reproductions from your last review are fixed in e8a2400, with restart tests for each (details in my comment above). Would you mind taking another look when you have a moment? The CI run for e8a2400 is still waiting for maintainer approval, so it has not run yet. Thanks! |
calesthio
left a comment
There was a problem hiding this comment.
Thank you for addressing the earlier status and ledger feedback. I reviewed the updated implementation; the previous findings have been improved, but two current-head blockers remain.
[P1] Do not mark a partially pending publish as successful. In tools/publishers/upload_post_publisher.py::execute, success is bool(published) and not failed. A request with YouTube published and Instagram still processing therefore returns success=True. I reproduced this with a mocked processing response containing those two platform results. This lets the production advance while a requested publication is still incomplete. Require every requested platform to reach an accepted terminal state before reporting complete success, and preserve the request ID and explicit pending/partial status for polling/resume. Add a mixed completed/processing test. The provider documents asynchronous status polling.
[P1] Restore Windows test collection. tests/tools/test_upload_post_publisher.py:434 calls os.geteuid() in a skip condition without checking whether that function exists. Windows has Path.chmod but no os.geteuid, so the focused suite fails during collection with AttributeError and can prevent the full suite from collecting. Guard the platform/function before calling it and verify the test is skipped appropriately on Windows.
No real upload or external publication was performed.
…; tests collect on Windows - success now requires every requested platform to be published (or delivered as a draft). One still processing returns success=False with an explicit status (partial_pending / pending / partial_failure), the request id and the pending platforms, and the ledger keeps the request in_flight so a re-run polls it instead of uploading again. - the unreadable-ledger test looked up os.geteuid in its skip condition, which does not exist on Windows and broke collection. The condition now checks the platform and the function first.
|
@calesthio thanks for the second pass. Both blockers are fixed in 95e4d5a, and the branch is merged with current [P1] Partially pending publish reported as successful. Reproduced as you described: YouTube published plus another platform still processing returned
New tests, all failing on e8a2400:
[P1] Windows test collection. The skip condition no longer calls Results on Python 3.11: No real upload was made for this change; everything runs against the in-memory fake. |
This is the narrower follow-up to #7 that you asked for when closing it in triage:
It adds one networked provider under the
publishcapability, next toexport_bundle, following the extension point inexport_bundle's own docstring ("A networked publisher ... can be added later as a separateproviderunder the samepublishcapability").How this addresses each point from #7
tools/publishers/upload_post_publisher.py(tierPUBLISH, capabilitypublish, providerupload_post). Video only, publish now. #7 covered photos, documents, text posts and 11 platforms; none of that is here. No scheduling, comments or DMs.tests/tools/test_upload_post_publisher.py: 19 mocked tests, no network (your session guard stays on). They cover the contract, registry discovery, status with and without the key, validation, request identity, a schema-validpublish_log, per-platform outcomes, and the interruption cases below. The full suite is green: 1848 passed, against 1829 onmain+ the 19 new ones, with no other changes.make lintpasses.docs/PROVIDERS.mdgets a provider section (setup, scope, pricing) plus rows in the env summary, the mapping and the coverage tables. Also.env.example,docs/ARCHITECTURE.md(thepublishers/line was "(Reserved)"), a Step 5b inpublish-director.md, andanimated-explainer.yamltools_available, so the manifest and the skill agree.Design: publishing is irreversible, so never post twice or post stale inputs
Most platforms can't unpublish, or swap a cover, through their APIs once a post is live. A publisher that retries blindly is dangerous. That includes the failure mode seen in this repo in #410: an interrupted create was retried with stale inputs, and the wrong cover went live with no way to correct it. The tool is built around preventing that:
attemptcounter. It's sent as bothrequest_idandIdempotency-Key. The same publish always maps to the same id, and a changed cover or caption is by construction a different publish.exports/upload_post_ledger.json) records every submission. If a different version of the same render (another cover or caption) is live or still in flight on an overlapping platform, the tool refuses to publish and names the earlier request.allow_additional_post: trueis the explicit override.publish_logwith one entry per platform:published/failed/draft(TikTok inbox) /pending_review(still processing), plus URL or post id and visibility.publish_log.metadatarecords the request id and the hashes of the video, thumbnail and caption that were actually sent. No schema changes.dry_run: truevalidates, fingerprints, and reports connected platforms,missing_platformsandconflictswithout uploading. The publish-director calls the dry run before the stage's human-approval gate and publishes only after approval, with the same inputs.private, and TikTok uses the account's own privacy.visibility: "private"is rejected for platforms with no private mode rather than posting publicly.retry_policy.max_retries = 0, because resumption goes through the request id instead.Registry check (no key set):
publish: configured 1/2, available ["local"], unavailable ["upload_post"], andget_status()isunavailablewith setup instructions.Provider-viability evidence
Real end-to-end run from this branch (test profile, a synthetic 1080×1920 render, TikTok
SELF_ONLY+ YouTubeprivate; the posts are private):1.
dry_run: validates, fingerprints, checks the account, uploads nothing{ "tool": "upload_post_publisher", "status": "available", "estimated_cost_usd": 0.0, "would_execute": true, "request_id": "om-8f6606699b5b7e8af0874b62d93ac3f5", "platforms": ["tiktok", "youtube"], "caption": "OpenMontage x Upload-Post provider test #openmontage", "input_hashes": { "video_sha256": "11d1e4610bd182b7aeedd345a6b70410730b8717ff51c238def672cc98225c03", "thumbnail_sha256": "87b31cba93ae1c2be1431858d0fababc32c7a4414ce815938689ef14c453fc5c", "caption_sha256": "536027b1bdeda5942a894cf976025a244db57d093c6b00307a674827e101a56f" }, "missing_platforms": [], "already_submitted": false, "conflicts": [] }2. Publish:
success=Truein 22 s, schema-validpublish_log{ "version": "1.0", "entries": [ {"platform": "tiktok", "status": "published", "video_id": "v_pub_file~v2-1.7691061209349343254", "visibility": "private", "metadata_used": {"title": "OpenMontage x Upload-Post provider test #openmontage", "description": "Provider-viability test of tools/publishers/upload_post_publisher.py", "hashtags": ["openmontage"]}, "timestamp": "2026-09-29T20:46:38.512288+00:00"}, {"platform": "youtube", "status": "published", "url": "https://www.youtube.com/watch?v=uuawQDIRsM4", "video_id": "uuawQDIRsM4", "visibility": "private", "metadata_used": {"title": "OpenMontage x Upload-Post provider test #openmontage", "description": "Provider-viability test of tools/publishers/upload_post_publisher.py", "hashtags": ["openmontage"]}, "timestamp": "2026-09-29T20:46:38.512288+00:00"} ], "metadata": { "provider": "upload_post", "request_id": "om-8f6606699b5b7e8af0874b62d93ac3f5", "attempt": 1, "resumed_existing_request": false, "upload_post_status": "completed", "caption_sent": "OpenMontage x Upload-Post provider test #openmontage", "input_hashes": {"video_sha256": "11d1e461…", "thumbnail_sha256": "87b31cba…", "caption_sha256": "536027b1…"}, "notes": {"youtube": "Post uploaded as Private. No public URL available."} } }3. Re-running the same publish resumes it: no second upload
{"resumed": true, "request_id": "om-8f6606699b5b7e8af0874b62d93ac3f5", "statuses": [["tiktok", "published"], ["youtube", "published"]]}4. The same render with a corrected cover is blocked while the first version is live
{"success": false, "error": "A different version of this render was already submitted and is live or in flight: om-8f6606699b5b7e8af0874b62d93ac3f5 (completed) on tiktok, youtube. Publishing now would create a second post. Check those posts first; set allow_additional_post=true only if a second post is intended.", "conflicts": [{"request_id": "om-8f6606699b5b7e8af0874b62d93ac3f5", "state": "completed", "platforms": ["tiktok", "youtube"]}]}5. A real mid-upload disconnect: one upload, no re-send, both platforms published once
The read timeout was forced to 50 ms so the client hung up while the API was still answering:
Provider facts:
Authorization: Apikey …). Social accounts are connected once in the Upload-Post dashboard, with no developer app or OAuth client per platformestimate_cost()returns0.0andPROVIDERS.mdstates the plansupload-post(2.13.0), JS SDKupload-post(2.14.1, ~5.1k downloads last month), MCP server@upload-post/mcp, n8n community nodeapp/services/upload_post.py) and autoclip (backend/services/upload_post_publisher.py, v1.3.2)Files
tools/publishers/upload_post_publisher.py: the tooltests/tools/test_upload_post_publisher.py: 19 mocked testsdocs/PROVIDERS.md,docs/ARCHITECTURE.md,.env.exampleskills/pipelines/explainer/publish-director.md(Step 5b, and Step 6 on merging log entries)pipeline_defs/animated-explainer.yaml(tools_available)Happy to trim further. For example, I can drop the ledger guard or the
publish-directorstep if you'd rather land the tool alone first.