Skip to content

Commit 34c13e0

Browse files
committed
Apply failure-status fix to --check-status; track new Facebook posts
Two more Codex findings, both real follow-ups to fixes made earlier this round: 1. The FAILED-status fix (raising instead of silently reporting "uploaded") only applied to publish_tiktok()'s immediate post-upload check, not to check_publish_status() -- exactly the documented `--check-status` recovery path a user is told to use INSTEAD of retrying a real publish blind. Extracted the shared logic into _raise_on_terminal_failure() so both call sites behave identically. A caller using the safe, recommended tool to check an uncertain result must not be told a failed post "succeeded" either. 2. auth/publish_facebook.py's article-scoped verification (this round's earlier fix) narrowed false positives from the profile bio/nav text, but could still match an OLDER post with similar leading text -- a repeated caption, or a common generic opening. Fixed by snapshotting the profile timeline's existing posts before submitting anything (_find_profile_url extracted, plus a new _article_fingerprints helper), then only counting a match in a post that's genuinely new since that snapshot. Added 2 tests for check_publish_status's FAILED/processing paths (no direct unit test for the Facebook browser-flow change itself -- consistent with how every other browser-driven publisher here is tested, via the shared dry-run safe-defaults suite, not by mocking Playwright internals). No live TikTok/Facebook API calls, no account access -- same as every other fix in this PR. 149 tests pass (was 147), compileall clean, git diff --check clean.
1 parent 1e51889 commit 34c13e0

6 files changed

Lines changed: 169 additions & 55 deletions

File tree

.claude/skills/publish-facebook/SKILL.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,13 @@ to know its real character limits or rendering quirks. Keep posts short and simp
3333
run says otherwise.
3434

3535
Verify after posting: reload the profile timeline and match on text unique to the post. The
36-
result dict's own `"verified"` field already tries this automatically (it revisits the profile and
37-
searches the rendered page text), but its `"verify_note"` says plainly when it couldn't confirm --
38-
treat that as "check by hand," not as a failure on its own.
36+
result dict's own `"verified"` field already tries this automatically -- it snapshots the profile
37+
timeline's existing posts *before* submitting anything, then afterward only counts a match in a
38+
post that's genuinely new since that snapshot, not just any post containing similar text (an older
39+
post with the same generic opening line would otherwise false-positive). Still not a unique post
40+
ID -- two genuinely new posts with identical leading text in the same run could still confuse it --
41+
so its `"verify_note"` says plainly when it couldn't confirm; treat that as "check by hand," not as
42+
a failure on its own.
3943

4044
## Guardrails
4145

.claude/skills/publish-tiktok/SKILL.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,10 @@ assume the post is stuck private forever.
8181
own `"status"` field distinguishes this now -- `"published"` means TikTok's status check
8282
reported `PUBLISH_COMPLETE`, `"processing"` means it hadn't finished yet as of that check (use
8383
`--check-status` to follow up), and a terminal `FAILED` status raises an error with TikTok's own
84-
`fail_reason` instead of silently returning either of those. Don't trust a bare `publish_id`
85-
alone as proof of success.
84+
`fail_reason` instead of silently returning either of those. This applies identically to
85+
`--check-status` itself, not just the initial publish call -- the documented "check instead of
86+
retrying" recovery path won't tell you a failed post "succeeded" either. Don't trust a bare
87+
`publish_id` alone as proof of success.
8688
- **Token expired and refresh fails**: `profiles/tiktok/client_secret.json` must still exist (not
8789
just `token.json`) for a refresh to succeed -- see `onboard-tiktok`'s known failures.
8890
- **The connected TikTok account gets reset/wiped after several real publish attempts in a short

