Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion openspec/changes/hackathon-participation/apply-progress.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Apply Progress: hackathon-participation

Completed so far: Phase 1 (1.1-1.12) and Phase 2 (2.1-2.11). Phase 3 (PR2) and Phase 4 pending.
Completed so far: Phase 1 (1.1-1.12), Phase 2 (2.1-2.11) and Phase 3 (3.1-3.10). Phase 4 (operator steps and final verification) pending.

## Batch 1 — Phase 1 Infrastructure (PR1a) — branch `feat/participation-infra`

Expand Down Expand Up @@ -89,3 +89,48 @@ Mode: Strict TDD. Base: main bfec91d (PR1a merged). Completed: Step 0 (PR1a revi
- An unknown error from `create` (not a `ForumTopicCreateError`) keeps the claim and rethrows: the topic may exist.
- `ForumTopicManager` is wired in `composition.ts` (needed by join); the button, `hp:` handler and consumer wiring stay in Phase 3.
- Phase 3 must keep the `runParticipation` `reply` param for the callback alert.

## Batch 3 — Phase 3 Button, Callback, Consumer (PR2) — branch `feat/participation-button`

Mode: Strict TDD. Base: main 934fd01 (PR1a and PR1b merged). Completed: Step 0 (PR1b review warning R3-001) + 3.1–3.10 (all of Phase 3).

### Step 0 (PR1b advisory warning)

| Finding | Resolution |
|---|---|
| R3-001 no test for a non-`ForumTopicCreateError` thrown by `create` | Test added in `participate-in-hackathon.test.ts`: the claim is kept (expiry in the future), the same error object is rethrown, nothing is linked or posted. It passed on first run because the branch already existed (regression guard, not a true RED) |

### TDD Cycle Evidence

| Task | Test File | Layer | Safety Net | RED | GREEN | TRIANGULATE | REFACTOR |
|------|-----------|-------|------------|-----|-------|-------------|----------|
| 3.1/3.2 | `test/domain/usecases/run-hackathon-job.test.ts` | Unit (fakes) | 29/29 | 3 failed (no options, no stored id, no store-failure log) | 33/33 | fresh General, topic (no button, null id), store failure (ack, one post, log), persisted repost | Both General sites share `postToGeneral` |
| 3.3/3.4 | `test/adapters/telegram/commands.test.ts` | Unit | 120/120 | 3 failed (not a function) | pass | full ctx, absent thread/message id, missing chat/user | None needed |
| 3.5/3.6/3.7 | `test/adapters/telegram/commands.test.ts` (real grammY Bot + fakes) | Integration | 120/120 | 8 failed (no handler) | 131/131 | non-admin, non-member, admin, deduped/different buttons, early answer order, answer failure, redelivery, unknown slug, team from chat, 6 malformed payloads, private chat, `sel:` still works | None needed |
| 3.8 | `test/http/hackathon-command-e2e.test.ts` | Integration (real route, composition, D1) | 8/8 | 3 failed with the handler unregistered (verified by disabling it) | 11/11 | admin tap, non-admin alert, redelivery, malformed + foreign prefix ignored | None needed |
| 3.9 | composition | — | — | — | — | ➖ Nothing to wire: the consumer already had `chatPublisher` and `hackathonAnalysisRepo`; the callback rides `registerHackathonCommands` | ➖ |
| 3.10 | full suite | — | — | — | 1027/1027 (76 files), typecheck clean | — | — |

### Work Unit Evidence

| Evidence | Value |
|---|---|
| Focused test command and result | `npx vitest run test/domain/usecases/run-hackathon-job.test.ts test/adapters/telegram test/http`: all green; full suite 76 files, 1027/1027, `npm run typecheck` clean |
| Runtime harness | N/A: no fetch/LLM path; e2e drives the real Hono route, composition, D1 and adapters with stubbed Telegram HTTP |
| Rollback boundary | `postToGeneral` in `run-hackathon-job.ts`, `callbackCallerLocation` in `context.ts`, `registerParticipationCallback` and its one-line registration in `hackathon-commands.ts` |

### Commits
- `94d4ba1` test(participation): cover unknown create error keeping the claim and rethrowing
- `812929c` feat(hackathon): post the General analysis with the participation button and store its message id
- `fe92e95` feat(telegram): confirm participation from the hp callback button
- `74f8924` test(http): cover the hp callback through the webhook route
- `9d2cfa1` docs: tasks and apply-progress (this file)

### Deviations / notes
- Handler tests live in `commands.test.ts` (which already has the real-Bot harness with alert recording), not `participation.test.ts` as tasks 3.5/3.6 name it. The `callbackCallerLocation` tests are there too, as in 3.3.
- `callbackCallerLocation` returns `CallbackLocation = CallerLocation & { messageId }`, so the button message id travels with the location.
- Order in the handler: the role is resolved first (fast, no use case). A non-member or non-admin gets the single alert (one `answerCallbackQuery` per query allows only one). An admin is answered silently and early, then `runParticipation` runs. Its `reply` (pre-creation refusals such as missing rights) is therefore a best-effort `ctx.reply`, not an alert, because the query is already answered.
- Slug length is capped at 40 in the handler (slug.ts limit) on top of the design regex.
- 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`.
20 changes: 10 additions & 10 deletions openspec/changes/hackathon-participation/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,16 @@ Each PR is independently deployable and keeps `npm test` green. PR1a adds unused

## Phase 3: Button, Callback, Consumer (PR2)

- [ ] 3.1 RED: `test/domain/usecases/run-hackathon-job.test.ts` — General post is sent with `participateSlug` and `setGeneralMessageId` stores the returned id at both General sites; a topic post has no button; a `setGeneralMessageId` failure is logged, the job still acks, and there is no retry/repost.
- [ ] 3.2 GREEN: `src/domain/usecases/run-hackathon-job.ts` — `postToGeneral(text, participateSlug)` helper used by both General sites; best-effort id store.
- [ ] 3.3 RED: `test/adapters/telegram/commands.test.ts` — `callbackCallerLocation(ctx)` reads `ctx.chat.id`, `ctx.from.id`, `ctx.msg?.message_thread_id`, `ctx.callbackQuery.message?.message_id`; `callerLocation` behavior for commands unchanged.
- [ ] 3.4 GREEN: `src/adapters/telegram/context.ts` (`callbackCallerLocation`).
- [ ] 3.5 RED: `test/adapters/telegram/participation.test.ts` — **non-admin alert**: `hp:<slug>` from a non-admin (and non-member) → `answerCallbackQuery` with `show_alert` "Solo un administrador del equipo puede confirmar la participación.", nothing created, button stays; private/missing chat ignored; team taken from chat id, never the payload; malformed data (`hp:Bad_Slug`, over-long) ignored; admin tap creates the topic.
- [ ] 3.6 RED: same file — **button removed after confirmation**: admin tap → `clearButtons` for the callback message id and the stored `generalMessageId` (deduped); button-clear failure ignored; callback answered early, best-effort.
- [ ] 3.7 GREEN: `src/adapters/telegram/participation.ts` (`bot.callbackQuery(/^hp:(slug)$/)` handler, registered from `registerHackathonCommands`); `src/adapters/telegram/hackathon-commands.ts`.
- [ ] 3.8 RED: `test/http/webhook-e2e.test.ts` and `test/http/hackathon-command-e2e.test.ts` — validated webhook `hp:` callback from a group is routed to the participation handler; callback with any other prefix is ignored without error; `/hackathon join` clears the stored message's button (old analysis with null id: join works, nothing removed).
- [ ] 3.9 GREEN: `src/composition.ts` wiring (consumer uses `postToGeneral`, handler gets `ForumTopicManager`); `test/fakes/index.ts` and `test/support/telegram-stub.ts` callback fixtures/`answerCallbackQuery`/`editMessageReplyMarkup` recording.
- [ ] 3.10 Run `npm test` and `npm run typecheck`.
- [x] 3.1 RED: `test/domain/usecases/run-hackathon-job.test.ts` — General post is sent with `participateSlug` and `setGeneralMessageId` stores the returned id at both General sites; a topic post has no button; a `setGeneralMessageId` failure is logged, the job still acks, and there is no retry/repost.
- [x] 3.2 GREEN: `src/domain/usecases/run-hackathon-job.ts` — `postToGeneral(text, participateSlug)` helper used by both General sites; best-effort id store.
- [x] 3.3 RED: `test/adapters/telegram/commands.test.ts` — `callbackCallerLocation(ctx)` reads `ctx.chat.id`, `ctx.from.id`, `ctx.msg?.message_thread_id`, `ctx.callbackQuery.message?.message_id`; `callerLocation` behavior for commands unchanged.
- [x] 3.4 GREEN: `src/adapters/telegram/context.ts` (`callbackCallerLocation`).
- [x] 3.5 RED: `test/adapters/telegram/participation.test.ts` — **non-admin alert**: `hp:<slug>` from a non-admin (and non-member) → `answerCallbackQuery` with `show_alert` "Solo un administrador del equipo puede confirmar la participación.", nothing created, button stays; private/missing chat ignored; team taken from chat id, never the payload; malformed data (`hp:Bad_Slug`, over-long) ignored; admin tap creates the topic.
- [x] 3.6 RED: same file — **button removed after confirmation**: admin tap → `clearButtons` for the callback message id and the stored `generalMessageId` (deduped); button-clear failure ignored; callback answered early, best-effort.
- [x] 3.7 GREEN: `src/adapters/telegram/participation.ts` (`bot.callbackQuery(/^hp:(slug)$/)` handler, registered from `registerHackathonCommands`); `src/adapters/telegram/hackathon-commands.ts`.
- [x] 3.8 RED: `test/http/webhook-e2e.test.ts` and `test/http/hackathon-command-e2e.test.ts` — validated webhook `hp:` callback from a group is routed to the participation handler; callback with any other prefix is ignored without error; `/hackathon join` clears the stored message's button (old analysis with null id: join works, nothing removed).
- [x] 3.9 GREEN: `src/composition.ts` wiring (consumer uses `postToGeneral`, handler gets `ForumTopicManager`); `test/fakes/index.ts` and `test/support/telegram-stub.ts` callback fixtures/`answerCallbackQuery`/`editMessageReplyMarkup` recording.
- [x] 3.10 Run `npm test` and `npm run typecheck`.

## Phase 4: Operator Step and Final Verification (after PR2)

Expand Down
21 changes: 21 additions & 0 deletions src/adapters/telegram/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,27 @@ export function callerLocation(ctx: Context): CallerLocation | null {
};
}

// A callback-query variant of `callerLocation`: the chat and topic are those of
// the message that carried the tapped button (`ctx.msg`), and the user is the
// one who tapped. `callerLocation` is unchanged so commands never widen to
// edited or channel messages.
export interface CallbackLocation extends CallerLocation {
// The message carrying the button, or null when Telegram did not send it.
messageId: number | null;
}

export function callbackCallerLocation(ctx: Context): CallbackLocation | null {
const chatId = ctx.chat?.id;
const userId = ctx.from?.id;
if (chatId === undefined || userId === undefined) return null;
return {
chatId,
userId,
threadId: ctx.msg?.message_thread_id ?? null,
messageId: ctx.callbackQuery?.message?.message_id ?? null,
};
}

// Resolves a Telegram (chatId, userId) pair to this team's existing
// membership, used by group commands that need the caller's own
// membership (e.g. /datachannel's admin check) before delegating to a use
Expand Down
3 changes: 2 additions & 1 deletion src/adapters/telegram/hackathon-commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { runCommand } from "./command-outcome";
import type { DomainErrorReasons, DomainErrorReplies } from "./command-outcome";
import { callerLocation, resolveGroupMembership } from "./context";
import type { CallerLocation } from "./context";
import { runParticipation } from "./participation";
import { registerParticipationCallback, runParticipation } from "./participation";
import { isPrivateChat } from "./team-picker";

// `/hackathon` and `/hackathons` (design.md "Data Flow", "Error Taxonomy" —
Expand Down Expand Up @@ -104,6 +104,7 @@ async function resolveMember(deps: HackathonCommandDeps, loc: CallerLocation) {
}

export function registerHackathonCommands(bot: Bot, deps: HackathonCommandDeps): void {
registerParticipationCallback(bot, deps);
bot.command("hackathon", async (ctx) => {
const loc = await requireGroupCaller(ctx, deps, "hackathon");
if (!loc) return;
Expand Down
73 changes: 73 additions & 0 deletions src/adapters/telegram/participation.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import type { Bot } from "grammy";
import type { TeamId, MembershipId } from "../../domain/ids";
import type { MemberRepo, MembershipRepo, TeamRepo } from "../../domain/ports";
import { participateInHackathon } from "../../domain/usecases/participate-in-hackathon";
import type { ParticipateInHackathonDeps } from "../../domain/usecases/participate-in-hackathon";
import { PARTICIPATE_CALLBACK_PREFIX } from "./chat-publisher";
import { participateCopy } from "./copy";
import { callbackCallerLocation, resolveGroupMembership } from "./context";
import { isPrivateChat } from "./team-picker";

export interface ParticipationParams {
event: string;
Expand Down Expand Up @@ -78,3 +83,71 @@ export async function runParticipation(
});
}
}

// The button's callback data is untrusted: `hp:` plus a slug of at most 40
// characters (slug.ts MAX_SLUG_LENGTH). The team is never read from it.
const PARTICIPATE_DATA = new RegExp(
`^${PARTICIPATE_CALLBACK_PREFIX}([a-z0-9]+(?:-[a-z0-9]+)*)$`,
);
const MAX_SLUG_LENGTH = 40;
const CALLBACK_EVENT = "hackathon-participate-callback";

export interface ParticipationCallbackDeps extends ParticipateInHackathonDeps {
teamRepo: TeamRepo;
memberRepo: MemberRepo;
membershipRepo: MembershipRepo;
}

function errorCodeOf(err: unknown): string {
return err instanceof Error ? err.name : "UnknownError";
}

// Registers the `hp:<slug>` handler. Malformed data or a foreign prefix never
// matches; a private or missing chat is ignored. A non-member or non-admin
// gets one alert and nothing changes. An admin's callback is answered early
// (creating a topic can be slow) and the rest is `runParticipation`. Only
// answerCallbackQuery and reply are best-effort: a Telegram failure on them
// must not become a 500 that redelivers the update.
export function registerParticipationCallback(bot: Bot, deps: ParticipationCallbackDeps): void {
bot.callbackQuery(PARTICIPATE_DATA, async (ctx) => {
const slug = PARTICIPATE_DATA.exec(ctx.callbackQuery.data)?.[1];
const loc = callbackCallerLocation(ctx);
if (slug === undefined || slug.length > MAX_SLUG_LENGTH || !loc || isPrivateChat(ctx)) return;

const safe = async (action: () => Promise<unknown>, reason: string): Promise<void> => {
try {
await action();
} catch (err) {
deps.logger.log({ event: CALLBACK_EVENT, outcome: "error", errorCode: errorCodeOf(err), reason });
}
};

const resolved = await resolveGroupMembership(deps, loc.chatId, loc.userId);
if (!resolved || resolved.membership.role !== "admin") {
deps.logger.log({
event: CALLBACK_EVENT,
outcome: "refused",
errorCode: resolved ? "UnauthorizedError" : "NotFoundError",
});
await safe(
() => ctx.answerCallbackQuery({ text: participateCopy.adminOnly, show_alert: true }),
"answer-failed",
);
return;
}

await safe(() => ctx.answerCallbackQuery(), "answer-failed");
await runParticipation(
{
event: CALLBACK_EVENT,
teamId: resolved.team.id,
membershipId: resolved.membership.id,
chatId: loc.chatId,
slug,
callbackMessageId: loc.messageId,
reply: (text) => safe(() => ctx.reply(text), "reply-failed"),
},
deps,
);
});
}
48 changes: 37 additions & 11 deletions src/domain/usecases/run-hackathon-job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import { analysisCopy, FETCH_FAILURE_PHRASES } from "../copy";
import { formatAnalysis } from "../hackathon/format";
import { normalizeUrlKey } from "../hackathon/url";
import type { AnalysisJob, AnalysisJobMessage, JobOutcome } from "../entities";
import type { AnalysisJob, AnalysisJobMessage, HackathonAnalysis, JobOutcome } from "../entities";
import { analyzeHackathon, type AnalyzeHackathonDeps } from "./analyze-hackathon";
import { postAnalysisAndLinkTopic } from "./link-analysis-to-topic";
import type { AnalysisJobRepo, AnalysisQuota, ChatPublisher, Logger } from "../ports";
Expand Down Expand Up @@ -98,11 +98,7 @@ async function postPersistedResult(
deps,
);
} else {
await deps.chatPublisher.post(job.chatId, null, formatAnalysis({
slug: analysis.slug,
fields: analysis.fields,
suggestions: analysis.suggestedRepos,
}));
await postToGeneral(job, analysis, deps);
}
} catch (err) {
// RESI-001: postPersistedResult must never throw out of
Expand Down Expand Up @@ -176,11 +172,7 @@ async function runClaimedJob(
deps,
);
} else {
await deps.chatPublisher.post(job.chatId, null, formatAnalysis({
slug: analysis.slug,
fields: analysis.fields,
suggestions: analysis.suggestedRepos,
}));
await postToGeneral(job, analysis, deps);
}
await deps.analysisJobRepo.markSucceeded(job.id);
await deps.analysisQuota.release(job.teamId, job.utcDay, job.id, false);
Expand All @@ -190,6 +182,40 @@ async function runClaimedJob(
}
}

// hackathon-participation: the General post carries the participation button
// (a topic post never does) and its message id is stored so the button can be
// removed later. The store is best-effort: the post already went out, so a
// failure here must not fail the job (a retry would repost the analysis). The
// only cost is that the stored message's button is not cleared on join; the
// tapped message's own button still is.
async function postToGeneral(
job: AnalysisJob,
analysis: HackathonAnalysis,
deps: RunHackathonJobDeps,
): Promise<void> {
const messageId = await deps.chatPublisher.post(
job.chatId,
null,
formatAnalysis({
slug: analysis.slug,
fields: analysis.fields,
suggestions: analysis.suggestedRepos,
}),
{ participateSlug: analysis.slug },
);
try {
await deps.hackathonAnalysisRepo.setGeneralMessageId(job.teamId, analysis.id, messageId);
} catch (err) {
deps.logger.log({
event: "hackathon-job",
teamId: job.teamId,
outcome: "error",
errorCode: err instanceof Error ? err.name : "UnknownError",
reason: "general-message-id-store-failed",
});
}
}

interface JobErrorClassification {
transient: boolean;
refund: boolean;
Expand Down
Loading
Loading