fix(participation): detect deleted topics by posting into them; recreate only on thread-gone

openspec/changes/hackathon-participation/apply-progress.md

@@ -134,3 +134,28 @@ Mode: Strict TDD. Base: main 934fd01 (PR1a and PR1b merged). Completed: Step 0 (
 - The `setGeneralMessageId` failure is logged as `hackathon-job` / `general-message-id-store-failed`. Consequence: the stored message's button is not cleared on join (the tapped message's own button still is).
 - The persisted repost site also stores the message id, so a repost after a crash replaces the stored id with the newest message.
 - The only new e2e coverage is the `hp:` callback; other prefixes were already ignored (only `sel:` and `hp:` handlers exist), so no routing code changed in `index.ts`.
+
+## Fix: existing-topic check by posting (branch `fix/participation-topic-check`)
+
+Production evidence: after the user deleted topic 262, `/hackathon join` still replied "Este hackathon ya tiene tema" (`hackathon-join outcome ok`). `ForumTopicManager.probe` used `sendChatAction`, and Telegram accepts a chat action for a deleted thread, so the check never detected the deletion.
+
+- `participateInHackathon` now verifies a linked topic by posting the analysis into it through `postAnalysisAndLinkTopic` (which also re-pins and refreshes `pinnedMessageId`).
+  - Post succeeds: the topic is live, reply `already(link)`.
+  - `PublishFailedError` with `failureClass === "rejected"`: the topic is deleted, then the existing claim, create, link, post/pin, confirm path runs.
+  - Any other failure (`telegram-unavailable`, `rate-limited`, unexpected errors): unknown, logged as `topic-check-failed`, reply `already(link)`, never recreate.
+- `probe`, `TopicProbe`, the adapter probe and its classifier, the fake probe and the adapter probe tests were removed (nothing else used them). The e2e `sendChatAction` stubs were dropped.
+- TDD: RED first (4 failing tests: live post, deleted via rejected post, unavailable and rate-limited without recreate, redelivery after recreation), then GREEN. Focused file 28/28, `npm run typecheck` clean, full suite 76 files, 1018/1018 (was 1027; the 10 probe tests were removed, 1 net new use-case test).
+- Known limit: a live re-check re-posts the analysis and re-pins it; the previous pinned message stays pinned (Telegram allows several pins).
+- Commits: `3e648d6` fix(participation) code and tests, `65c2fac` docs (design decision 2, spec scenario, proposal).
+
+## Fix: recreate only on a thread-gone post (review R3-001..R3-004)
+
+Review found that treating any `rejected` post as "deleted" would recreate a LIVE topic that answers 400 TOPIC_CLOSED or 403, creating a duplicate and moving the link.
+
+- New `PublishFailureClass = AlertSendFailureClass | "thread-gone"` in `errors.ts`; `PublishFailedError` carries it. `AlertSendFailureClass` and `classifyTelegramFailure` are unchanged, so the alert sender behaves as before.
+- New `classifyPublishFailure` in `send-failure.ts`: a 400 whose description matches `/message thread not found|topic_id_invalid|topic_deleted/i` is `thread-gone`; everything else defers to `classifyTelegramFailure`. Only `ChatPublisher.post` uses it (pin, unpin and clearButtons keep `rejected`).
+- `participateInHackathon` recreates only on `thread-gone`. `rejected`, `unavailable`, `rate-limited` and unexpected errors log `topic-check-failed` and return `already(link)`.
+- `run-hackathon-job` treats `thread-gone` like `rejected` (`publish:rejected`, permanent), so a post to a deleted thread in the job path is not retried.
+- TDD: RED first (7 failing: 3 adapter thread-gone cases, the use-case rejected-no-recreate case and 3 recreate tests moved to thread-gone), then GREEN. Added adapter cases (400 thread not found, TOPIC_ID_INVALID, TOPIC_DELETED to thread-gone; TOPIC_CLOSED, 403 and a 403 with thread wording to rejected; pin stays rejected) and use-case cases (rejected, unexpected TypeError, with the topic-check-failed log asserted).
+- R3-003 and R3-004: proposal, design and spec now describe the post-based check and its residual risk (a different Telegram description would not recreate: the safe side, checked by the smoke test); the `claimTopicCreation` comment no longer mentions probe.
+- Full suite 76 files, 1027/1027; `npm run typecheck` clean.