CHANGELOG.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ that will firm up once it leaves beta. Dates are when a release was tagged, not
3535
safe-by-default test suite; added a dedicated TikTok OAuth check group to `doctor.py`
3636
(`python doctor.py --tiktok`). Still safe by default either way -- `--confirm-publish` is
3737
required for a real publish on both, exactly as for every other platform.
38-
- **Seven further issues found by code review before any live retest, none of them guessed:**
38+
- **Nine further issues found by code review before any live retest, none of them guessed:**
3939
(1) `auth/publish_facebook.py` never forced its browser context to English like every other
4040
publisher does -- fixed with the same `FORCE_ENGLISH_LOCALE` constant. (2) The documented
4141
`--client-secrets path/to/tiktok_client.json` setup form silently broke every token refresh
@@ -68,7 +68,16 @@ that will firm up once it leaves beta. Dates are when a release was tagged, not
6868
against the flags it sends before publishing -- confirmed against TikTok's public docs that
6969
the `creator_info/query` endpoint exposes this, but fixing it properly needs a live account to
7070
confirm what the init endpoint actually does when a flag doesn't match, so it's left as an
71-
explicit, documented gap rather than guessed.
71+
explicit, documented gap rather than guessed. (8) The FAILED-status fix above was applied to
72+
`publish_tiktok()`'s immediate post-upload check but not to `check_publish_status()` --
73+
exactly the documented `--check-status` recovery path a user is told to use *instead* of
74+
retrying blind. Extracted the shared validation into `_raise_on_terminal_failure()` so both
75+
call sites behave identically; that recovery path won't tell you a failed post "succeeded"
76+
either now. (9) `auth/publish_facebook.py`'s article-scoped verification (added earlier this
77+
round) narrowed false positives from the profile bio/nav text, but could still match an
78+
*older* post with similar leading text (a repeated caption, or a common generic opening).
79+
Fixed by snapshotting the profile timeline's existing posts before submitting anything, then
80+
only counting a match in a post that's genuinely new since that snapshot.
7281
- **YouTube's upload category is no longer hardcoded.** `auth/publish_youtube.py` gained
7382
`--category-id` (default unchanged: `22`, People & Blogs) -- closes a previously-documented
7483
defect in the upload path.

auth/publish_facebook.py

Lines changed: 68 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,36 @@ def _dismiss_cookie_banner(page) -> None:
8181
pass
8282

8383

