Real-Telegram test suite green (11/11) + manual /add barcode OCR#269
Merged
Conversation
…od logs on failure The "Verify webhook self-registration" step's http=404 was misdiagnosed as "webhook not registered" — a 404/ok=false from Telegram actually means the TOKEN itself was rejected (an unregistered webhook on a valid token returns http=200, ok=true, url=""). Adds an early getMe-based token validation step (shape/whitespace diagnostics, never prints the value) so a bad secret fails in seconds instead of after the ~5min deploy, splits the misleading error message into the two distinct failure shapes, and dumps kubectl pod/logs on failure before the always()-teardown step deletes the evidence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU
Add a getChat/getChatMemberCount validation step for the real-test CI group, positioned right after the existing bot-token check. A basic group's Telegram id changes shape when Telegram auto-upgrades it to a supergroup, and MembershipService.IsMember collapses that failure mode into the same "not a member" false as a genuine non-member — this catches a stale id in seconds instead of every real test timing out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU
…NCE_MS override Run 30162785282 (2026-07-25) proved the new "Validate CI chat and bot membership" step's root-cause diagnosis correct: 10 of 11 real-Telegram tests failed because TelegramMembershipService.IsMember got "chat not found" from getChatMember for the CI account against COMMUNITY_CHAT_ID on every single update (bot log: "Failed to check membership ... chat not found"), so every command was silently refused. Two follow-up fixes from reviewing that step and that run's logs: - The step's echo included getChat's .result.id, which for a real group/supergroup is the same value as the COUPON_CI_CHAT_ID secret (or its supergroup-migrated form) — printing it leaked the secret into the run log. Removed; ok/type/title carry the same diagnostic value. - "Seed bot_setting" used ON CONFLICT (key) DO NOTHING for all 8 rows, but migration V16__pending_add_batch.sql already seeds BATCH_DEBOUNCE_MS=5000 with its own ON CONFLICT DO NOTHING — so on a freshly-migrated DB the CI's intended 1000ms override always lost silently, leaving the real debounce at 5000ms (the run's INSERT reported "INSERT 0 7" against 8 VALUES rows, confirming exactly one row was skipped). Changed to DO UPDATE SET value = EXCLUDED.value so the CI's values are authoritative regardless of what a migration pre-seeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU
Run 30163952072 (2026-07-25) showed used/return real tests failing with
"Expected a used:6/return:7 button on the take confirmation" even though
the take succeeded. Root cause: BotHelpers.singleTakenKeyboard (the
take-confirmation card's keyboard, BotHelpers.fs:290-294) emits
"used:<id>:del" / "return:<id>:del" — CallbackHandler.fs's EndsWith(":del")
handling strips that suffix and deletes the confirmation message on
success — but these two tests searched for the bare "used:<id>"/
"return:<id>" (no suffix), an exact-match predicate that can never match
production's real button data. The /my listing's separate keyboard
(CommandHandler.fs:247-248) genuinely has no suffix, so the "my" test
above was already correct and is untouched. Verified against
CallbackHandler.fs's parsing (:del stripped via EndsWith/Substring) that
the corrected value presses a real, valid button — not weakening either
assertion, just correcting the expected string to match unchanged src
behavior. `dotnet build tests/CouponHubBot.RealTests -c Release` passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU
…using DATA_INVALID TL.Methods.Messages_GetBotCallbackAnswer is a raw TL request type whose `data` field is only serialized when the `flags` bitfield has its has_data bit set (WriteTL: `if ((flags & Flags.has_data) != 0) writer.WriteTLBytes(data)`). Constructing it via F# named-argument/object-initializer syntax sets the `data` property but never touches `flags`, which defaults to 0 — every press went out with msg_id but zero callback bytes attached, which Telegram rejects with DATA_INVALID on every single press (4/4 reproducible). Decompiled via ilspycmd to confirm both the raw request's field layout and the WTelegram-generated `Client.Messages_GetBotCallbackAnswer` extension method, which correctly computes `flags` from which optional args are non-null. Switched PressCallbackButton to that extension method. Verified against real DM history with @coupon_test_bot (no bot deployed, no workflow dispatched): the broken raw-request shape reproduced DATA_INVALID locally; the extension-method shape (now in PressCallbackButton) was accepted by Telegram and instead got BOT_RESPONSE_TIMEOUT, since no bot is running to answer it — RPC acceptance is the fix's success criterion here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU
…oding null Manual /add (photo + explicit caption) previously passed a literal null barcode to TryAddCoupon, exempting those coupons from the coupon_barcode_active_uniq duplicate check. HandleAddManual now runs OCR on the photo (gated on OCR_ENABLED, reusing the wizard's GetFile/download approach) and passes only the extracted barcode through — value, min-check and expiry always come from the caption. OCR failure (backend error, timeout, missing file path, oversized image) degrades to a null barcode exactly as before; adding a coupon never fails because OCR is unavailable. The existing AddCouponResult.DuplicateBarcode handling in HandleAddManual was already correct and unused (barcode was always null) — no change needed there, it just becomes reachable now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU
…iry items, not barcode dupes The three album fixtures already have distinct barcodes (2706688198821/838/845, confirmed both by reading their raw Azure OCR text and by the passing CouponHubBot.Ocr.Tests theory cases for these exact filenames), so the duplicate-barcode hypothesis is refuted. Real cause: unlike the single-photo /add flow, the album/batch flow has no caption to override OCR's own printed (past) expiry, and DbService.TryAddCoupon rejects `expires_at < todayUtc()` for every batch item during BulkBatchConfirm (DbService.fs:198-200) — same gate the manual /add path already works around via an explicit future-date caption. Since every fixture in Images/ is deliberately dated in the past (repo-public constraint), all 3 album items were silently skipped as "Expired", landing 0 net coupons. Test-only fix: after OCR lands each item as 'ok', bump their expires_at via direct SQL to a future date before the bulk-confirm button is pressed — the same "direct SQL, real-test-only" override pattern ReminderRealTests.fs and AddFlowRealTests' caption already use. Leaves value/min_check/barcode_text (the actual OCR output under test) untouched; assertion (3 coupons added) unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU
Szer
added a commit
that referenced
this pull request
Jul 26, 2026
…vered (#272) * Real-tests expansion: shared plumbing + 4 stub files for parallel authors Adds DB/Telegram/cross-cutting helpers to CouponHubBot.RealTests so four agents can each author one expansion test file without touching a shared file: - DbSeed.fs: deleteCouponsByBarcodeAsync, deletePendingAddFlowAsync, deletePendingBatchesAsync, getCouponStatusAsync, getLatestOwnedCouponIdAsync, setCouponExpiryAsync. - TgUserClient.fs: PressCallbackButtonMatching (self-explanatory button-not-found errors) and AwaitAndPressButton, kept assertion-class vs timeout-class distinct so TestRetry's single-retry-on-AwaitTimeoutException semantics are preserved. - RealTestHelpers.fs (new): fixtureBarcodes map (read off each OCR fixture's own filename) and addCouponViaCaptionAsync, the retry-safe/fixture-reusable /add helper that closes the coupon_barcode_active_uniq flake introduced by merge #269. - Four compiling stub files (AddWizardRealTests, ReportButtonFlowRealTests, MyAndAddedRealTests, AdminAndFeedbackRealTests) registered in the .fsproj, each with one placeholder /help-round-trip [<Fact>] for the four authors to replace. - README.md: authoring guide (skeleton, helper inventory, retry contract, fixture reuse rule, CI-dispatch-only rule). No production code changes; existing 11 real tests left untouched (see report for why the DB-helper substitution was judged more than mechanical and skipped). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU * Real-Telegram tests: /added + void, myAdded, bare used/return, text /used /return /void Adds 7 happy-path tests to MyAndAddedRealTests.fs covering the slice not already exercised by CouponLifecycleRealTests.fs: /added listing + its bare void:<id> button, the myAdded button (renders the same as /added), the bare used:<id>/return:<id> buttons under /my (distinct from the :del-suffixed take-confirmation-card buttons), and the text commands /used, /return, /void. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU * Real tests: cover the /report button chain and reportedUsed acknowledgement Adds ReportButtonFlowRealTests.fs (real-Telegram happy paths, this agent's exclusive file): /my -> report -> report:<id> -> report:<id>:confirm marks a coupon reported; reportCancel at both the select and confirm steps leaves the coupon untouched; and the owner-side reportedUsed:<id> button marks a 'reported' coupon 'used'. Complements ReportFlowRealTests.fs's TEXT /report coverage with the button-driven chain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU * Real-tests: /feedback, admin /debug + /undo, and short-alias coverage Replaces AdminAndFeedbackRealTests.fs's placeholder with four real tests: - /feedback happy path: arm the flag, send a uniquely-marked message, assert the thank-you reply, the admin forward (lands in the same DM since FEEDBACK_ADMINS is seeded with the test account's own id), and the DB row via a private user_feedback query (no DbSeed.fs helper exists for it). - /debug <id>: add + take a coupon, assert its event-history dump contains both event types. - /undo <id>: add + take a coupon, undo, assert the confirmation text and the DB rollback (taken -> available, taken_by cleared) via DbSeed.getCouponStatusAsync. - Short aliases (/l, /coupons, bare /take, /m, /ad, /s, /a, /f) against CommandHandler.fs's Dispatch, with explicit cleanup of the two stateful aliases (/a's add-wizard row, /f's pending-feedback flag) in a finally block so this test can't corrupt a later test class in the same serial run. No GitHub issue creation is exercised or asserted on (GITHUB_TOKEN is deliberately absent from the CI test pod, gating GitHubService.IsConfigured false). No other file touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU * Real-tests: add /add wizard + bulk-cancel happy-path coverage AddWizardRealTests.fs: full wizard happy path (ocr:no -> disc:10:50 -> date:today -> confirm, DB-verified), date:tomorrow confirm-screen check, addflow:cancel, the other three discount buttons (cheap: confirm-screen assert + cancel, no DB writes), and addflow:bulk:cancel for an album. addflow:ocr:yes is left untested — unreachable as a happy path for these (deliberately expired) fixtures, since TryAddCoupon rejects any expires_at < today before ocr:yes's direct-add path can succeed (DbService.fs:198-200); addflow:ocr:no explicitly clears the OCR-derived expiry so every other route reaches a genuine, DB-verified happy path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU * Real-tests expansion: fix cross-test state leak, async cleanup, clock-dependent assertion, add ocr:yes coverage, raise CI timeout Review fixes on top of the four merged real-test branches: - AddWizardRealTests.fs: date:tomorrow no longer leaves a pending_add row for another test to sweep; rewritten to drive both date:today/date:tomorrow to addflow:confirm and assert the DB expiries are exactly one day apart, independent of the test process's own clock. - AddWizardRealTests.fs: added the previously-untestable addflow:ocr:yes happy path, reachable by bumping the pending_add row's own expires_at via direct SQL (documented why this must never be "cleaned up" — fixtures must stay expired since the repo is public). - AdminAndFeedbackRealTests.fs: alias test's pending-feedback cleanup no longer blocks on .GetAwaiter().GetResult() from inside a task body; replaced with a DB-level DELETE FROM pending_feedback + try/with re-raise so the flag is disarmed on every exit path without a blocking wait. - .github/workflows/coupon-real-test.yml: timeout-minutes 30 -> 50 for the ~21 additional real tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU * Fix 5 real-Telegram CI failures from run 30181924500 Root-caused each from bot pod logs + CI test-runner logs, not guessed: 1. ReportFlowRealTests "report command..." — hand-rolled, non-deleting /add collided with a leftover VOIDED row for the same fixture barcode left by MyAndAddedRealTests (which ran first), because DbService.TryAddCoupon's primary duplicate-barcode check doesn't filter by status (production bug, reported separately, not fixed here). Switched to the idempotent RealTestHelpers.addCouponViaCaptionAsync helper. 2. AdminAndFeedbackRealTests "short aliases..." — its /m step crashed handleMy's sendMediaGroupPhotos on a coupon with a synthetic, non-Telegram photo_file_id that ReminderRealTests inserts directly via SQL and never cleaned up (unbarcode-tied, so deleteCouponsByBarcodeAsync could never find it). Confirmed via the two "wrong remote file identifier ... Wrong padding" bot-log errors landing exactly on this test's two TestRetry attempts. Fixed by deleting the coupon after ReminderRealTests' assertion. 3. AdminAndFeedbackRealTests "undo rewinds..." — its own /take hit DbService.TryTakeCoupon's LimitReached (MAX_TAKEN_COUPONS=6) because several earlier tests leave a coupon 'taken' with no further use and never release it. Added DbSeed.releaseTakenCouponAsync and called it after each such test's own assertions (CouponLifecycleRealTests x2, ReportButtonFlowRealTests x2, MyAndAddedRealTests x1), plus the ReminderRealTests cleanup from (2), eliminating the permanent leaks. 4. AddWizardRealTests "addflow:bulk:cancel..." — awaited a NEW Telegram message for the cancel confirmation, but BulkBatchCancel delivers it via EditBulkOrSend, which EDITS the existing message in place; TgUserClient's `record` explicitly drops all edits, so the await could never succeed by construction. Switched to the file's own waitForBatchCleared DB poll, matching BulkAddRealTests' sibling confirm test's existing pattern. 5. AddWizardRealTests "date:today and date:tomorrow..." — QuerySingleAsync against a bare DateOnly scalar silently returned default(DateOnly) (Dapper doesn't treat DateOnly as a scalar-mappable type). Mapped into CouponRow instead, matching the working pattern used elsewhere in the same file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU * Fix real-Telegram test suite: make every coupon-add idempotent regardless of order CI run 30193635437 (28/32) failed 4 tests via duplicate-barcode rejection because AddFlowRealTests, CouponLifecycleRealTests's local addCoupon helper, and BulkAddRealTests's album confirm all sent /add without deleting any pre-existing row for their fixture's barcode first. The previous pass's "the later user always deletes first" reasoning is wrong: test class order is not guaranteed, so whichever add runs second collides. Route CouponLifecycleRealTests through the shared RealTestHelpers.addCouponViaCaptionAsync helper, delete-by-barcode-first in AddFlowRealTests (kept inline — it tests the manual-add path itself) and BulkAddRealTests (all 3 album fixtures). Also close a taken-coupon leak the prior MAX_TAKEN_COUPONS audit missed: AdminAndFeedbackRealTests's "debug shows..." test took a coupon and never released it. Document the idempotence rule prominently in README.md so a future test author can't reintroduce this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Gets the real-Telegram (MTProto) test suite from red to 11/11 green, and closes a
duplicate-detection gap in manual
/addalong the way.Real-run evidence: https://github.com/Szer/bots/actions/runs/30179358723 —
Passed: 11.Hermetic suite: 192/192 (188 baseline + 4 new).
Production change — user-visible
Manual
/add(photo + caption) now extracts the coupon barcode via OCR instead of storingnull(CouponFlowHandler.fs:106-132,:160-161).Why it matters:
coupon_barcode_active_uniqis a partial unique index on(barcode_text, expires_at), so a coupon with anullbarcode was exempt from duplicatedetection. Every manual-add coupon bypassed dedupe entirely. Production is 1459/1489 (98%)
barcoded because real users mostly use the wizard/album flows, which already ran OCR.
Consequence to expect: adding a duplicate coupon by caption will now be rejected where it
previously silently succeeded. The user-facing message for this already existed
(
AddCouponResult.DuplicateBarcode→ «Купон с таким штрихкодом уже есть в базе…») but wasunreachable dead code; it is now live.
Safety properties, each with a test:
OCR_ENABLED, like the other two OCR paths.still created with a
nullbarcode. Adding a coupon must not depend on a third-party visionservice.
Four real bugs the pipeline found
None were catchable by the hermetic suite, which passed throughout.
COUPON_CI_BOT_TOKENwas set but empty (len=0) — surfaced as a404fromapi.telegram.org, which reads as "webhook missing" rather than "token rejected".getChatMemberthrowchat not found, whichMembershipServicecollapses intofalse— so thebot told a genuine member «Бот доступен только членам сообщества».
PressCallbackButtonnever sent its callback data. ConstructingTL.Methods.Messages_GetBotCallbackAnswervia F# object-initializer syntax setsdatabut notthe request's
flagsbitfield, andWriteTLonly serializesdatawhenhas_data(0x1) isset — so every press went out with a valid
msg_idand zero bytes, and Telegram answeredDATA_INVALID. Fixed by using theSchemaExtensionshelper, which computesflags.This code path had never been executed against real Telegram by anything in this repo.
TryAddCouponrejectsexpires_at < today. Manual/addescapes this via the caption's explicit date; the album pathreads no caption, so all three items were silently skipped and the bulk confirm added zero
coupons.
Diagnostics added, because the first failures were unreadable
getMe, thengetChat/getChatMemberCount. The first bad-token run burned five minutes to produce amisleading error; it now fails in ~12 seconds naming the cause and the fix.
if: always(), so it had alreadydeleted the pod whose logs the error message told you to read.
404/ok=falsepoints at the token;200with awrong url points at
WEBHOOK_URLand the pod logs.bot_settingseed usedON CONFLICT DO NOTHING, so migration-seeded valuessilently won and CI's intended
BATCH_DEBOUNCE_MS=1000was discarded in favour of5000(
INSERT 0 7for 8 rows). NowDO UPDATE.getChat's returnedidequals theCOUPON_CI_CHAT_IDsecret, so echoing it leakeda secret into the run log. Removed from the echo.
Known limits
/mytiming outat 90s.
TestRetry's single timeout-class retry is not always enough. Expect occasional red fromreal-Telegram latency rather than from regressions.
BulkAddRealTestsoverridesexpires_atvia direct SQL after OCR lands the items, becausefixtures must stay expired (this repo is public — a current coupon image would publish a live
usable barcode) and the album path has no caption to carry a future date. That test therefore does
not exercise the batch path's expiry validation.
/reportis covered only as a text command. The «pick-list → confirm button flow — the path real users take — has no real-Telegram coverage yet.
How to run
workflow_dispatchonly, never on the PR gate.🤖 Generated with Claude Code
https://claude.ai/code/session_01P41roCWN5Fec7m69sSaunU