diff --git a/.github/workflows/assign-issue.yml b/.github/workflows/assign-issue.yml new file mode 100644 index 000000000..4d330ad4f --- /dev/null +++ b/.github/workflows/assign-issue.yml @@ -0,0 +1,217 @@ +name: assign-issue + +# Implements contributor issue-claiming via /assign and /unassign comments. +# +# WHY body-marker: GitHub's Assignees API rejects non-collaborators, so the bot +# edits the issue body to embed an invisible +# marker as the claim of record. This mirrors the Rust @rustbot claim precedent +# (which hit the same API restriction) and matches the marker +# convention already in use in this repo (preview-reap.yml). +# +# ALL decision logic lives in scripts/issue-claim.mjs (pure functions, unit-tested). +# This workflow is thin I/O glue: checkout → parse → fetch → write. +# +# Security: untrusted comment body is passed via env var and read via process.env +# inside the github-script — never interpolated into the JS string (S1, injection guard). + +on: + issue_comment: + types: [created] + +concurrency: + group: assign-issue-${{ github.event.issue.number }} + cancel-in-progress: false + +jobs: + claim: + runs-on: ubuntu-latest + timeout-minutes: 5 + + # Cheap pre-filter before any API calls: + # - only issue comments (not PR review comments) — github.event.issue.pull_request is + # absent on real issues and present on PR threads + # - skip bot authors to prevent feedback loops (O5) + # - only /assign or /unassign comment starters + if: >- + github.event.issue.pull_request == null && + github.event.comment.user.type != 'Bot' && + (startsWith(github.event.comment.body, '/assign') || + startsWith(github.event.comment.body, '/unassign')) + + permissions: + issues: write + contents: read + + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: 22 + + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + # Pass untrusted comment body via env — never interpolate into the JS string (S1). + COMMENT_BODY: ${{ github.event.comment.body }} + with: + script: | + const { parseCommand, parseClaimMarker, writeClaim, stripClaim, canUnassign, IN_PROGRESS } + = await import(`${process.env.GITHUB_WORKSPACE}/scripts/issue-claim.mjs`); + + const actor = context.payload.comment.user.login; + const { owner, repo } = context.repo; + const issue_number = context.payload.issue.number; + + // Authoritative command parse from the env-isolated body (S1). + const command = parseCommand(process.env.COMMENT_BODY); + if (!command) return; + + // Fetch the current issue body (source of truth for the claim marker). + const { data: issueData } = await github.rest.issues.get({ owner, repo, issue_number }); + let body = issueData.body || ''; + let claim = parseClaimMarker(body); + + // ── /assign ────────────────────────────────────────────────────────── + if (command === '/assign') { + if (claim && claim.user !== actor) { + await github.rest.issues.createComment({ + owner, repo, issue_number, + body: `Задачата вече е заета от @${claim.user}. ` + + `Заетостта се освобождава автоматично след 21 дни неактивност. ` + + `Ако искаш да работиш по нея, изчакай или се свържи с @${claim.user}.`, + }); + return; + } + + if (claim && claim.user === actor) { + await github.rest.issues.createComment({ + owner, repo, issue_number, + body: `Задачата вече е заета от теб (@${actor}). ` + + `Напиши \`/unassign\` ако се откажеш.`, + }); + return; + } + + // Write the claim with a 3-attempt retry on concurrent body edits (O2). + let updateError; + for (let attempt = 1; attempt <= 3; attempt++) { + try { + await github.rest.issues.update({ + owner, repo, issue_number, + body: writeClaim(body, actor), + }); + updateError = null; + break; + } catch (error) { + if (error.status === 409 || error.status === 422) { + updateError = error; + // Re-read the body before retrying so we apply onto the latest version. + const { data: fresh } = await github.rest.issues.get({ owner, repo, issue_number }); + body = fresh.body || ''; + claim = parseClaimMarker(body); + // If someone else claimed while we were retrying, bail out gracefully. + if (claim && claim.user !== actor) { + await github.rest.issues.createComment({ + owner, repo, issue_number, + body: `Задачата беше заета от @${claim.user} точно преди теб. ` + + `Свържи се с тях ако смяташ да работиш по нея.`, + }); + return; + } + await new Promise((r) => setTimeout(r, attempt * 500)); + } else { + throw error; + } + } + } + if (updateError) throw updateError; + + // Add the in-progress label — best-effort; 404/422 are benign (label may not exist yet). + try { + await github.rest.issues.addLabels({ owner, repo, issue_number, labels: [IN_PROGRESS] }); + } catch (error) { + if (error.status !== 404 && error.status !== 422) { + core.warning(`addLabels(${IN_PROGRESS}): ${error.status} ${error.message}`); + } + } + + // Assign the actor — best-effort; non-collaborators get a 422 and that is expected. + try { + await github.rest.issues.addAssignees({ owner, repo, issue_number, assignees: [actor] }); + } catch (error) { + if (error.status !== 404 && error.status !== 422) { + core.warning(`addAssignees(${actor}): ${error.status} ${error.message}`); + } + } + + await github.rest.issues.createComment({ + owner, repo, issue_number, + body: `Задачата е заета от @${actor}! ` + + `Когато отвориш PR, сложи \`Closes #${issue_number}\` в описанието. ` + + `Ако се откажеш, напиши \`/unassign\` за да освободиш задачата.`, + }); + return; + } + + // ── /unassign ──────────────────────────────────────────────────────── + if (command === '/unassign') { + // Resolve privilege via collaborator permission level (O6). + let privileged = false; + try { + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ + owner, repo, username: actor, + }); + privileged = ['admin', 'write'].includes(data.permission); + } catch { + privileged = false; + } + + if (!claim) { + await github.rest.issues.createComment({ + owner, repo, issue_number, + body: `Няма активно заемане на тази задача.`, + }); + return; + } + + if (!canUnassign({ actor, claimedUser: claim.user, privileged })) { + await github.rest.issues.createComment({ + owner, repo, issue_number, + body: `Само @${claim.user} или maintainer може да освободи тази задача.`, + }); + return; + } + + await github.rest.issues.update({ + owner, repo, issue_number, + body: stripClaim(body), + }); + + // Remove label — 404 means label was never added or already removed (benign). + try { + await github.rest.issues.removeLabel({ + owner, repo, issue_number, name: IN_PROGRESS, + }); + } catch (error) { + if (error.status !== 404 && error.status !== 422) { + core.warning(`removeLabel(${IN_PROGRESS}): ${error.status} ${error.message}`); + } + } + + // Remove assignee — 404/422 benign (was never formally assigned). + try { + await github.rest.issues.removeAssignees({ + owner, repo, issue_number, assignees: [claim.user], + }); + } catch (error) { + if (error.status !== 404 && error.status !== 422) { + core.warning(`removeAssignees(${claim.user}): ${error.status} ${error.message}`); + } + } + + await github.rest.issues.createComment({ + owner, repo, issue_number, + body: `Задачата е освободена от @${claim.user}. ` + + `Вече е свободна — коментирай \`/assign\` ако искаш да я вземеш.`, + }); + } diff --git a/.github/workflows/stale-assignment-check.yml b/.github/workflows/stale-assignment-check.yml new file mode 100644 index 000000000..9d1cbdbe0 --- /dev/null +++ b/.github/workflows/stale-assignment-check.yml @@ -0,0 +1,146 @@ +name: stale-assignment-check + +# Daily sweep of open issues labelled 'status: in-progress'. +# +# Purpose: auto-release zombie claims so contributors who picked up an issue and +# went quiet don't block others indefinitely. +# +# Windows: +# 14 days of inactivity → nudge comment (Bulgarian) with . +# 21 days of inactivity → release: strip claim marker, remove label + assignee, +# post release comment (Bulgarian) with . +# +# The invisible markers ( / ) make re-runs +# idempotent: shouldNudge() detects a prior nudge by marker only, not prose. +# +# Linked-PR awareness: if a cross-referenced PR is still open the issue is skipped +# regardless of idle time (fork-source PRs included — hasOpenLinkedPr reads state +# off the timeline event, no second API call required). +# +# All decision logic lives in scripts/issue-claim.mjs (pure functions, unit-tested). +# This workflow is thin I/O glue: paginate → call pure fn → write result. + +on: + schedule: + - cron: '17 6 * * *' # daily at 06:17 UTC + workflow_dispatch: {} + +concurrency: + group: stale-assignment-check + cancel-in-progress: false + +jobs: + sweep: + runs-on: ubuntu-latest + # Paging through potentially many issues + fetching timelines; cap well under + # the 360-min default so a runaway paginate can't pin a runner all day. + timeout-minutes: 10 + permissions: + issues: write + pull-requests: read + + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: 22 + + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const { parseClaimMarker, stripClaim, computeIdleDays, hasOpenLinkedPr, shouldNudge, + IN_PROGRESS, NUDGE_DAYS, RELEASE_DAYS, NUDGE_MARKER, RELEASE_MARKER } + = await import(`${process.env.GITHUB_WORKSPACE}/scripts/issue-claim.mjs`); + + const { owner, repo } = context.repo; + const nowMs = Date.now(); + + // Paginate all open issues labelled 'status: in-progress'. + const issues = await github.paginate( + github.rest.issues.listForRepo, + { owner, repo, state: 'open', labels: IN_PROGRESS, per_page: 100 }, + ); + + for (const issue of issues) { + // Isolate each issue so one poisoned entry cannot abort the whole sweep (S2). + try { + // PRs are returned by listForRepo; skip them. + if (issue.pull_request) continue; + + const issue_number = issue.number; + + // Skip issues that carry the label but no valid claim record — + // leave those for humans to triage. + const claim = parseClaimMarker(issue.body || ''); + if (!claim) continue; + + // Fetch the full timeline for idle-time + linked-PR calculations. + const timeline = await github.paginate( + github.rest.issues.listEventsForTimeline, + { owner, repo, issue_number, per_page: 100 }, + ); + + // If the CLAIMER has an open PR linked, work is in flight — skip (O3). Only their + // own PR counts, so a stranger's fork PR can't lock the claim indefinitely. + if (hasOpenLinkedPr(timeline, claim.user)) continue; + + // Idle is measured from the claimer's own activity, not any human's, so other + // people commenting on the issue can't keep an abandoned claim alive. + const idle = computeIdleDays({ + timeline, + createdAt: issue.created_at, + nowMs, + claimedUser: claim.user, + }); + + if (idle >= RELEASE_DAYS) { + // ── Release path ───────────────────────────────────────────── + // Strip the claim from the issue body. + await github.rest.issues.update({ + owner, repo, issue_number, + body: stripClaim(issue.body || ''), + }); + + // Remove the 'status: in-progress' label; swallow 404/422 only (O4). + try { + await github.rest.issues.removeLabel({ + owner, repo, issue_number, name: IN_PROGRESS, + }); + } catch (labelErr) { + if (labelErr.status !== 404 && labelErr.status !== 422) { + core.warning(`#${issue_number}: removeLabel failed (${labelErr.status}): ${labelErr.message}`); + } + } + + // Remove the assignee; swallow 404/422 only (O4). + try { + await github.rest.issues.removeAssignees({ + owner, repo, issue_number, + assignees: [claim.user], + }); + } catch (assignErr) { + if (assignErr.status !== 404 && assignErr.status !== 422) { + core.warning(`#${issue_number}: removeAssignees failed (${assignErr.status}): ${assignErr.message}`); + } + } + + // Post a Bulgarian release comment with the invisible marker (O1). + await github.rest.issues.createComment({ + owner, repo, issue_number, + body: `${RELEASE_MARKER}\n@${claim.user} Заявката ти за тази задача беше автоматично освободена след 21 дни липса на активност. Ако искаш да продължиш, коментирай \`/assign\` за да я заявиш отново.`, + }); + + } else if (shouldNudge({ idleDays: idle, nudgeDays: NUDGE_DAYS, timeline, nudgeMarker: NUDGE_MARKER })) { + // ── Nudge path ─────────────────────────────────────────────── + // Post a Bulgarian nudge comment with the invisible marker (O1). + await github.rest.issues.createComment({ + owner, repo, issue_number, + body: `${NUDGE_MARKER}\n@${claim.user} Все още работиш ли по тази задача? Не сме виждали активност от ${Math.floor(idle)} дни. Ако се окажеш в затруднение, питай в коментар. При липса на активност задачата ще бъде освободена автоматично след 21 дни.`, + }); + } + + } catch (e) { + core.warning(`stale-assignment-check: skipping issue #${issue.number}: ${e.message}`); + } + } diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ad866013e..6086d48a9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,6 +14,9 @@ - За **уязвимост в сигурността** — **не** отваряйте публичен issue. Вижте политиката за сигурност (частен канал) в раздела **Security** на хранилището. - За по-голяма промяна е добре първо да я обсъдим в issue, преди да отделяте време за код. +- Преди да започнете работа по съществуващ issue, коментирайте `/assign`, за да го заявите + (и `/unassign`, ако се откажете) — така избягваме двама души да работят по едно и също. Сложете + `Closes #NN` в PR-а, за да се свърже с issue-то. ## Среда за разработка diff --git a/docs/README.md b/docs/README.md index 568550ad6..4396b39b7 100644 --- a/docs/README.md +++ b/docs/README.md @@ -9,6 +9,7 @@ - [`etl-pipeline-state.md`](etl-pipeline-state.md) — анализ на текущото състояние на ETL pipeline-а. - [`etl-architecture.md`](etl-architecture.md) — целевата ETL архитектура (RFC): предложение за състоянието и реда на изпълнение. - [`v1-implementation-plan.md`](v1-implementation-plan.md) — precompute слоят и пагинацията (защо rollup-и и keyset вместо per-request GROUP BY / OFFSET). +- [`implementation-plans/230-issue-claim-bot.md`](implementation-plans/230-issue-claim-bot.md) — план за внедряване на бота за заявяване на Issue-та (`/assign`) срещу дублирана работа (#230). - [`integrity-gate.md`](integrity-gate.md) — reconciliation gate-ът: hard asserts върху тоталите при import/CI. - [`anomaly-report.md`](anomaly-report.md) — cross-row аномалии при опресняване: какво `value_flag` не хваща на ниво отделен договор. - [`deploy.md`](deploy.md) — деплой към Cloudflare: двата Worker-а (`sigma`, `sigma-etl`) и споделеният D1 per environment. diff --git a/docs/implementation-plans/230-issue-claim-bot.md b/docs/implementation-plans/230-issue-claim-bot.md new file mode 100644 index 000000000..af455f626 --- /dev/null +++ b/docs/implementation-plans/230-issue-claim-bot.md @@ -0,0 +1,275 @@ +# Implementation Plan: #230 — Issue-claiming bot (`/assign`) against duplicated work + +## Executive Summary + +| Field | Value | +|---|---| +| Ticket | [midt-bg/sigma#230](https://github.com/midt-bg/sigma/issues/230) (+ expanded English proposal provided alongside it — same design) | +| Goal | Let drive-by contributors *claim* an issue via a `/assign` comment so two people don't start the same work — without granting repo access. Auto-release zombie claims. | +| Approach | Two thin GitHub Actions workflows + **one tested `scripts/issue-claim.mjs` module** holding all decision logic | +| Complexity | Medium (logic is small; the traps are security + testability, not volume) | +| Time Estimate | ~1 day (0.5d module + tests, 0.25d workflows, 0.25d owner steps + docs) | +| Risk Level | Medium — runs with `issues: write` on **untrusted comment input**; the issue's inline draft has exploitable defects (below) that this plan fixes | +| Status | **Draft — awaiting maintainer decision.** #230 is labeled `status: needs-decision`; the model itself is not yet approved. This plan is the "if approved, build it this way" blueprint and also touches `.github/` (coordinate with @cefothe per the issue). | + +> **Decision gate first.** Do not open the implementation PR until the model is accepted on #230 and the `.github/` overlap with the CI/branch-protection thread is cleared with @cefothe. This plan assumes acceptance. + +--- + +## 🚨 Critical Implementation Standards + +From the global CLAUDE.md + repo `AGENTS.md`/`CONTRIBUTING.md`, non-negotiable for this change: + +- **`tests-with-code`** — logic ships with `node --test` unit tests. The issue's inline-YAML JS is untestable; **this is the single biggest deviation the plan corrects.** +- **`error-handling`** — every parse/API path guarded. The draft's unguarded `JSON.parse` is a live DoS (Finding S2). +- **Pin actions to SHA** — repo pins every third-party action with a version comment. The draft's `actions/github-script@v7` (floating tag) must become the in-repo-vetted `@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0`. +- **Least privilege** — per-job `permissions:`; **do not** flip the repo default to "Read and write" (the issue's owner-step #2 over-privileges every other workflow — see Finding S6). +- **User-facing text in Bulgarian** — `CONTRIBUTING.md` mandates it. The draft's bot replies are English (Decision D3). +- **No secrets, no scope creep** — `.github/` only + one `scripts/` module + one `CONTRIBUTING.md` line. + +--- + +## Current State Analysis + +### What exists (verified) + +| Path | Relevance | +|---|---| +| `.github/workflows/preview-reap.yml` | **Direct precedent.** Scheduled job, `` body-marker, `github-script@…v9.0.0` SHA-pinned, `timeout-minutes: 10`, `concurrency`, least-priv `permissions`. Mirror this shape. | +| `.github/workflows/scripts-test.yml` | Runs `node --test scripts/*.test.mjs`. A new `scripts/issue-claim.test.mjs` is **auto-picked-up with zero workflow changes** — the intended extension point. | +| `scripts/reap-previews.mjs` + `.test.mjs` | **The extraction pattern to copy:** exported pure functions (`selectStale`, `reapStale`) + injected I/O (`fetchImpl`, `del`) + `isMain(...)` guard, fixture-driven tests including exact boundary cases. | +| `scripts/check-docs.mjs` | Canonical `isMain(import.meta.url, process.argv[1])` guard; pure-vs-fs separation. | +| Labels (`gh label list`) | `status: needs-triage/needs-decision/blocked`, `good first issue`, `help wanted` exist. **`status: in-progress` does NOT — must be created.** | +| `CONTRIBUTING.md` | Bulgarian, forks+PR flow. Add one Bulgarian line about `/assign`. | +| `.github/ISSUE_TEMPLATE/` | `config.yml`, bug/data/feature templates. Optional hint line. | + +### Gap vs. the issue draft + +The issue ships **complete inline JS inside both YAML files**. That surface pattern (markers, github-script) is right and consistent with `preview-reap.yml`, but it: +1. Cannot be unit-tested (violates `tests-with-code`). +2. Duplicates `MARKER`/`BANNER`/label constants across both files → drift bug. +3. Uses a floating action tag. +4. Carries three exploitable defects (below). + +--- + +## Target Architecture + +``` +.github/workflows/assign-issue.yml (thin: issue_comment → env → node → octokit writes) +.github/workflows/stale-assignment-check.yml (thin: schedule → node → octokit writes) + │ + └── imports ──▶ scripts/issue-claim.mjs (ALL decision logic, pure) + scripts/issue-claim.test.mjs (node:test, ~50 cases) +``` + +**Principle (from `reap-previews.mjs`):** functions never receive an octokit client — they receive *already-fetched plain data* and return decisions. Workflows are I/O glue: fetch → call pure fn → write result. This is exactly the `preview-reap.yml` boundary. + +### `scripts/issue-claim.mjs` — exported pure functions + +| Function | Signature (in → out) | Replaces draft logic | +|---|---|---| +| `parseCommand(body)` | comment string → `'/assign' \| '/unassign' \| null` | the `startsWith` gate + exact-word guard | +| `parseClaimMarker(body)` | issue body → `{ user } \| null`, **never throws**, validates `user` matches `/^[A-Za-z0-9-]{1,39}$/` | `JSON.parse(m[1])` | +| `writeClaim(body, user)` | → body with banner+marker prepended to a **stripped** base | `writeClaim` | +| `stripClaim(body)` | → body with **all** markers+banners removed (global regex) | `stripClaim` | +| `canUnassign({actor, claimedUser, authorAssociation})` | → boolean | the privilege branch | +| `computeIdleDays({timeline, createdAt, nowMs})` | → number, last **human** event, bot events excluded | the `lastHuman` reduction | +| `hasOpenLinkedPr(timeline)` | → boolean; reads state off the event, null-safe on `source`/`issue`/`repository`; fork-source aware (O3) | the `cross-referenced` check | +| `shouldNudge({idleDays, nudgeDays, timeline, nudgeMarker})` | → boolean, idempotent; detects prior nudge by the **`` marker** only, not free-text (O1) | the `nudged` guard | + +Constants (`MARKER`, `BANNER`, `IN_PROGRESS`, `NUDGE_DAYS=14`, `RELEASE_DAYS=21`) live **once** here; both workflows import them → kills the duplication/drift bug. + +`isMain(import.meta.url, process.argv[1])` guard so the file is importable by tests but runnable if ever needed directly. + +### Marker format (unchanged, consistent with repo) + +`` + visible `**🔧 Claimed by:** @x` banner + `status: in-progress` label. Mirrors the existing `` precedent. + +### "Two files" vs. the proposal's "three workflows" + +The expanded proposal lists **three** workflows, but its own rollout says *"add both workflow files"* — item #3 (**linked-PR awareness**) is a *pre-check inside* the stale sweep (it needs the same timeline fetch to decide skip-vs-nudge), not a separate file. This plan builds **two** workflow files; linked-PR awareness is the `hasOpenLinkedPr(timeline)` guard in `stale-assignment-check.yml`. Splitting it into a third workflow would duplicate the timeline fetch for no benefit. Flag D4 if the maintainer actually intended a distinct file. + +### Precedent (from the proposal, load-bearing for the design choice) + +The body-marker approach is not novel: **Rust's `@rustbot claim`** hit the identical GitHub non-collaborator restriction and solved it the same way (bot edits the top comment to record the claim); **Kubernetes' Prow** uses `/assign`/`/unassign` chatops. GitHub's **Triage role** is the native alternative (deferred — needs a per-contributor maintainer invite, doesn't fit drive-by volunteers). **`actions/stale`** is the standard tool for the *PR*-staleness companion (open question, out of scope here). + +--- + +## Multi-Agent Review — Findings & Resolutions + +Three specialists reviewed the draft against repo conventions. **Consensus: accept the *model*, reject the *inline implementation*; extract + test + harden.** + +### 🔒 Security (must-fix before merge) + +| ID | Sev | Finding | Fix in this plan | +|---|---|---|---| +| **S1** | HIGH | **Claim/identity spoofing.** An issue author can edit their own body to inject `` + a fake banner. `MARKER.match` (no `/g`) trusts it; the `/unassign` gate then compares against attacker-supplied `claim.user`. | Treat the body as **untrusted**. `parseClaimMarker` validates `user` shape and rejects malformed. The privilege gate uses `context`-derived `actor`/`author_association` (trustworthy), never a body-derived identity for authorization. Consider (D2) storing the claim of record in a **bot-authored comment** rather than the editable body. | +| **S2** | HIGH | **Unguarded `JSON.parse` DoSes the whole daily sweep.** One issue with `` throws inside the sweep loop → every later claimed issue skipped, permanently. | `parseClaimMarker` wraps parse in try/catch, returns `null`. Sweep `continue`s on `null`. Covered by test 2.3. | +| **S3** | HIGH | **Non-global strip leaves a second marker → sticky claim.** `.replace(MARKER,'')` without `/g` removes only the first; a seeded duplicate survives `/unassign` and auto-release. | `stripClaim` uses global regexes; reject/normalize >1 marker. Tests 3.8/3.9. | +| **S4** | MED | `author_association` gate: `MEMBER` = any public org member, not necessarily write. | Acceptable for a low-stakes "release a claim" (defense-in-depth, self-assertion impossible). Optional hardening (D2): `repos.getCollaboratorPermissionLevel` ≥ `write`. | +| **S5** | MED | `claim.user` flows into comment bodies / `removeAssignees` unvalidated (newline/markdown injection into bot comments). | `parseClaimMarker` validates against `/^[A-Za-z0-9-]{1,39}$/`; non-matching → treated as no claim. | +| **S6** | LOW | **Both** the issue *and* the expanded proposal's rollout step #2 ("Settings → Read and write permissions") over-privilege **every** workflow in the repo. | **Drop it.** Per-job `permissions:` blocks already scope correctly (`preview-reap.yml` proves this works with the repo default read-only). Do not flip the org/repo default. | +| **S7** | LOW | Comment-spam re-triggers; no throttle. | Keep no-op early-returns (already in draft); `concurrency` per issue serializes. Accept residual (GITHUB_TOKEN rate-limited). | + +### 🏛 Architecture + +- **P0** Extract logic to `scripts/issue-claim.mjs` (above). **P0** SHA-pin `github-script` → `…v9.0.0`; pin `checkout`/`setup-node` (SHAs already in `scripts-test.yml`) now that workflows run node. +- **P1** De-dup constants via the shared module (free once extracted). **P1** Add `timeout-minutes: 5`, `concurrency`, least-priv `permissions` to both jobs. +- **P2** Keep **two** workflow files (different triggers `issue_comment` vs `schedule`) — correct decomposition, mirrors `preview.yml` vs `preview-reap.yml`. Share the *module*, not the workflow. +- **Scale note:** bound the sweep's `listForRepo` pagination (issue-paced volume is fine; add a `MAX_PAGES`-style guard comment like `listWorkerScripts`). + +### 🧪 Testing + +Full `node:test` matrix accepted as the coverage contract (see Testing Strategy). Critical: `computeIdleDays` bot-vs-human (2 cases pin "bot nudge must not reset the clock"), the malformed-marker no-throw cases, and the anti-spoof strip cases. + +### ⚙️ Operational correctness (round-2 review) + +A second review pass (security-auditor + deployment-engineer + test-automator, 2026-07-14) reconfirmed S1–S7 and added the following **operational** fixes. All are folded into the phases/tests below. + +| ID | Sev | Finding | Fix | +|---|---|---|---| +| **O1** | MED | **Nudge-detection by free-text `/auto-release/` is brittle** — the release comment can contain the same substring, and a re-claimed-then-idle issue then never re-nudges. | Emit an **invisible marker** in each bot comment: `` in nudges, `` in releases. `shouldNudge` matches the nudge marker only — never shared prose. Kills the false-positive. | +| **O2** | MED | **`issues.update` (body) is a read-modify-write with no retry.** `github-script` does not retry 409/422; a colliding edit silently drops the claim while the confirmation still posts. Per-issue `concurrency` serializes the *queue* but not an external concurrent body edit. | Wrap the single `issues.update` in a small 3-try backoff on `status ∈ {409,422}`; re-read the body before each retry. Bounded, no new deps. | +| **O3** | MED | **`hasOpenLinkedPr` assumes the base repo.** A `cross-referenced` event from a **fork** PR carries `source.issue.repository` = the fork; skip-logic that queries `context.repo` misreads fork PRs and can wrongly release a still-active claim. | Read `event.source.issue.state`/`pull_request` directly off the timeline event (already includes state); do **not** re-`pulls.get` against `context.repo`. Null-safe on `source`/`issue`/`repository`. Test 6.x adds a fork-source fixture. | +| **O4** | LOW | **Blanket `try/catch {}` around `removeLabel`/`removeAssignees` hides real failures** — a swallowed `removeLabel` leaves `status: in-progress` set, so the sweep re-processes and re-comments the issue forever. | Catch **by `error.status`**: swallow only the known-benign `404` (label/assignee absent) / `422` (not assignable); `core.warning(...)` anything else instead of silent `{}`. | +| **O5** | LOW | **Bot feedback-loop guarded only by comment prose** (`startsWith('/assign')`). Any future bot message starting with the token re-triggers. | Add an identity guard to the job `if:`: `github.event.comment.user.type != 'Bot'`. Defense-in-depth beside the existing prose gate. | +| **O6** | LOW | **`author_association` is coarse** — `MEMBER` = any public-org member, not necessarily write on *this* repo (also reconfirms S4). | Preferred: `repos.getCollaboratorPermissionLevel(actor) ∈ {admin, write}` for the `/unassign` override. `canUnassign` already takes an injected `privileged` boolean, so the resolver swaps in with **no test churn**. | + +`shouldNudge`/`computeIdleDays` signatures gain the nudge-marker string (O1); `canUnassign` keeps its injected `privileged` boolean so O6 is a workflow-glue swap, not a logic change. `hasOpenLinkedPr` reads state off the timeline event (O3). + +--- + +## Implementation Phases + +### Phase 0 — Decision & coordination (BLOCKING, no code) +- Confirm #230 model accepted by maintainers (@todorkolev, @nedda76). +- Clear `.github/` overlap with the CI/branch-protection thread with **@cefothe**. +- Confirm Decisions D1–D3 below. + +### Phase 1 — Module + tests FIRST (TDD) + +**Task 1.0 — Write `scripts/issue-claim.test.mjs` (red).** +Encode the full matrix (Testing Strategy). Fixed reference instant `const NOW = Date.parse('2026-07-14T12:00:00Z')` + `daysAgo(n)` helper — no live `Date.now()` (repo convention; and `Date.now()` is unavailable in some harnesses). `import { describe, it } from 'node:test'`, `import assert from 'node:assert/strict'`. + +**Task 1.1 — Implement `scripts/issue-claim.mjs` (green).** +Pure functions above + constants + `isMain` guard. No octokit import. Every parse guarded (`error-handling`). Global strip regexes (S3). `user` validation (S5). + +**Verification** +```bash +node --test scripts/issue-claim.test.mjs +pnpm lint +``` + +### Phase 2 — Workflows (thin glue) + +**Task 2.1 — `.github/workflows/assign-issue.yml`.** +- `on: issue_comment: [created]`; job `if:` filters PR comments + non-claim bodies + **bot authors** (`github.event.comment.user.type != 'Bot'`, O5) (cheap gate; authoritative parse via `parseCommand`). +- `permissions: { issues: write, contents: read }`; `concurrency: assign-issue-${{ github.event.issue.number }}` (`cancel-in-progress: false`); `timeout-minutes: 5`. +- Steps: `checkout` (pinned SHA) → `setup-node@…` (node 22) → `github-script@…v9.0.0`. +- **Pass `comment.body` via `env:`**, read `process.env` inside the script — never interpolate untrusted input into `${{ }}` or `run:` (injection guard). `import` the pure fns from `../../scripts/issue-claim.mjs`. +- Claim write: `issues.update(body)` wrapped in a **3-try backoff on 409/422**, re-reading the body per attempt (O2). +- `/unassign` privilege: resolve `privileged` via `repos.getCollaboratorPermissionLevel(actor) ∈ {admin, write}` and pass it into `canUnassign` (O6). +- Native `addAssignees` stays a best-effort no-op, **caught by `error.status`** (swallow 404/422 only; `core.warning` else — O4). + +**Task 2.2 — `.github/workflows/stale-assignment-check.yml`.** +- `on: schedule: cron '17 6 * * *'` + `workflow_dispatch`. +- `permissions: { issues: write, pull-requests: read }`; `concurrency`; `timeout-minutes: 10`. +- Sweep: `listForRepo(labels: status: in-progress)` (paginated) → per issue, wrapped in `try/catch` so one poisoned issue can't abort the run (S2): `parseClaimMarker` (skip null), `hasOpenLinkedPr` (skip; fork-source aware, O3), `computeIdleDays`, then `shouldNudge`/release. +- Bot comments carry invisible markers — `` (nudge) / `` (release) — so `shouldNudge` detects a prior nudge deterministically (O1). +- `removeLabel`/`removeAssignees` on release caught **by `error.status`** (404/422 benign; else `core.warning`, O4). + +**Verification** +```bash +# YAML/action-pin sanity +grep -rn "github-script@" .github/workflows/assign-issue.yml .github/workflows/stale-assignment-check.yml # must show the v9.0.0 SHA +# Dry-run the sweep after merge to a branch: +gh workflow run stale-assignment-check.yml --repo +``` + +### Phase 3 — Owner steps & docs + +**Task 3.1 — Create label** (style-matched to existing `status:`): +```bash +gh label create "status: in-progress" --repo midt-bg/sigma \ + --description "По задачата се работи (заявена чрез /assign)" --color 1d76db +``` +**Task 3.2 — `CONTRIBUTING.md`** — one Bulgarian line in the workflow section: *«Коментирай `/assign` преди да започнеш; `/unassign` ако се откажеш. Сложи `Closes #NN` в PR-а.»* +**Task 3.3 — Do NOT** change repo Workflow-permission default (S6). Confirm branch protection allows these workflows (coordinate w/ @cefothe). + +**Verification** +```bash +gh label list --repo midt-bg/sigma | grep "in-progress" +``` + +--- + +## Testing Strategy + +`node --test scripts/issue-claim.test.mjs` (auto-run by `scripts-test.yml`). ~50 cases grouped by function; boundary/adversarial cases explicit. + +- **`parseCommand`**: `/assign`, `/unassign`, whitespace, trailing text; **exact-word guard rejects** `/assignee`, `/assign-me`, `/assigned` (even uppercased); **case-insensitive** (`/ASSIGN` → `/assign`, matching the GHA `startsWith` gate); empty/null/undefined. +- **`parseClaimMarker`** (8): valid; absent; **malformed JSON → null, no throw (S2)**; missing/non-string `user` → null (S5); mid-body; duplicate markers. +- **`writeClaim`/`stripClaim`** (10): round-trip; re-claim idempotent same user; re-claim different user drops old; **strip removes duplicate banner + duplicate marker (S3 anti-spoof)**; preserves surrounding text. +- **`canUnassign`** (7): self; OWNER/MEMBER/COLLABORATOR release other; CONTRIBUTOR/NONE cannot; unknown assoc → false (allowlist). +- **`computeIdleDays`** (11): single human; **bot nudge must NOT reset clock**; latest human wins; no human → `created_at` fallback; boundaries at 14 & 21; unparseable timestamp excluded, no throw. +- **`hasOpenLinkedPr`** (9): open→true; closed/merged→false; cross-ref to issue (not PR)→false; `source`/`issue`/`repository` null → false, no throw; **fork-source PR still open→true (O3)**. +- **`shouldNudge`** (9): ≥14 & not nudged→true; <14→false; already-nudged (`` present)→false; **human comment containing the words "auto-release" does NOT suppress (O1)**; a ``-marked comment does not count as a nudge; empty timeline→nudge. +- **Composed decision cases** (12): the `/assign`/`/unassign`/sweep outcomes end-to-end over the pure fns (no API). + +No mocking of the module under test; feed fixture arrays shaped like octokit responses (mirrors `reap-previews.test.mjs`). + +--- + +## Risk Assessment + +| Risk | Sev | Mitigation | +|---|---|---| +| Claim/identity spoofing via editable body (S1) | High | Untrusted-body model; authz off `context` not body; optional bot-comment-of-record (D2) | +| Malformed marker DoSes daily sweep (S2) | High | Guarded parse + `continue`; test 2.3 | +| Sticky claim via duplicate marker (S3) | Med | Global strip regex; tests 3.8/3.9 | +| `.github/` overlap w/ branch-protection thread | Med | Phase 0 coordination w/ @cefothe (per issue) | +| Over-privileging via "Read and write" default (S6) | Med | Drop owner-step #2; per-job permissions only | +| Unenforced convention (PR opened on unclaimed issue) | Low | Accepted (documented in issue); social, not technical | +| 14/21-day windows are guesses | Low | Constants in one module; tune after real data | + +--- + +## Rollout Plan + +**Pre-merge:** module tests green; actions SHA-pinned; `@cefothe` sign-off on `.github/`; label created. +**Deploy:** merge to `main` — scheduled sweep goes live from `main` automatically (like `preview-reap.yml`); comment workflow active on next `issue_comment`. +**Post-deploy validation:** (1) `/assign` on a throwaway issue → banner+marker+label appear, confirmation comment posts. (2) `/unassign` clears all three. (3) `workflow_dispatch` the sweep as a dry check. (4) Seed a malformed marker on a test issue, run the sweep, confirm it skips that issue and still processes others (S2 regression). +**Rollback:** delete the two workflow files (idempotent; markers/labels are inert text a maintainer can strip). + +--- + +## Decisions Needed + +- **D1 — Model acceptance.** #230 is `needs-decision`. Build only after maintainer go-ahead. *(Recommend: proceed once accepted.)* +- **D2 — Claim of record: body-marker vs bot-comment.** Body-marker matches the draft + `` precedent but is user-editable (S1/S5 mitigated but present). A bot-authored comment is tamper-resistant (`user.type==='Bot'` verifiable) at the cost of more API calls. *(Recommend: ship body-marker with S1/S3/S5 hardening now; note bot-comment as a follow-up if spoofing is observed.)* +- **D3 — Bot reply language.** `CONTRIBUTING.md` mandates Bulgarian user-facing text; the draft replies are English. *(Recommend: Bulgarian replies to match the contributor-facing convention.)* +- **D4 — Two files vs. "three workflows".** The proposal's prose says three; its rollout says two. This plan ships two (linked-PR awareness inline in the sweep). *(Recommend: confirm two; a third file would duplicate the timeline fetch.)* +- Open questions carried from the issue + expanded proposal (all **deferred, out of scope**): + - Separate `actions/stale` workflow for stale **PRs** (distinct from issue claims)? + - Per-`priority`-label 14/21-day windows? + - At what contributor-base size does granting the **Triage role** become worth it (can coexist with this bot)? + +--- + +## Success Criteria + +- [ ] Maintainer decision on #230 recorded; `.github/` overlap cleared with @cefothe (Phase 0) +- [ ] `scripts/issue-claim.mjs` — all logic extracted, pure, guarded, `isMain` guard +- [ ] `scripts/issue-claim.test.mjs` — full matrix green under `node --test` +- [ ] Both workflows thin, `github-script@…v9.0.0` SHA-pinned, `timeout-minutes` + `concurrency` + least-priv `permissions` +- [ ] S1/S2/S3/S5 fixes present and test-covered; owner-step "Read and write" **not** applied (S6) +- [ ] Operational fixes O1 (nudge marker), O2 (update retry), O3 (fork-source PR), O4 (status-scoped catch), O5 (bot-author guard), O6 (permission-level override) present and, where pure, test-covered +- [ ] `status: in-progress` label created; `CONTRIBUTING.md` Bulgarian line added +- [ ] `pnpm lint` clean; no secrets; change scoped to `.github/` + `scripts/` + `CONTRIBUTING.md` + +--- +**Status:** Draft — pending #230 decision +**Created:** 2026-07-14 +**Updated:** 2026-07-14 (round-2 operational review: O1–O6 folded in) +**Reviewers:** security-auditor, architect-review, deployment-engineer, test-automator (consensus: accept model, extract + harden) diff --git a/scripts/issue-claim.mjs b/scripts/issue-claim.mjs new file mode 100644 index 000000000..32fbc2d91 --- /dev/null +++ b/scripts/issue-claim.mjs @@ -0,0 +1,242 @@ +// Issue-claim bot — pure decision logic for assign-issue.yml and stale-assignment-check.yml. +// +// All functions receive plain data (octokit responses already fetched) and return decisions. +// The workflows are I/O glue: fetch → call pure fn → write result. +// This mirrors the reap-previews.mjs boundary exactly. +// +// No octokit import; no external deps — node built-ins only. +import { pathToFileURL } from 'node:url'; +import { resolve } from 'node:path'; + +// ── Constants ───────────────────────────────────────────────────────────────── + +export const IN_PROGRESS = 'status: in-progress'; +export const NUDGE_DAYS = 14; +export const RELEASE_DAYS = 21; +export const NUDGE_MARKER = ''; +export const RELEASE_MARKER = ''; + +// MARKER: the invisible HTML comment that records the claim payload. +// BANNER: the visible heading line that attributes the claim to the user. +// Both use GLOBAL regexes so stripClaim removes ALL occurrences — S3 anti-spoof. +const MARKER_RE = //g; +const BANNER_RE = /^\*\*🔧 Claimed by:\*\* @[A-Za-z0-9-]{1,39}\n?/gm; + +// GitHub username: 1–39 alphanumeric or hyphens, no leading/trailing hyphen per spec. +// We accept any sequence matching /^[A-Za-z0-9-]{1,39}$/ as a safe allowlist — S5. +const USERNAME_RE = /^[A-Za-z0-9-]{1,39}$/; + +// ── parseCommand ────────────────────────────────────────────────────────────── + +/** + * Parses a comment body and returns the first whitespace-delimited token if it + * is '/assign' or '/unassign', matched case-insensitively (so '/Assign', + * '/ASSIGN' etc. resolve to the canonical lowercase command). Any other token + * (including '/assignee', '/assign-me', '/assigned') returns null. + * + * @param {string | null | undefined} body + * @returns {'/assign' | '/unassign' | null} + */ +export function parseCommand(body) { + if (!body) return null; + // Case-insensitive: matches the GitHub Actions `startsWith` gate (also case-insensitive) so a + // '/ASSIGN' comment actually takes effect instead of running the job and silently no-opping. + // The exact-word guard still rejects '/assignee', '/assign-me', '/assigned', etc. + const token = (body.trim().split(/\s+/)[0] ?? '').toLowerCase(); + if (token === '/assign') return '/assign'; + if (token === '/unassign') return '/unassign'; + return null; +} + +// ── parseClaimMarker ────────────────────────────────────────────────────────── + +/** + * Extracts and validates the claim marker embedded in an issue body. + * Never throws — wraps JSON.parse in try/catch (S2). + * Validates `user` against the GitHub username pattern (S5). + * + * @param {string | null | undefined} body + * @returns {{ user: string } | null} + */ +export function parseClaimMarker(body) { + if (!body) return null; + // MARKER_RE is global; String.match returns every marker (or null). + const matches = body.match(MARKER_RE); + if (!matches) return null; + // More than one claim marker means the body was tampered with — the normal flow (writeClaim) + // always leaves exactly one. Fail safe: treat as no valid claim rather than trusting whichever + // marker happens to come first (a planted marker could otherwise shadow the legitimate one). + if (matches.length > 1) return null; + // Extract the JSON payload between the first ':' and the closing '-->'. + const raw = matches[0].replace(/^$/, ''); + let parsed; + try { + parsed = JSON.parse(raw); + } catch { + return null; + } + if (typeof parsed?.user !== 'string') return null; + if (!USERNAME_RE.test(parsed.user)) return null; + return { user: parsed.user }; +} + +// ── stripClaim ──────────────────────────────────────────────────────────────── + +/** + * Returns body with ALL claim markers and ALL banner lines removed. + * Uses global regexes — S3 anti-spoof (removes duplicates too). + * Null-safe: returns '' for null/undefined input. + * + * @param {string | null | undefined} body + * @returns {string} + */ +export function stripClaim(body) { + const s = body ?? ''; + // Reset lastIndex before each replace (global regexes retain state across calls). + MARKER_RE.lastIndex = 0; + BANNER_RE.lastIndex = 0; + return s.replace(BANNER_RE, '').replace(MARKER_RE, '').replace(/^\n+/, ''); +} + +// ── writeClaim ──────────────────────────────────────────────────────────────── + +/** + * Returns the issue body with the claim banner and marker prepended, stripping + * any prior claim first (idempotent — safe to call on an already-claimed body). + * + * Format: + * **🔧 Claimed by:** @ + * + * + * + * + * @param {string | null | undefined} body + * @param {string} user + * @returns {string} + */ +export function writeClaim(body, user) { + const base = stripClaim(body); + return `**🔧 Claimed by:** @${user}\n\n\n${base}`; +} + +// ── canUnassign ─────────────────────────────────────────────────────────────── + +/** + * Returns true if the actor may release a claim. + * Two cases: + * 1. The actor is the original claimer (self-release). + * 2. The actor is privileged (write/admin on the repo — resolved by the workflow, O6). + * + * @param {{ actor: string, claimedUser: string | null, privileged: boolean }} opts + * @returns {boolean} + */ +export function canUnassign({ actor, claimedUser, privileged }) { + return actor === claimedUser || privileged === true; +} + +// ── computeIdleDays ─────────────────────────────────────────────────────────── + +/** + * Returns the number of days since the last HUMAN (non-Bot) event in the timeline. + * Bot events (actor.type === 'Bot') are excluded so a nudge comment from the bot + * does not reset the idle clock (O1-adjacent). + * + * Timestamp is `e.created_at || e.submitted_at`. Unparseable timestamps are skipped. + * Falls back to `Date.parse(createdAt)` when no human event is found. + * + * @param {{ timeline: object[] | null, createdAt: string, nowMs: number }} opts + * @returns {number} + */ +export function computeIdleDays({ timeline, createdAt, nowMs, claimedUser }) { + const events = timeline ?? []; + let lastMs = NaN; + for (const e of events) { + // Measure inactivity from the CLAIMER's own activity only. Other people commenting or + // triaging the issue must not keep an abandoned claim alive — that would defeat the sweep's + // purpose ("contributors who picked up an issue and went quiet don't block others"). When + // claimedUser is unset, fall back to any non-bot activity (the bot's own nudge never counts). + const isClaimerEvent = claimedUser ? e?.actor?.login === claimedUser : e?.actor?.type !== 'Bot'; + if (!isClaimerEvent) continue; + const ts = Date.parse(e?.created_at || e?.submitted_at); + if (!Number.isFinite(ts)) continue; + if (Number.isNaN(lastMs) || ts > lastMs) { + lastMs = ts; + } + } + const baseline = Number.isFinite(lastMs) ? lastMs : Date.parse(createdAt); + return (nowMs - baseline) / 86_400_000; +} + +// ── hasOpenLinkedPr ─────────────────────────────────────────────────────────── + +/** + * Returns true if the timeline contains a cross-reference to an open pull request. + * Reads `state` directly off the event (event.source.issue.state) so fork-source + * PRs are handled correctly without a second API call (O3). + * Null-safe on source / issue / repository. + * + * @param {object[] | null} timeline + * @returns {boolean} + */ +export function hasOpenLinkedPr(timeline, claimedUser) { + if (!timeline) return false; + return timeline.some( + (e) => + e?.event === 'cross-referenced' && + e?.source?.issue?.pull_request != null && + e?.source?.issue?.state === 'open' && + // Only the CLAIMER's own open PR keeps their claim alive. A stranger opening a PR from their + // fork that references an idle issue must not be able to lock the claim indefinitely + // (griefing). Author matching supports the normal drive-by flow (contributors work from + // their own forks). When claimedUser is unset, any open linked PR counts (back-compat). + (!claimedUser || e?.source?.issue?.user?.login === claimedUser), + ); +} + +// ── shouldNudge ─────────────────────────────────────────────────────────────── + +/** + * Returns true if the issue should receive a nudge comment: + * - idleDays >= nudgeDays, AND + * - no prior nudge detected (defined as: no `commented` event whose body includes + * the invisible nudgeMarker string — O1). + * + * The release branch (idle >= RELEASE_DAYS) is decided in the workflow before + * calling shouldNudge; this function only guards the nudge step. + * + * @param {{ idleDays: number, nudgeDays: number, timeline: object[] | null, nudgeMarker: string }} opts + * @returns {boolean} + */ +export function shouldNudge({ idleDays, nudgeDays, timeline, nudgeMarker }) { + if (idleDays < nudgeDays) return false; + const events = timeline ?? []; + // Only a BOT-authored comment carrying the marker counts as a prior nudge. Checking the author + // type stops a human from planting in a comment to suppress nudges forever + // (anti-spoof, sibling to the claim-marker hardening). + const alreadyNudged = events.some( + (e) => + e?.event === 'commented' && + e?.actor?.type === 'Bot' && + typeof e?.body === 'string' && + e.body.includes(nudgeMarker), + ); + return !alreadyNudged; +} + +// ── isMain guard ────────────────────────────────────────────────────────────── + +/** + * True iff this module is the Node.js entry point. + * URL-safe (handles percent-encoded paths, spaces, non-ASCII). + * + * @param {string} importMetaUrl + * @param {string | undefined} argvPath + * @returns {boolean} + */ +export function isMain(importMetaUrl, argvPath) { + return Boolean(argvPath) && importMetaUrl === pathToFileURL(resolve(argvPath)).href; +} + +if (isMain(import.meta.url, process.argv[1])) { + console.log('issue-claim.mjs: loaded as entry point. No CLI command implemented.'); +} diff --git a/scripts/issue-claim.test.mjs b/scripts/issue-claim.test.mjs new file mode 100644 index 000000000..38c0421e3 --- /dev/null +++ b/scripts/issue-claim.test.mjs @@ -0,0 +1,777 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import { + canUnassign, + computeIdleDays, + hasOpenLinkedPr, + IN_PROGRESS, + NUDGE_DAYS, + NUDGE_MARKER, + parseClaimMarker, + parseCommand, + RELEASE_DAYS, + RELEASE_MARKER, + shouldNudge, + stripClaim, + writeClaim, +} from './issue-claim.mjs'; + +// Fixed reference instant — no live Date.now() (repo convention + harness constraint). +const NOW = Date.parse('2026-07-14T12:00:00Z'); +const daysAgo = (n) => new Date(NOW - n * 86_400_000).toISOString(); + +// ── parseCommand ─────────────────────────────────────────────────────────────── + +describe('parseCommand', () => { + it('returns /assign for bare /assign', () => { + assert.equal(parseCommand('/assign'), '/assign'); + }); + + it('returns /assign when followed by trailing text', () => { + assert.equal(parseCommand('/assign please'), '/assign'); + }); + + it('returns /assign when preceded by leading whitespace', () => { + assert.equal(parseCommand(' /assign'), '/assign'); + }); + + it('returns /unassign for bare /unassign', () => { + assert.equal(parseCommand('/unassign'), '/unassign'); + }); + + it('returns /unassign when followed by trailing text', () => { + assert.equal(parseCommand('/unassign thanks'), '/unassign'); + }); + + it('returns /unassign when preceded by leading whitespace', () => { + assert.equal(parseCommand(' /unassign'), '/unassign'); + }); + + it('rejects /assignee (extra suffix — not an exact match)', () => { + assert.equal(parseCommand('/assignee'), null); + }); + + it('rejects /assign-me (extra suffix — not an exact match)', () => { + assert.equal(parseCommand('/assign-me'), null); + }); + + it('rejects /assigned (extra suffix — not an exact match)', () => { + assert.equal(parseCommand('/assigned'), null); + }); + + it('returns null for unrelated comment body', () => { + assert.equal(parseCommand('This looks great!'), null); + }); + + it('returns null for empty string', () => { + assert.equal(parseCommand(''), null); + }); + + it('returns null for null', () => { + assert.equal(parseCommand(null), null); + }); + + it('is case-insensitive — /Assign resolves to /assign', () => { + assert.equal(parseCommand('/Assign'), '/assign'); + }); + + it('is case-insensitive — /ASSIGN resolves to /assign', () => { + assert.equal(parseCommand('/ASSIGN'), '/assign'); + }); + + it('is case-insensitive — /UnAssign resolves to /unassign', () => { + assert.equal(parseCommand('/UnAssign'), '/unassign'); + }); + + it('normalizes case but returns the canonical lowercase command', () => { + assert.equal(parseCommand('/UNASSIGN please'), '/unassign'); + }); + + it('rejects /ASSIGNEE even when uppercased (exact-word guard survives lowercasing)', () => { + assert.equal(parseCommand('/ASSIGNEE'), null); + }); + + it('returns null when the command is preceded by text (not the first token)', () => { + assert.equal(parseCommand('yes /assign'), null); + }); + + it('returns null for a quoted reply — GitHub quotes lines with a leading >', () => { + assert.equal(parseCommand('> /assign'), null); + assert.equal(parseCommand('> /unassign'), null); + }); + + it('returns null for undefined', () => { + assert.equal(parseCommand(undefined), null); + }); +}); + +// ── parseClaimMarker ─────────────────────────────────────────────────────────── + +describe('parseClaimMarker', () => { + it('parses a valid claim marker mid-body', () => { + const body = 'Some text\n\nMore text'; + assert.deepEqual(parseClaimMarker(body), { user: 'alice' }); + }); + + it('returns null when no marker is present', () => { + assert.equal(parseClaimMarker('No claim here'), null); + }); + + it('returns null (never throws) for malformed JSON — S2', () => { + const body = ''; + assert.doesNotThrow(() => parseClaimMarker(body)); + assert.equal(parseClaimMarker(body), null); + }); + + it('returns null when user field is missing — S5', () => { + const body = ''; + assert.equal(parseClaimMarker(body), null); + }); + + it('returns null when user is not a string — S5', () => { + const body = ''; + assert.equal(parseClaimMarker(body), null); + }); + + it('returns null when user contains invalid characters — S5', () => { + const body = ''; + assert.equal(parseClaimMarker(body), null); + }); + + it('returns null when user is too long (>39 chars) — S5', () => { + const long = 'a'.repeat(40); + const body = ``; + assert.equal(parseClaimMarker(body), null); + }); + + it('returns null when duplicate markers are present — tamper fail-safe', () => { + // A planted second marker must not shadow the legitimate one; >1 marker = treat as no claim. + const body = '\n'; + assert.equal(parseClaimMarker(body), null); + }); +}); + +// ── stripClaim / writeClaim ──────────────────────────────────────────────────── + +describe('stripClaim', () => { + it('removes banner and marker leaving surrounding text', () => { + const body = writeClaim('Original body', 'alice'); + const stripped = stripClaim(body); + assert.ok(!stripped.includes('sigma-claim'), 'marker not removed'); + assert.ok(!stripped.includes('🔧'), 'banner not removed'); + assert.ok(stripped.includes('Original body')); + }); + + it('removes DUPLICATE banners and markers — S3 anti-spoof', () => { + // Attacker seeds two markers; strip must remove both. + const seeded = + '**🔧 Claimed by:** @attacker\n\n\n' + + '**🔧 Claimed by:** @attacker\n\n\n' + + 'Real content'; + const stripped = stripClaim(seeded); + assert.equal((stripped.match(/sigma-claim/g) ?? []).length, 0); + assert.equal((stripped.match(/🔧/g) ?? []).length, 0); + assert.ok(stripped.includes('Real content')); + }); + + it('is null-safe — returns empty string for null input', () => { + assert.equal(stripClaim(null), ''); + }); + + it('is null-safe — returns empty string for undefined input', () => { + assert.equal(stripClaim(undefined), ''); + }); + + it('returns body unchanged when no claim present', () => { + assert.equal(stripClaim('Plain text'), 'Plain text'); + }); +}); + +describe('writeClaim', () => { + it('prepends banner and marker to the body', () => { + const result = writeClaim('Issue description', 'bob'); + assert.ok(result.startsWith('**🔧 Claimed by:** @bob')); + assert.ok(result.includes('')); + assert.ok(result.includes('Issue description')); + }); + + it('is idempotent — re-claiming with the same user produces one marker', () => { + const once = writeClaim('Body', 'alice'); + const twice = writeClaim(once, 'alice'); + assert.equal((twice.match(/sigma-claim/g) ?? []).length, 1); + assert.equal((twice.match(/🔧/g) ?? []).length, 1); + }); + + it('re-claim by different user drops old marker and banner — S3', () => { + const first = writeClaim('Body', 'alice'); + const second = writeClaim(first, 'bob'); + assert.ok(!second.includes('"user":"alice"'), 'old claim not removed'); + assert.ok(second.includes('"user":"bob"'), 'new claim not present'); + assert.equal((second.match(/sigma-claim/g) ?? []).length, 1); + assert.equal((second.match(/🔧/g) ?? []).length, 1); + }); + + it('preserves surrounding body text after stripping', () => { + const result = writeClaim('Description\n\nDetails', 'carol'); + assert.ok(result.includes('Description')); + assert.ok(result.includes('Details')); + }); + + it('round-trips: writeClaim → stripClaim → writeClaim is stable', () => { + const a = writeClaim('Body', 'dave'); + const b = writeClaim(stripClaim(a), 'dave'); + assert.equal(a, b); + }); +}); + +// ── canUnassign ──────────────────────────────────────────────────────────────── + +describe('canUnassign', () => { + it('allows the original claimer to unassign themselves — O6 self case', () => { + assert.equal(canUnassign({ actor: 'alice', claimedUser: 'alice', privileged: false }), true); + }); + + it('allows when privileged is true — O6 admin/write override', () => { + assert.equal( + canUnassign({ actor: 'maintainer', claimedUser: 'alice', privileged: true }), + true, + ); + }); + + it('allows privileged=true even when actor and claimedUser differ', () => { + assert.equal(canUnassign({ actor: 'admin', claimedUser: 'bob', privileged: true }), true); + }); + + it('denies a different user when not privileged', () => { + assert.equal(canUnassign({ actor: 'charlie', claimedUser: 'alice', privileged: false }), false); + }); + + it('denies when privileged is undefined', () => { + assert.equal( + canUnassign({ actor: 'charlie', claimedUser: 'alice', privileged: undefined }), + false, + ); + }); + + it('denies when privileged is null', () => { + assert.equal(canUnassign({ actor: 'charlie', claimedUser: 'alice', privileged: null }), false); + }); + + it('denies when claimedUser is absent and actor is not privileged', () => { + assert.equal(canUnassign({ actor: 'alice', claimedUser: null, privileged: false }), false); + }); +}); + +// ── computeIdleDays ──────────────────────────────────────────────────────────── + +describe('computeIdleDays', () => { + it('measures days since the single human event', () => { + const timeline = [{ event: 'commented', created_at: daysAgo(10), actor: { type: 'User' } }]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(20), nowMs: NOW }); + assert.ok(Math.abs(idle - 10) < 0.01, `expected ~10, got ${idle}`); + }); + + it('bot nudge must NOT reset the clock — O1-adjacent', () => { + const timeline = [ + { event: 'commented', created_at: daysAgo(15), actor: { type: 'User' } }, + { event: 'commented', created_at: daysAgo(5), actor: { type: 'Bot' } }, + ]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + assert.ok(Math.abs(idle - 15) < 0.01, `bot must not reset clock; expected ~15, got ${idle}`); + }); + + it('second bot event also does not reset the clock', () => { + const timeline = [ + { event: 'commented', created_at: daysAgo(20), actor: { type: 'User' } }, + { event: 'commented', created_at: daysAgo(7), actor: { type: 'Bot' } }, + { event: 'commented', created_at: daysAgo(2), actor: { type: 'Bot' } }, + ]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + assert.ok(Math.abs(idle - 20) < 0.01, `expected ~20, got ${idle}`); + }); + + it('uses the latest human event when multiple humans commented', () => { + const timeline = [ + { event: 'commented', created_at: daysAgo(20), actor: { type: 'User' } }, + { event: 'commented', created_at: daysAgo(8), actor: { type: 'User' } }, + ]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + assert.ok(Math.abs(idle - 8) < 0.01, `expected ~8, got ${idle}`); + }); + + it('falls back to createdAt when no human event exists', () => { + const timeline = [{ event: 'commented', created_at: daysAgo(5), actor: { type: 'Bot' } }]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(20), nowMs: NOW }); + assert.ok(Math.abs(idle - 20) < 0.01, `expected ~20, got ${idle}`); + }); + + it('falls back to createdAt when timeline is empty', () => { + const idle = computeIdleDays({ timeline: [], createdAt: daysAgo(14), nowMs: NOW }); + assert.ok(Math.abs(idle - 14) < 0.01, `expected ~14, got ${idle}`); + }); + + it('boundary: exactly 14 days idle', () => { + const timeline = [{ event: 'commented', created_at: daysAgo(14), actor: { type: 'User' } }]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + assert.ok(Math.abs(idle - 14) < 0.01, `expected ~14, got ${idle}`); + }); + + it('boundary: exactly 21 days idle', () => { + const timeline = [{ event: 'commented', created_at: daysAgo(21), actor: { type: 'User' } }]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + assert.ok(Math.abs(idle - 21) < 0.01, `expected ~21, got ${idle}`); + }); + + it('ignores events with unparseable timestamps — no throw', () => { + const timeline = [ + { event: 'commented', created_at: 'not-a-date', actor: { type: 'User' } }, + { event: 'commented', created_at: daysAgo(10), actor: { type: 'User' } }, + ]; + assert.doesNotThrow(() => { + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + assert.ok(Math.abs(idle - 10) < 0.01, `expected ~10, got ${idle}`); + }); + }); + + it('uses submitted_at when created_at is absent', () => { + const timeline = [{ event: 'reviewed', submitted_at: daysAgo(12), actor: { type: 'User' } }]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + assert.ok(Math.abs(idle - 12) < 0.01, `expected ~12, got ${idle}`); + }); + + it('handles null timeline gracefully', () => { + const idle = computeIdleDays({ timeline: null, createdAt: daysAgo(5), nowMs: NOW }); + assert.ok(Math.abs(idle - 5) < 0.01, `expected ~5, got ${idle}`); + }); + + it('when claimedUser is set, non-claimer activity does NOT reset the clock — review finding', () => { + // Claimer went quiet 20 days ago; someone else commented 1 day ago. The claim is abandoned, + // so idle must be measured from the claimer's last activity (~20), not the other person's. + const timeline = [ + { event: 'commented', created_at: daysAgo(20), actor: { type: 'User', login: 'claimer' } }, + { + event: 'commented', + created_at: daysAgo(1), + actor: { type: 'User', login: 'someone-else' }, + }, + ]; + const idle = computeIdleDays({ + timeline, + createdAt: daysAgo(30), + nowMs: NOW, + claimedUser: 'claimer', + }); + assert.ok(Math.abs(idle - 20) < 0.01, `expected ~20 (claimer's clock), got ${idle}`); + }); + + it("when claimedUser is set, the claimer's own recent activity resets the clock", () => { + const timeline = [ + { event: 'commented', created_at: daysAgo(20), actor: { type: 'User', login: 'claimer' } }, + { event: 'commented', created_at: daysAgo(3), actor: { type: 'User', login: 'claimer' } }, + ]; + const idle = computeIdleDays({ + timeline, + createdAt: daysAgo(30), + nowMs: NOW, + claimedUser: 'claimer', + }); + assert.ok(Math.abs(idle - 3) < 0.01, `expected ~3, got ${idle}`); + }); + + it('when claimedUser has no events, falls back to createdAt', () => { + const timeline = [ + { + event: 'commented', + created_at: daysAgo(2), + actor: { type: 'User', login: 'someone-else' }, + }, + ]; + const idle = computeIdleDays({ + timeline, + createdAt: daysAgo(18), + nowMs: NOW, + claimedUser: 'claimer', + }); + assert.ok(Math.abs(idle - 18) < 0.01, `expected ~18 (fallback), got ${idle}`); + }); +}); + +// ── hasOpenLinkedPr ──────────────────────────────────────────────────────────── + +describe('hasOpenLinkedPr', () => { + it('returns true for a cross-reference to an open PR on the same repo', () => { + const timeline = [ + { + event: 'cross-referenced', + source: { issue: { pull_request: {}, state: 'open' } }, + }, + ]; + assert.equal(hasOpenLinkedPr(timeline), true); + }); + + it('returns false for a cross-reference to a closed PR', () => { + const timeline = [ + { + event: 'cross-referenced', + source: { issue: { pull_request: {}, state: 'closed' } }, + }, + ]; + assert.equal(hasOpenLinkedPr(timeline), false); + }); + + it('returns false for a cross-reference to a merged PR (state=closed)', () => { + const timeline = [ + { + event: 'cross-referenced', + source: { issue: { pull_request: { merged_at: '2026-01-01T00:00:00Z' }, state: 'closed' } }, + }, + ]; + assert.equal(hasOpenLinkedPr(timeline), false); + }); + + it('returns false for a cross-reference to a plain issue (no pull_request field)', () => { + const timeline = [ + { + event: 'cross-referenced', + source: { issue: { state: 'open' } }, + }, + ]; + assert.equal(hasOpenLinkedPr(timeline), false); + }); + + it("returns true for the claimer's own open fork PR — O3 (state read off the event)", () => { + // Fork-source: source.issue.repository differs from the base repo — state is read off the + // event directly, so fork-source PRs are handled without re-fetching. Authored by the claimer, + // so it legitimately keeps the claim alive (the normal drive-by flow works from a fork). + const timeline = [ + { + event: 'cross-referenced', + source: { + issue: { + pull_request: {}, + state: 'open', + user: { login: 'claimer' }, + repository: { full_name: 'claimer/sigma' }, + }, + }, + }, + ]; + assert.equal(hasOpenLinkedPr(timeline, 'claimer'), true); + }); + + it("returns false for a stranger's open PR referencing the issue — griefing guard (review finding)", () => { + // An external user opens a PR from their fork that references an idle issue. It must NOT lock + // the claim: only the claimer's own PR counts. + const timeline = [ + { + event: 'cross-referenced', + source: { + issue: { + pull_request: {}, + state: 'open', + user: { login: 'griefer' }, + repository: { full_name: 'griefer/sigma' }, + }, + }, + }, + ]; + assert.equal(hasOpenLinkedPr(timeline, 'claimer'), false); + }); + + it('without claimedUser, any open linked PR counts (back-compat)', () => { + const timeline = [ + { + event: 'cross-referenced', + source: { issue: { pull_request: {}, state: 'open', user: { login: 'anyone' } } }, + }, + ]; + assert.equal(hasOpenLinkedPr(timeline), true); + }); + + it('returns false when source is null — null-safe', () => { + const timeline = [{ event: 'cross-referenced', source: null }]; + assert.equal(hasOpenLinkedPr(timeline), false); + }); + + it('returns false when source.issue is null — null-safe', () => { + const timeline = [{ event: 'cross-referenced', source: { issue: null } }]; + assert.equal(hasOpenLinkedPr(timeline), false); + }); + + it('returns false for an empty timeline', () => { + assert.equal(hasOpenLinkedPr([]), false); + }); + + it('returns false for a null timeline', () => { + assert.equal(hasOpenLinkedPr(null), false); + }); +}); + +// ── shouldNudge ──────────────────────────────────────────────────────────────── + +describe('shouldNudge', () => { + it('returns true when idle ≥ NUDGE_DAYS and not yet nudged', () => { + assert.equal( + shouldNudge({ idleDays: 14, nudgeDays: NUDGE_DAYS, timeline: [], nudgeMarker: NUDGE_MARKER }), + true, + ); + }); + + it('returns false when idle < NUDGE_DAYS', () => { + assert.equal( + shouldNudge({ idleDays: 13, nudgeDays: NUDGE_DAYS, timeline: [], nudgeMarker: NUDGE_MARKER }), + false, + ); + }); + + it('returns false when idle > NUDGE_DAYS but already nudged via marker — O1', () => { + const timeline = [ + { + event: 'commented', + body: `Heads up! ${NUDGE_MARKER}`, + actor: { type: 'Bot' }, + created_at: daysAgo(5), + }, + ]; + assert.equal( + shouldNudge({ idleDays: 20, nudgeDays: NUDGE_DAYS, timeline, nudgeMarker: NUDGE_MARKER }), + false, + ); + }); + + it('human comment containing the words "auto-release" does NOT suppress nudge — O1', () => { + // Only the invisible marker suppresses; free-text "auto-release" in a human comment does not. + const timeline = [ + { + event: 'commented', + body: 'This issue will auto-release if idle', + actor: { type: 'User' }, + created_at: daysAgo(2), + }, + ]; + assert.equal( + shouldNudge({ idleDays: 16, nudgeDays: NUDGE_DAYS, timeline, nudgeMarker: NUDGE_MARKER }), + true, + ); + }); + + it('a HUMAN comment with an injected nudge marker does NOT suppress — anti-spoof (review finding)', () => { + // A user plants the invisible marker to try to block nudges/auto-release forever. Only a + // bot-authored comment counts, so the nudge still fires. + const timeline = [ + { + event: 'commented', + body: `sneaky ${NUDGE_MARKER}`, + actor: { type: 'User', login: 'griefer' }, + created_at: daysAgo(2), + }, + ]; + assert.equal( + shouldNudge({ idleDays: 16, nudgeDays: NUDGE_DAYS, timeline, nudgeMarker: NUDGE_MARKER }), + true, + ); + }); + + it('a release-marked comment does NOT count as a nudge', () => { + // A sigma-release comment must not satisfy the nudge-already-sent guard. + const timeline = [ + { + event: 'commented', + body: `Claim released. ${RELEASE_MARKER}`, + actor: { type: 'Bot' }, + created_at: daysAgo(3), + }, + ]; + assert.equal( + shouldNudge({ idleDays: 16, nudgeDays: NUDGE_DAYS, timeline, nudgeMarker: NUDGE_MARKER }), + true, + ); + }); + + it('returns true for idle ≥ NUDGE_DAYS with an empty timeline', () => { + assert.equal( + shouldNudge({ idleDays: 15, nudgeDays: NUDGE_DAYS, timeline: [], nudgeMarker: NUDGE_MARKER }), + true, + ); + }); + + it('returns false when idle is exactly 0 (just commented)', () => { + assert.equal( + shouldNudge({ idleDays: 0, nudgeDays: NUDGE_DAYS, timeline: [], nudgeMarker: NUDGE_MARKER }), + false, + ); + }); + + it('returns true when idle equals RELEASE_DAYS and not yet nudged', () => { + // RELEASE_DAYS ≥ NUDGE_DAYS — workflow decides release vs nudge, but shouldNudge itself + // still returns true at 21 days if no marker is present. + assert.equal( + shouldNudge({ + idleDays: RELEASE_DAYS, + nudgeDays: NUDGE_DAYS, + timeline: [], + nudgeMarker: NUDGE_MARKER, + }), + true, + ); + }); + + it('returns true for null timeline when idle >= nudgeDays (null coerces to empty — no prior nudge)', () => { + assert.equal( + shouldNudge({ + idleDays: 20, + nudgeDays: NUDGE_DAYS, + timeline: null, + nudgeMarker: NUDGE_MARKER, + }), + true, + ); + }); +}); + +// ── exported constants ───────────────────────────────────────────────────────── + +describe('exported constants', () => { + it('IN_PROGRESS is the correct label string', () => { + assert.equal(IN_PROGRESS, 'status: in-progress'); + }); + + it('NUDGE_DAYS is 14', () => { + assert.equal(NUDGE_DAYS, 14); + }); + + it('RELEASE_DAYS is 21', () => { + assert.equal(RELEASE_DAYS, 21); + }); + + it('NUDGE_MARKER is the invisible HTML comment', () => { + assert.equal(NUDGE_MARKER, ''); + }); + + it('RELEASE_MARKER is the invisible HTML comment', () => { + assert.equal(RELEASE_MARKER, ''); + }); +}); + +// ── composed decision cases ──────────────────────────────────────────────────── + +describe('composed decision cases', () => { + it('/assign flow: writeClaim produces parseable marker', () => { + const body = writeClaim('Fix the bug', 'alice'); + const parsed = parseClaimMarker(body); + assert.deepEqual(parsed, { user: 'alice' }); + }); + + it('/unassign flow: stripClaim leaves no parseable marker', () => { + const claimed = writeClaim('Fix the bug', 'alice'); + const stripped = stripClaim(claimed); + assert.equal(parseClaimMarker(stripped), null); + }); + + it('stale sweep: idle issue with no nudge gets nudged', () => { + const timeline = [{ event: 'commented', created_at: daysAgo(16), actor: { type: 'User' } }]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + const nudge = shouldNudge({ + idleDays: idle, + nudgeDays: NUDGE_DAYS, + timeline, + nudgeMarker: NUDGE_MARKER, + }); + assert.equal(nudge, true); + }); + + it('stale sweep: issue with open PR is skipped', () => { + const timeline = [ + { + event: 'cross-referenced', + source: { issue: { pull_request: {}, state: 'open' } }, + }, + { event: 'commented', created_at: daysAgo(20), actor: { type: 'User' } }, + ]; + assert.equal(hasOpenLinkedPr(timeline), true); + }); + + it('stale sweep: already-nudged issue does not get a second nudge', () => { + const timeline = [ + { event: 'commented', created_at: daysAgo(20), actor: { type: 'User' } }, + { + event: 'commented', + body: `Please update us. ${NUDGE_MARKER}`, + actor: { type: 'Bot' }, + created_at: daysAgo(6), + }, + ]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + const nudge = shouldNudge({ + idleDays: idle, + nudgeDays: NUDGE_DAYS, + timeline, + nudgeMarker: NUDGE_MARKER, + }); + // idle is ~20d (bot doesn't reset), but already nudged + assert.equal(nudge, false); + }); + + it('stale sweep: canUnassign allows privileged release of another user', () => { + const body = writeClaim('Refactor login', 'alice'); + const claim = parseClaimMarker(body); + assert.ok(claim !== null); + const allowed = canUnassign({ actor: 'maintainer', claimedUser: claim.user, privileged: true }); + assert.equal(allowed, true); + }); + + it('stale sweep: malformed marker body is skipped without throw — S2', () => { + const poisonBody = ''; + assert.doesNotThrow(() => { + const claim = parseClaimMarker(poisonBody); + assert.equal(claim, null); + }); + }); + + it('re-assign: second writeClaim over existing strips old, embeds new — S3', () => { + const first = writeClaim('Issue body', 'alice'); + const second = writeClaim(first, 'bob'); + assert.equal(parseClaimMarker(second)?.user, 'bob'); + assert.ok(!second.includes('"user":"alice"')); + }); + + it('bot comment in timeline does not reset idle clock — full scenario', () => { + const timeline = [ + { event: 'commented', created_at: daysAgo(22), actor: { type: 'User' } }, + { + event: 'commented', + body: `Reminding you. ${NUDGE_MARKER}`, + actor: { type: 'Bot' }, + created_at: daysAgo(8), + }, + ]; + const idle = computeIdleDays({ timeline, createdAt: daysAgo(30), nowMs: NOW }); + // idle must be based on the human event 22 days ago, not the bot at 8 days ago + assert.ok(idle >= 21, `expected idle >= 21, got ${idle}`); + // already nudged, so shouldNudge must be false + const nudge = shouldNudge({ + idleDays: idle, + nudgeDays: NUDGE_DAYS, + timeline, + nudgeMarker: NUDGE_MARKER, + }); + assert.equal(nudge, false); + }); + + it('parseCommand + canUnassign: non-privileged third party cannot unassign', () => { + const cmd = parseCommand('/unassign'); + assert.equal(cmd, '/unassign'); + const allowed = canUnassign({ actor: 'random-user', claimedUser: 'alice', privileged: false }); + assert.equal(allowed, false); + }); + + it('full assign-then-release cycle is clean', () => { + const original = 'Report a bug in the dashboard'; + const claimed = writeClaim(original, 'dave'); + assert.ok(parseClaimMarker(claimed) !== null); + const released = stripClaim(claimed); + assert.equal(parseClaimMarker(released), null); + assert.ok(released.includes(original)); + }); +});