84+
def _find_profile_url(page) -> str | None:
85+
"""Find the user's own profile link dynamically, the same approach already used for
86+
LinkedIn -- read `.href` (resolved absolute URL), not getAttribute('href'). Facebook's left
87+
rail (and this nav link) persists across pages, so this works from the home feed too, not
88+
just the profile page itself -- used both before posting (to snapshot existing articles) and
89+
after (to verify)."""
90+
return page.evaluate("""() => {
91+
const link = document.querySelector('a[aria-label="Your profile"]')
92+
|| document.querySelector('a[href*="/profile.php?id="]')
93+
|| document.querySelector('div[data-pagelet="LeftRail"] a[role="link"]');
94+
return link ? link.href : null;
95+
}""")
96+
97+
98+
_ARTICLE_FINGERPRINT_PREFIX_LENGTH = 80
99+
100+
101+
def _article_fingerprints(page) -> list[str]:
102+
"""Snapshot every `[role="article"]` (Facebook's own ARIA role for a feed/timeline post)
103+
currently on the page, as a short text prefix each -- used to tell a genuinely NEW post
104+
(one that wasn't in an earlier snapshot) apart from an older post that happens to start with
105+
similar text. Not a unique post ID (unverified against a live account like everything else in
106+
this file), just a much narrower fingerprint than the article's full text."""
107+
return page.evaluate(
108+
"""(prefixLen) => [...document.querySelectorAll('[role="article"]')]
109+
.map(article => (article.innerText || '').slice(0, prefixLen))""",
110+
_ARTICLE_FINGERPRINT_PREFIX_LENGTH,
111+
)
112+
113+
84114
def _open_composer(page) -> bool:
85115
"""Click the "What's on your mind?" opener on the home feed. The visible text includes the
86116
logged-in user's own first name (e.g. "What's on your mind, Alex?"), so this matches on the
@@ -193,6 +223,23 @@ def publish_facebook(
193223
page.wait_for_timeout(2000)
194224
_dismiss_cookie_banner(page)
195225

226+
# Snapshot the profile timeline's existing posts BEFORE submitting anything -- this
227+
# is what makes the later verification step check for a genuinely NEW post rather
228+
# than matching an older one with similar leading text (Codex-reported: scoping the
229+
# match to `[role="article"]` alone narrows out bio/nav-text false positives, but an
230+
# older post starting with the same ~40 characters -- a repeated caption, or a common
231+
# generic opening -- would still false-positive "verified"). Best-effort: if the
232+
# profile link isn't found yet (still unverified selector), verification below just
233+
# falls back to no before-snapshot rather than failing the whole publish over it.
234+
profile_url = _find_profile_url(page)
235+
existing_article_fingerprints: set[str] = set()
236+
if profile_url:
237+
page.goto(profile_url, timeout=STEP_TIMEOUT_MS)
238+
page.wait_for_timeout(3000)
239+
existing_article_fingerprints = set(_article_fingerprints(page))
240+
page.goto("https://www.facebook.com/", timeout=STEP_TIMEOUT_MS)
241+
page.wait_for_timeout(2000)
242+
196243
if not _open_composer(page):
197244
screenshot_path = _save_debug_screenshot(page)
198245
raise RuntimeError(
@@ -281,34 +328,34 @@ def publish_facebook(
281328
+ (f"Screenshot saved to {screenshot_path}." if screenshot_path else "")
282329
)
283330

284-
# Find the user's own profile link dynamically, the same approach already used for
285-
# LinkedIn -- read `.href` (resolved absolute URL), not getAttribute('href').
286-
profile_url = page.evaluate("""() => {
287-
const link = document.querySelector('a[aria-label="Your profile"]')
288-
|| document.querySelector('a[href*="/profile.php?id="]')
289-
|| document.querySelector('div[data-pagelet="LeftRail"] a[role="link"]');
290-
return link ? link.href : null;
291-
}""")
331+
# Re-resolve profile_url here too, in case it wasn't found in the earlier
332+
# before-snapshot step (best-effort there) but is findable now.
333+
if not profile_url:
334+
profile_url = _find_profile_url(page)
292335

293336
verified = False
294337
if profile_url:
295338
snippet = text[:40]
296339
page.goto(profile_url, timeout=STEP_TIMEOUT_MS)
297340
page.wait_for_timeout(4000)
298341
for attempt in range(3):
299-
# Scoped to `[role="article"]` (Facebook's own ARIA role for a feed post),
300-
# not the whole page -- a bare document.body.innerText.includes() check would
301-
# also match the snippet in the profile bio, nav text, or an older post,
302-
# falsely reporting a post as verified when the new one never actually
303-
# appeared. Still unverified against a live account like everything else in
304-
# this file -- this narrows the false-positive surface, it doesn't eliminate
305-
# every way this selector could be wrong.
306-
match = page.evaluate(
307-
"""(snippet) => [...document.querySelectorAll('[role="article"]')]
308-
.some(article => (article.innerText || '').includes(snippet))""",
309-
snippet,
310-
)
311-
if match:
342+
# Codex-reported, in two parts. First: a bare
343+
# document.body.innerText.includes() check would also match the snippet in
344+
# the profile bio or nav text -- fixed by scoping to `[role="article"]`
345+
# (Facebook's own ARIA role for a feed post). Second: scoping to articles
346+
# alone still doesn't prove the match is the post just made, not an older one
347+
# starting with similar text (a repeated caption, or a common generic
348+
# opening) -- fixed by comparing against the before-snapshot taken earlier
349+
# and only counting a match in an article that's NEW since then. Still
350+
# unverified against a live account like everything else in this file -- this
351+
# narrows the false-positive surface further, it doesn't eliminate every way
352+
# this could be wrong (e.g. two genuinely new posts with identical leading
353+
# text in the same run).
354+
current_fingerprints = _article_fingerprints(page)
355+
new_fingerprints = [
356+
fp for fp in current_fingerprints if fp not in existing_article_fingerprints
357+
]
358+
if any(snippet in fp for fp in new_fingerprints):
312359
verified = True
313360
break
314361
page.mouse.wheel(0, 4000)

auth/publish_tiktok.py

Lines changed: 44 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,36 @@ def _upload_video_chunks(
327327
offset += len(chunk)
328328

329329

330+
def _raise_on_terminal_failure(status_response: dict, publish_id: str) -> str | None:
331+
"""Interpret a status_response from STATUS_FETCH_URL and return TikTok's own status string
332+
(e.g. "PUBLISH_COMPLETE", "PROCESSING_UPLOAD") unchanged -- or raise on a genuine API-level
333+
error or a terminal FAILED status. TikTok's documented post-status values
334+
(https://developers.tiktok.com/doc/content-posting-api-reference-get-video-status):
335+
PUBLISH_COMPLETE (done), FAILED (terminal, with a fail_reason field), and several
336+
PROCESSING_* values for "still working on it."
337+
338+
Shared by publish_tiktok()'s immediate post-upload check and check_publish_status()'s
339+
--check-status follow-up -- Codex-reported: the first version of this fix only applied to the
340+
former, leaving the documented "use --check-status instead of retrying" recovery path just as
341+
blind to a real failure as the original bug it was meant to replace would have been."""
342+
status_error = status_response.get("error", {})
343+
if status_error.get("code") not in (None, "ok"):
344+
raise RuntimeError(
345+
f"TikTok status check failed for publish_id={publish_id}: {status_error}. "
346+
"Do not retry blind -- use --check-status to re-check later."
347+
)
348+
349+
tiktok_status = status_response.get("data", {}).get("status")
350+
if tiktok_status == "FAILED":
351+
fail_reason = status_response.get("data", {}).get("fail_reason", "unknown")
352+
raise RuntimeError(
353+
f"TikTok rejected this post (fail_reason={fail_reason}, publish_id={publish_id}). "
354+
"Do not retry blind -- see the troubleshoot-publishing skill before attempting "
355+
"another real publish."
356+
)
357+
return tiktok_status
358+
359+
330360
def publish_tiktok(
331361
video_path: str,
332362
title: str = "",
@@ -423,31 +453,7 @@ def publish_tiktok(
423453
_upload_video_chunks(upload_url, video_file, effective_chunk, total_size, total_chunk_count)
424454

425455
status_response = _api_post_json(STATUS_FETCH_URL, access_token, {"publish_id": publish_id})
426-
427-
# Codex-reported regression, caught before any live test: this used to always return
428-
# "status": "uploaded" regardless of what status_response actually said, even a terminal
429-
# FAILED -- reporting a rejected post as if it succeeded unless the caller happened to
430-
# manually inspect the nested status_response. TikTok's documented post-status values
431-
# (https://developers.tiktok.com/doc/content-posting-api-reference-get-video-status):
432-
# PUBLISH_COMPLETE (done), FAILED (terminal, with a fail_reason field), and several
433-
# PROCESSING_* values for "still working on it" -- see check_publish_status above for
434-
# re-checking one of those later. A FAILED status must raise, not silently succeed; anything
435-
# short of PUBLISH_COMPLETE is honestly reported as still processing, not "uploaded" outright.
436-
status_error = status_response.get("error", {})
437-
if status_error.get("code") not in (None, "ok"):
438-
raise RuntimeError(
439-
f"TikTok status check failed for publish_id={publish_id}: {status_error}. "
440-
"Do not retry blind -- use --check-status to re-check later."
441-
)
442-
443-
tiktok_status = status_response.get("data", {}).get("status")
444-
if tiktok_status == "FAILED":
445-
fail_reason = status_response.get("data", {}).get("fail_reason", "unknown")
446-
raise RuntimeError(
447-
f"TikTok rejected this post after upload (fail_reason={fail_reason}, "
448-
f"publish_id={publish_id}). Do not retry blind -- see the troubleshoot-publishing "
449-
"skill before attempting another real publish."
450-
)
456+
tiktok_status = _raise_on_terminal_failure(status_response, publish_id)
451457

452458
return {
453459
"dry_run": False,
@@ -472,11 +478,22 @@ def check_publish_status(publish_id: str) -> dict:
472478
was returned right after upload. Added 2026-08-18 after a real publish_id came back
473479
"PROCESSING_UPLOAD" (a transient state) and the video never actually appeared in the TikTok
474480
app -- the original code only ever checked status once, immediately after uploading, and
475-
never found out whether processing later succeeded or failed."""
481+
never found out whether processing later succeeded or failed.
482+
483+
This is the documented, safe way to check an uncertain publish instead of retrying it for
484+
real -- see the troubleshoot-publishing/publish-tiktok guardrails. It must raise on a genuine
485+
FAILED status exactly like publish_tiktok()'s own post-upload check does (via the same shared
486+
_raise_on_terminal_failure), not just return the raw response unchanged: a caller using this
487+
tool specifically to avoid a blind retry must not be told a failed post "succeeded" either."""
476488
token = _load_token()
477489
token = _refresh_token_if_needed(token)
478490
status_response = _api_post_json(STATUS_FETCH_URL, token["access_token"], {"publish_id": publish_id})
479-
return {"publish_id": publish_id, "status_response": status_response}
491+
tiktok_status = _raise_on_terminal_failure(status_response, publish_id)
492+
return {
493+
"publish_id": publish_id,
494+
"status": "published" if tiktok_status == "PUBLISH_COMPLETE" else "processing",
495+
"status_response": status_response,
496+
}
480497

481498

482499
def main() -> None:

tests/test_publish_tiktok.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,12 +350,47 @@ def fake_api_post_json(url, access_token, body):
350350
result = check_publish_status("v_pub_file~v2-1.example")
351351

352352
assert result["publish_id"] == "v_pub_file~v2-1.example"
353+
assert result["status"] == "published"
353354
assert result["status_response"]["data"]["status"] == "PUBLISH_COMPLETE"
354355
assert captured["access_token"] == "tok123"
355356
assert captured["body"] == {"publish_id": "v_pub_file~v2-1.example"}
356357
assert "status/fetch" in captured["url"]
357358

358359

360+
def test_check_publish_status_raises_on_terminal_failed_status(monkeypatch):
361+
"""Codex-reported gap: the FAILED-status fix was applied to publish_tiktok()'s immediate
362+
post-upload check but not to check_publish_status() -- the documented --check-status recovery
363+
path a user is told to use INSTEAD of retrying blind. That path must not be just as capable of
364+
silently reporting a failed post as a success as the original bug it was meant to fix."""
365+
monkeypatch.setattr("auth.publish_tiktok._load_token", lambda: {"access_token": "tok123"})
366+
monkeypatch.setattr("auth.publish_tiktok._refresh_token_if_needed", lambda t: t)
367+
monkeypatch.setattr(
368+
"auth.publish_tiktok._api_post_json",
369+
lambda url, access_token, body: {
370+
"data": {"status": "FAILED", "fail_reason": "video_format_check_failed"},
371+
"error": {"code": "ok"},
372+
},
373+
)
374+
375+
with pytest.raises(RuntimeError, match="video_format_check_failed"):
376+
check_publish_status("v_pub_file~v2-1.example")
377+
378+
379+
def test_check_publish_status_reports_still_processing_honestly(monkeypatch):
380+
monkeypatch.setattr("auth.publish_tiktok._load_token", lambda: {"access_token": "tok123"})
381+
monkeypatch.setattr("auth.publish_tiktok._refresh_token_if_needed", lambda t: t)
382+
monkeypatch.setattr(
383+
"auth.publish_tiktok._api_post_json",
384+
lambda url, access_token, body: {
385+
"data": {"status": "PROCESSING_UPLOAD"},
386+
"error": {"code": "ok"},
387+
},
388+
)
389+
390+
result = check_publish_status("v_pub_file~v2-1.example")
391+
assert result["status"] == "processing"
392+
393+
359394
def test_doctor_registers_tiktok_without_treating_it_as_a_browser_platform():
360395
"""TikTok, like YouTube, uses OAuth + an API and never touches a browser profile -- it must
361396
not be listed alongside the Chrome-session platforms, but it must have its own check group."""

0 commit comments

Comments
 (0)