openspec/changes/hackathon-participation/design.md

@@ -6,14 +6,14 @@ Hexagonal, same as change 3. There is one pure use case, `participateInHackathon
 - the `hp:<slug>` callback on the General analysis post;
 - `/hackathon join <slug>`.
 
-The use case owns five things: the admin gate, the slug lookup, the live/deleted probe, a D1 compare-and-set claim, and topic creation. It then links through the existing `moveTopicLink` / `postAnalysisAndLinkTopic` flow and removes the button. Telegram access sits behind a new `ForumTopicManager` port plus two small `ChatPublisher` additions. The domain never imports grammY. The consumer (`runHackathonJob`) attaches the button to its two General post sites and stores the message id. Specs: `specs/hackathon-participation`, plus the deltas for `hackathon-analysis` and `telegram-webhook`.
+The use case owns five things: the admin gate, the slug lookup, the existing-topic check (a real post), a D1 compare-and-set claim, and topic creation. It then links through the existing `moveTopicLink` / `postAnalysisAndLinkTopic` flow and removes the button. Telegram access sits behind a new `ForumTopicManager` port plus two small `ChatPublisher` additions. The domain never imports grammY. The consumer (`runHackathonJob`) attaches the button to its two General post sites and stores the message id. Specs: `specs/hackathon-participation`, plus the deltas for `hackathon-analysis` and `telegram-webhook`.
 
 ## Architecture Decisions
 
 | # | Topic | Choice | Rejected (tradeoff) |
 |---|---|---|---|
-| 1 | Ports | New ISP port `ForumTopicManager { create, probe }`. `ChatPublisher.post` gains an optional `PostOptions { participateSlug?: string }`, which is semantic, not a keyboard. It also gains `clearButtons(chatId, messageId)`. The adapter owns the label, the `hp:` encoding and `editMessageReplyMarkup` (omitting `reply_markup` removes the keyboard). | Extending `ChatPublisher` with topic methods: the consumer would see topic rights it never needs. A domain-built `{label,data}` keyboard would leak the Telegram payload format into the domain. |
-| 2 | Deleted-topic probe | `sendChatAction(chat, "typing", {message_thread_id})`. **ok ⇒ `live`** (a positive signal). A 400 whose description matches `/message thread not found\|TOPIC_ID_INVALID\|TOPIC_DELETED/i` ⇒ `deleted`. **Anything else** (other 400s, 403, 429, 5xx, timeout) ⇒ `unknown`, treated as live: never recreate on ambiguity. It needs no `can_manage_topics`. Its side effect is a "typing…" indicator of at most 5 s. | `editForumTopic` with no changes: needs `can_manage_topics` unless the bot created the topic, and a live topic also fails (`TOPIC_NOT_MODIFIED`), so live vs deleted hinges on two error strings. `closeForumTopic`/`reopenForumTopic`: they change state, post a service message, and need `can_manage_topics`. Telegram does not document its error descriptions (they come from community reports), so validation is an operator smoke step. The port hides the mechanism, so switching it is adapter-only. |
+| 1 | Ports | New ISP port `ForumTopicManager { create }`. `ChatPublisher.post` gains an optional `PostOptions { participateSlug?: string }`, which is semantic, not a keyboard. It also gains `clearButtons(chatId, messageId)`. The adapter owns the label, the `hp:` encoding and `editMessageReplyMarkup` (omitting `reply_markup` removes the keyboard). | Extending `ChatPublisher` with topic methods: the consumer would see topic rights it never needs. A domain-built `{label,data}` keyboard would leak the Telegram payload format into the domain. |
+| 2 | Existing-topic check | Do the real action: `postAnalysisAndLinkTopic` posts (and re-pins) the analysis into the linked thread. **Post succeeds ⇒ the topic is live**: reply `already(link)`; the analysis is re-posted there. A `PublishFailedError` classified `thread-gone` (the adapter assigns it only to a 400 whose description matches `message thread not found`, `TOPIC_ID_INVALID` or `TOPIC_DELETED`) ⇒ the topic is deleted: `expected = threadId`, then claim → create → link → post/pin → confirm. `rejected` (any other 4xx: a closed topic or missing rights on a live topic), `telegram-unavailable` and `rate-limited` (5xx, network, 429) ⇒ unknown: never recreate on ambiguity, reply `already(link)`. | `sendChatAction(chat, "typing", {message_thread_id})`: production showed Telegram accepts a chat action for a deleted thread, so it never detected the deletion. `editForumTopic`, `closeForumTopic`/`reopenForumTopic`: they need `can_manage_topics` and change topic state. Treating every `rejected` as deleted: a live topic can return 400 TOPIC_CLOSED or 403, so it would create a duplicate topic. The `PublishFailedError.failureClass` carries the classification, so grammY and description matching stay in the adapter and the domain stays free of Telegram error strings. |
 | 3 | Concurrency and redelivery | **(a) Migration 0004 with a claim.** `topic_claim_until` is set by a conditional `UPDATE … WHERE team_id=? AND id=? AND thread_id IS ?expected AND topic_claim_until <= ?now` (CAS on the observed `thread_id`: null, or the stale id). TTL is 60 s. | (b) Accepting the race. Webhook `max_connections` defaults to 40, so parallel taps are real, and the cost (a duplicate public topic) is visible and manual to undo. |
 | 4 | Button and message id | Migration 0004 adds `general_message_id`. A helper `postToGeneral` in `run-hackathon-job` (used by both General sites) posts with `participateSlug`. It then calls `setGeneralMessageId`, best-effort: catch and log, never retry, because a retry would repost. Each repost overwrites the id; old buttons stay idempotent. `/hackathon join` clears the stored id's button. Old analyses have null, so nothing is cleared. | Not storing the id: `join` could never remove the button. |
 | 5 | Callback handling | `bot.callbackQuery(/^hp:([a-z0-9]+(?:-[a-z0-9]+)*)$/)`. A private chat or a missing chat is ignored. The new `callbackCallerLocation(ctx)` reads `ctx.chat.id`, `ctx.from.id`, `ctx.msg?.message_thread_id` and `ctx.callbackQuery.message?.message_id`. The team comes from `teamRepo.findByChatId` and the role from `ctx.from.id` (`resolveGroupMembership`). Non-members and non-admins get the same alert (`show_alert`). The handler answers the callback early and best-effort. | Changing `callerLocation` to `ctx.msg`: it would widen every command to edited or channel messages. Trusting a team id in the payload is ruled out by the spec. |
@@ -24,7 +24,7 @@ The use case owns five things: the admin gate, the slug lookup, the live/deleted
 ## Use Case Step Order (`participateInHackathon`)
 
 1. Membership role check: not an admin ⇒ `UnauthorizedError`. `findBySlug` finds nothing ⇒ `AnalysisNotFoundError`.
-2. `threadId != null` ⇒ `probe`. On `live` or `unknown`: clear the buttons (best-effort) and return `already(link)`. On `deleted`: set `expected = threadId`.
+2. `threadId != null` ⇒ post the analysis into that thread. On success or an unknown failure (`rejected`, `telegram-unavailable`, `rate-limited`, unexpected errors; logged as `topic-check-failed`): clear the buttons (best-effort) and return `already(link)`. On a `thread-gone` post: set `expected = threadId`.
 3. `claimTopicCreation(expected, now, 60s)`. If it returns false, re-read the analysis. Linked ⇒ `already(link)`. Otherwise `busy`, a neutral no-op: the callback is answered silently and nothing is posted.
 4. `create(chat, topicNameFor(analysis))` can fail as follows:
 
@@ -45,19 +45,17 @@ The use case owns five things: the admin gate, the slug lookup, the live/deleted
 tap hp:<slug> ─ callbackCallerLocation ─ member+admin? ─no→ alert
 /hackathon join <slug> ─ parseJoinArgument ─┐       │yes
                                             └─ participateInHackathon
-   probe ─ claim(CAS) ─ create ─ moveTopicLink ─ post+pin (existing) ─ clearButtons
+   post-check ─ claim(CAS) ─ create ─ moveTopicLink ─ post+pin (existing) ─ clearButtons
    └→ replyText ─ safe post to General (threadId null)
 consumer: postToGeneral(text, participateSlug) ─ setGeneralMessageId (best-effort)
 ```
 
 ## Interfaces / Contracts
 
 ```ts
-type TopicProbe = "live" | "deleted" | "unknown";            // probe never throws
 type TopicCreateFailure = "no-rights" | "not-forum" | "rate-limited" | "rejected" | "unavailable";
 interface ForumTopicManager {
   create(chatId: number, name: string): Promise<number>;      // throws ForumTopicCreateError(failure)
-  probe(chatId: number, threadId: number): Promise<TopicProbe>;
 }
 interface ChatPublisher { post(chatId, threadId, text, options?: { participateSlug?: string }): Promise<number>;
   pin; unpin; clearButtons(chatId: number, messageId: number): Promise<void>; }
@@ -112,9 +110,9 @@ or record `size:exception`.
 | Layer | Cases | Approach |
 |---|---|---|
 | Pure | `topicNameFor` (control and bidi characters, whitespace, empty ⇒ slug, 128-unit cap, no split surrogate); `topicLink` (`-100` stripping, non-`-100` ⇒ null); `parseJoinArgument` (bare, non-slug, three tokens) | Vitest |
-| Use case | Non-admin or non-member changes nothing. A live topic gets `already` and nothing is created. An `unknown` probe is treated as live. A **deleted** topic is recreated and the stale id replaced. **Redelivery:** a second call after success returns `already`. **Concurrent taps:** a claim loss followed by a re-read gives `already` or `busy`, with exactly one `create`. Missing rights or not a forum: the claim is released and nothing is linked. `unavailable` keeps the claim. **Link failure / post failure after creation**: never throws. **Pin failure** adds a note. A button-clear failure is ignored. | `FakeForumTopicManager` (scripted outcomes, call log), plus the fake publisher and repo |
+| Use case | Non-admin or non-member changes nothing. A live topic (the post succeeds) gets `already` and nothing is created. A rejected (closed topic, no rights), unavailable or rate-limited post, or an unexpected error, is treated as unknown: no recreate, logged as `topic-check-failed`. A **deleted** topic (the post is `thread-gone`) is recreated and the stale id replaced. **Redelivery:** a second call after success returns `already`. **Concurrent taps:** a claim loss followed by a re-read gives `already` or `busy`, with exactly one `create`. Missing rights or not a forum: the claim is released and nothing is linked. `unavailable` keeps the claim. **Link failure / post failure after creation**: never throws. **Pin failure** adds a note. A button-clear failure is ignored. | `FakeForumTopicManager` (scripted outcomes, call log), plus the fake publisher and repo |
 | D1 | Claim CAS on `IS NULL` and `IS stale`, TTL expiry, release, `setGeneralMessageId`; `save`/`persistAnalysis` leave the new columns alone | vitest-pool-workers |
-| Adapters | Create-error classification table; probe mapping (ok, deleted strings, other ⇒ unknown); `clearButtons` sends `editMessageReplyMarkup` without markup; the keyboard carries `hp:<slug>` of at most 64 bytes | Injected `Api` stub |
+| Adapters | Create-error classification table; `clearButtons` sends `editMessageReplyMarkup` without markup; the keyboard carries `hp:<slug>` of at most 64 bytes | Injected `Api` stub |
 | Handlers | The `hp:` alert for non-admins; a private chat is ignored; the join usage; a failing safe General post never produces a 500; the consumer General post has the button, a topic post has none, and a `setGeneralMessageId` failure still acks | `telegram-stub.ts`, callback fixtures from `commands.test.ts` |
 
 No fetch, LLM or validation path changes, so `npm run harness` is not required.
@@ -132,12 +130,12 @@ The shell, VCS and PR rows are N/A: this change has no shell, subprocess, VCS or
 Both are additive. Operator steps:
 1. Apply 0004 remotely, then deploy.
 2. Grant "Administrar temas".
-3. Smoke test: tap the button; delete the topic and tap again (validates the probe strings and the link format); run a join on an old analysis.
+3. Smoke test: tap the button; delete the topic and tap again (validates that a thread-gone post recreates the topic and that the description matches, and the link format); run a join on an old analysis.
 
 Rollback: redeploy the previous Worker. The columns are ignored.
 
 ## Open Questions
 
 - [ ] Can the spec adopt the three design-proposed strings (`createFailed`, `createUncertain`, `linkFailed`)?
-- [ ] Probe error strings and the `t.me/c` topic link are unverified against production (smoke step 3).
+- [ ] The `t.me/c` topic link and the thread-gone recreation (the exact Telegram description) are unverified against production (smoke step 3).
 - [ ] Known race: a refresh job whose `persistAnalysis` upsert runs during participation can overwrite `thread_id` with its stale value. This is pre-existing behavior; recovery is `/hackathon <slug>` in the topic.

openspec/changes/hackathon-participation/proposal.md

@@ -17,7 +17,8 @@ After a General-chat analysis, the team decides whether to join. Today an admin
   - the result stays within 1–128 characters.
 - Idempotency:
   - if the analysis has a live topic, create nothing and reply with the topic link;
-  - if the link is stale (the topic was deleted), a lightweight probe detects it, the stale link is dropped, and the topic is recreated.
+  - if the link is stale (the topic was deleted), posting the analysis into it detects it (only a 400 that says the thread does not exist counts), the stale link is dropped, and the topic is recreated;
+  - any other failure of that post (closed topic, missing rights, 5xx, 429) is inconclusive: nothing is recreated and the link is kept.
 - On success:
   - post "✅ Participating in <name> → <link>" in General;
   - remove the button from the original message;
@@ -50,7 +51,7 @@ After a General-chat analysis, the team decides whether to join. Today an admin
 ## Approach
 
 This follows the exploration's recommendation, D+A.
-- A new ISP port, `ForumTopicManager`, provides create and probe, with rejections classified.
+- A new ISP port, `ForumTopicManager`, provides create, with rejections classified. The existing-topic check reuses `ChatPublisher.post`, whose adapter reports a distinct `thread-gone` failure class.
 - `ChatPublisher.post` gets an optional inline keyboard, plus a way to remove it.
 - `callerLocation` gets a `callback_query` variant.
 - Once the topic exists, link it immediately and never rethrow. Partial failures reply with a recovery hint.
@@ -75,7 +76,8 @@ Fetch, LLM and validation are untouched, so `npm run harness` is not required.
 | Redelivery creates a second topic | Med | Link right after creating; no rethrow after that |
 | Concurrent taps | Low | Claim migration, or accept the race (design) |
 | Bot lacks topic rights | Med | Operator reply; nothing persisted |
-| The probe misreads a live topic as deleted | Low | Probe mechanism validated in the design |
+| The check recreates a live topic | Low | Only a 400 whose description matches "message thread not found", `TOPIC_ID_INVALID` or `TOPIC_DELETED` is `thread-gone`; a closed topic or missing rights stay `rejected` and never recreate |
+| A deleted topic is not recreated because Telegram words the error differently | Low | Safe side: the link is kept and `already` is returned; the Telegram smoke test verifies the wording |
 
 ## Rollback Plan
 
@@ -104,7 +106,7 @@ About 350–500 lines, delivered as 2 chained PRs:
 ## Open questions (for design)
 
 1. A claim migration versus accepting the concurrent-tap race.
-2. The mechanism for the stale-topic probe.
+2. The mechanism for the stale-topic check.
 3. How the button is removed after a `/hackathon join`, when the General message id is unknown.
 
 ## Proposal question round