From ea0ed8462d1088604a09229ec9b3d71ecf1982cf Mon Sep 17 00:00:00 2001 From: Aashil Date: Wed, 30 Sep 2026 10:42:18 +0545 Subject: [PATCH 1/2] docs: add CLAUDE.md, split changelog, new skills, and remove stray diagnostics Adds a root CLAUDE.md indexing .claude/skills/ and the doc split, so an agent working from a fresh checkout discovers existing institutional knowledge instead of re-deriving it -- directly motivated by today's PR #93 mistake, which happened while working from a scratch clone with none of this loaded. - Splits the changelog out of PHASE2-SETUP.md into CHANGELOG.md. - Adds debug-crisp-triage-not-investigated skill: the elimination checklist from the 2026-09-29 investigations. - Extends verify-github-actions-change with the stale-branch-name lesson from PR #93. - Corrects PHASE2-SETUP.md's file listing: investigated.json no longer gates anything post-checkedThroughAt; escalated.json (which actually does) wasn't listed at all. - Removes 3 temporary diagnostic scripts + their workflow, left on master from the 2026-09-29 investigations and missed in that day's own cleanup -- caught while reviewing scripts/ for this same work. Co-Authored-By: Claude Sonnet 5 --- .../SKILL.md | 34 +++++++++++++++++++ .../verify-github-actions-change/SKILL.md | 9 +++++ .../workflows/diagnose-conversation-state.yml | 28 --------------- CHANGELOG.md | 28 +++++++++++++++ CLAUDE.md | 25 ++++++++++++++ PHASE2-SETUP.md | 17 ++-------- README.md | 4 ++- scripts/diagnose-active-cap.mjs | 21 ------------ scripts/diagnose-conversation-state.mjs | 9 ----- scripts/diagnose-message-timestamps.mjs | 16 --------- 10 files changed, 102 insertions(+), 89 deletions(-) create mode 100644 .claude/skills/debug-crisp-triage-not-investigated/SKILL.md delete mode 100644 .github/workflows/diagnose-conversation-state.yml create mode 100644 CHANGELOG.md create mode 100644 CLAUDE.md delete mode 100644 scripts/diagnose-active-cap.mjs delete mode 100644 scripts/diagnose-conversation-state.mjs delete mode 100644 scripts/diagnose-message-timestamps.mjs diff --git a/.claude/skills/debug-crisp-triage-not-investigated/SKILL.md b/.claude/skills/debug-crisp-triage-not-investigated/SKILL.md new file mode 100644 index 0000000..ada1192 --- /dev/null +++ b/.claude/skills/debug-crisp-triage-not-investigated/SKILL.md @@ -0,0 +1,34 @@ +--- +name: debug-crisp-triage-not-investigated +description: Diagnose why a specific Crisp conversation was never auto-investigated (no issue filed, no note posted), working through real confirmed causes in order instead of guessing. +--- + +# Debug "why wasn't this conversation investigated" + +Three real, distinct root causes have each separately explained this symptom during actual investigations. Don't assume it's whichever one you found last time — work through these in order, checking state directly rather than reasoning from code alone. + +## 0. First, get the facts, not a theory + +For the session_id in question, pull the relevant state directly: + +``` +# search state/resolved-seen.json, state/investigated.json, state/escalated.json for the session_id +``` + +And get its *current live state* from Crisp directly (a temporary read-only `workflow_dispatch` diagnostic, deleted after use — see `verify-github-actions-change` skill's pattern for building one safely). Fetch the conversation object (`state`, `created_at`, `updated_at`, `active.last`) — **never** dump raw message content/sender identity into a workflow log; that's a real PII boundary, confirmed blocked once already. Timestamps and `type` only, if you need message-level data at all. + +## Elimination checklist (each confirmed real, on a different session, in the same investigation) + +1. **Is it actually past `AUTO_ESCALATE_MAX_HOURS` (30 days)?** Compute hours since `created_at`. If it's genuinely older than 720h and was never resolved, this is *working as designed* — "past this, only a manual note escalates it" is a deliberate policy, not a bug. (`session_2fc63232`, once its `checkedThroughAt` reset correctly, still didn't fire because it had aged past this window in the meantime.) +2. **Was it permanently blocked by a stale flag?** Pre-2026-09-29, `investigated.has(session_id)` and `escalated[session_id].autoEscalated` were permanent once set, with no reset. If you're reading this after that fix, this specific cause shouldn't recur — but if `checkedThroughAt` looks like it should have re-armed and didn't, that's a regression worth treating seriously, not shrugging off. +3. **Did a resolve/reopen cycle mask an independent staleness check?** If `state/resolved-seen.json` has an entry for this session (`isReopen` would be true) and the reopen delta was empty (nothing new since that resolve), confirm the fallback to a full-history stale check actually fired — check for a `stale ... classifier says` log line, not just a `reopened after resolve` one. If neither log line exists at all for recent runs, it may have fallen outside the active-conversation page cap (200/account) — check its rank directly rather than assuming (`session_bd0acc7b` was rank 12; a different real case that hit the cap was confirmed on `THEMEGRILL` specifically). +4. **Does the screenshot you're comparing against actually match this session_id?** Cross-check the exact `session_id` in the URL before trusting a customer/team screenshot's content against your state-file findings — a mismatched screenshot sent an entire investigation down the wrong path once (confused a different, unrelated conversation's real content for this one's, which had a much shorter, quieter real history). +5. **Was it a genuine metadata lag, not a bug?** `active.last`/`updated_at` can lag real message timestamps — confirmed for real on at least one email-origin conversation. If the state file's timestamp looks "too old" compared to what a human says actually happened, this is the most likely explanation, and no code fix resolves it except computing freshness from real message timestamps directly (which `checkedThroughAt` now does, post-2026-09-29). + +## What forces an answer right now, regardless of root cause + +`crisp-investigate-now.yml` (`workflow_dispatch`, input = session_id) bypasses every guard above entirely — it always routes and investigates, with real side effects (may file a real issue, always posts a note). Use it once you've understood *why* the normal path didn't fire, not as a substitute for finding out. + +## After finding the cause + +If it points at a real code gap (not "working as designed" or "genuine metadata limitation"), fix the root cause in `crisp-classify.mjs`, not just this one session — and check whether `crisp-dedupe-active.mjs` or any other script shares the same class of bug before calling it done (as of 2026-09-29, it does: no freshness gate at all, a known but unfixed gap — see `CHANGELOG.md`). diff --git a/.claude/skills/verify-github-actions-change/SKILL.md b/.claude/skills/verify-github-actions-change/SKILL.md index 66011fb..2a2afdd 100644 --- a/.claude/skills/verify-github-actions-change/SKILL.md +++ b/.claude/skills/verify-github-actions-change/SKILL.md @@ -20,3 +20,12 @@ This also compounds with **GitHub Actions' own indexing lag**: a newly merged wo ## When this matters most Any time you've just made a change and the *very next* verification check comes back looking wrong — pause before concluding the fix failed. The two most common causes in this system have both been caching/indexing lag, not a real regression, often enough that it's worth ruling out first. + +## A different trap: a stale local branch name silently absorbing your push + +`git push origin ` with no local branch checked out under that exact name resolves to *whatever ref git finds matching that name* — including a leftover local branch from an **earlier, already-merged, remote-deleted** PR that used the same name. Confirmed for real: a branch name was reused for a second, larger fix; the push silently succeeded, but against the old stale local ref, not the new work; the resulting PR's merge commit had **zero file changes** — it was byte-identical to the already-merged first PR, title and body included, because that's genuinely all it contained. + +**The fix, every time, no exceptions:** +- After pushing a branch you intend to open a PR from, verify its *actual remote content* directly — `gh api repos/OWNER/REPO/contents/PATH?ref=BRANCH` for the specific file(s) you changed, checking for the actual new content, not just that the API call succeeds. +- After a PR merges, check the merge commit's own `files` list (`gh api repos/OWNER/REPO/commits/SHA --jq '.files'`) is non-empty and matches what you expected — an empty or unexpected `files` list on a merge you just made is the tell that something upstream of the PR was already wrong. +- Delete a branch locally (not just check out something else) once its PR is merged, so its name can never be silently reused by a later `git push origin ` from a fresh local branch that happens to share it. diff --git a/.github/workflows/diagnose-conversation-state.yml b/.github/workflows/diagnose-conversation-state.yml deleted file mode 100644 index 6c5946b..0000000 --- a/.github/workflows/diagnose-conversation-state.yml +++ /dev/null @@ -1,28 +0,0 @@ -name: Diagnose conversation state (temporary, read-only) -on: - workflow_dispatch: - inputs: - account_key: { required: true, type: string } - session_id: { required: true, type: string } - cutoff_ms: { required: false, type: string, default: "0" } -jobs: - diagnose: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 - with: { node-version: "20" } - - run: node scripts/diagnose-message-timestamps.mjs - env: - ACCOUNT_KEY: ${{ inputs.account_key }} - TARGET_SESSION_ID: ${{ inputs.session_id }} - CUTOFF_MS: ${{ inputs.cutoff_ms }} - CRISP_USER_REGISTRATION_IDENTIFIER: ${{ secrets.CRISP_USER_REGISTRATION_IDENTIFIER }} - CRISP_USER_REGISTRATION_KEY: ${{ secrets.CRISP_USER_REGISTRATION_KEY }} - CRISP_USER_REGISTRATION_WEBSITE_ID: ${{ vars.CRISP_USER_REGISTRATION_WEBSITE_ID }} - CRISP_EVEREST_FORMS_IDENTIFIER: ${{ secrets.CRISP_EVEREST_FORMS_IDENTIFIER }} - CRISP_EVEREST_FORMS_KEY: ${{ secrets.CRISP_EVEREST_FORMS_KEY }} - CRISP_EVEREST_FORMS_WEBSITE_ID: ${{ vars.CRISP_EVEREST_FORMS_WEBSITE_ID }} - CRISP_THEMEGRILL_IDENTIFIER: ${{ secrets.CRISP_THEMEGRILL_IDENTIFIER }} - CRISP_THEMEGRILL_KEY: ${{ secrets.CRISP_THEMEGRILL_KEY }} - CRISP_THEMEGRILL_WEBSITE_ID: ${{ vars.CRISP_THEMEGRILL_WEBSITE_ID }} diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..d48dc37 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,28 @@ +# Changelog + +Dated entries for notable fixes, redesigns, and policy changes to the bot. See `SETUP.md` (Phase 1: pr-build-zip) and `PHASE2-SETUP.md` (Phase 2: Crisp triage) for the design rationale these entries reference. + +## 2026-09-30 — maintainability pass + +- Added root `CLAUDE.md` indexing `.claude/skills/`, the doc split, and the hard-won rules below — none of this was being discovered by an agent working from a fresh clone instead of a checkout with `.claude/` loaded, which is exactly how the 2026-09-29 PR #93 mistake happened in the first place. +- Split the changelog out of `PHASE2-SETUP.md` into this file. +- Added `debug-crisp-triage-not-investigated` skill (the elimination checklist from the 2026-09-29 investigations, so a future "why didn't this fire" question doesn't start from zero). +- Extended `verify-github-actions-change` with the stale-branch-name lesson from PR #93. +- Removed 3 temporary diagnostic scripts (`diagnose-active-cap.mjs`, `diagnose-conversation-state.mjs`, `diagnose-message-timestamps.mjs`) and their workflow, left on master from the 2026-09-29 investigations and missed in that day's own cleanup pass — caught while reviewing `scripts/` for this same maintainability work, not before. +- Corrected `PHASE2-SETUP.md`'s file listing: `state/investigated.json`'s description still said it gates re-investigation, which stopped being true the moment `checkedThroughAt` shipped. `state/escalated.json` (the one that actually matters now) wasn't listed there at all. + +## 2026-09-29 — permanent-block bug fix, real-timestamp freshness, cron/debug tooling, QA labels + +Two real chat sessions (`session_2fc63232`, `session_bd0acc7b`) stopped producing issues despite genuine, never-addressed problems. Root cause and fix: + +- `investigated.has(session_id)` / `escalated[session_id].autoEscalated` were permanent flags that never cleared, so a session auto-escalated once could never be auto-escalated again, ever — even after resolving and reopening with a brand new problem weeks later. Replaced with `checkedThroughAt` (timestamp from real fetched messages, not conversation metadata) — see § 4c of `PHASE2-SETUP.md`. +- A reopen with nothing new since a (possibly spurious) resolve took the reopen branch, found an empty delta, and skipped forever instead of falling through to the independently-stale check. Now falls back to a full-history stale check. +- Along the way, confirmed `active.last`/`updated_at` can lag real message activity for at least some conversations — the reason `checkedThroughAt` is deliberately message-based, not metadata-based. The 12h–720h staleness *window* itself still uses that metadata and is a known, unfixed residual gap (see the ⚠️ warning in `PHASE2-SETUP.md` § 4c). +- `seed-escalated.mjs` had the same metadata-vs-real-timestamp bug on first attempt (seeded from `updated_at`, which undercounted real activity for a large fraction of a backlog and caused it to immediately re-fire) — fixed to fetch real messages instead, and given a `--force` flag to recompute existing entries. +- All 3 accounts' current backlog was re-seeded with real-timestamp `checkedThroughAt` on 2026-09-29, so this fix applies to new activity going forward rather than re-litigating months of history in one burst. +- Added `skip_dedupe_check` (workflow_dispatch input on `crisp-triage.yml`) to skip the slow "check active conversations for duplicates" step (~15–20 min, unrelated to classify logic) for fast manual debugging. **Caveat learned the hard way: if `matrix` isn't empty, the downstream `investigate` job still auto-fires immediately after classify finishes** — a "fast test" run isn't safe to leave unattended once real conversations match, and must be watched/cancelled if you don't want real issues filed. +- Added `manual-qa-required`/`qa-verified` labels to AI-filed issues, decided per-item from the agent's own confidence score and reproduction method (see `prompts/crisp-triage-agent.md`). In practice, almost everything lands on `manual-qa-required` — this pipeline only has the target repo checked out, no live WordPress site to actually execute against, so genuine direct reproduction is the exception, not the rule. +- Cron cadence changed several times this session while chasing a real multi-day pattern of scheduled runs silently not firing (no error, no run object — confirmed not a billing/budget block by checking actual usage data; still unconfirmed root cause). Settled on `17 */3 * * *` (offset off the exact hour, a documented mitigation, not a proven fix) as of this writing. +- Found (not yet fixed): `crisp-dedupe-active.mjs` has no freshness gate at all — it re-runs a real OpenAI call for every still-open, still-unmatched conversation on every single run, forever, for as long as it stays open. Same class of bug as the `checkedThroughAt` fix above, just not yet applied here. Also calls `listOpenIssues(repo)` once per conversation instead of once per repo per run. + +PRs: themegrill/.github#91, #92 (superseded, see below), #93 (accidentally a no-op — opened from a stale branch, merge commit had zero file changes; see the `verify-github-actions-change` skill for the lesson), #94, #95 (the real fix), #96, #97 (QA labels), #98 (cron + docs), #99 (cron offset). diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..75909bf --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,25 @@ +# tg-autopilot (themegrill/.github) + +This repo hosts ThemeGrill's shared bot automation: reusable GitHub Actions workflows (PR build-zip, Copilot review) and the Crisp → AI → GitHub issue triage pipeline. Machine user: `tg-autopilot`. + +**Before doing anything here, check `.claude/skills/` first.** There is very likely already a skill covering the task — onboarding a repo, debugging a specific failure mode, verifying a change actually took effect. Re-deriving one of these from scratch instead of using the existing skill is exactly how repeat mistakes happen; see `CHANGELOG.md` 2026-09-29 for one that cost real debugging time. + +**Work from a persistent local checkout, not a disposable clone.** A fresh `git clone` into a scratch directory (e.g. `/tmp`) never has `.claude/skills/` loaded, which defeats the point of this file and everything below it. + +## Map + +- `SETUP.md` — Phase 1 (pr-build-zip, Copilot review): one-time credentials/setup. +- `PHASE2-SETUP.md` — Phase 2 (Crisp triage): credentials/setup **and** the actual design policy — § 4c/4d explain what triggers an investigation and why, and are the first thing to read before touching `crisp-classify.mjs`. +- `CHANGELOG.md` — dated entries for notable fixes/redesigns. Read the most recent entries before assuming you understand current behavior; policy here has changed more than once. +- `.claude/skills/` — task- and symptom-oriented playbooks (onboarding, debugging, verification). Check this before improvising. +- `scripts/` — all Node scripts, most with dense "why" comments at the point of the actual gotcha, not just a docstring at the top. Read the comment before changing the line it's attached to. +- `state/` — committed JSON state (`cursor.json`, `escalated.json`, `resolved-seen.json`, `investigated.json`, `active-notified.json`). These are data, not config — don't hand-edit without understanding what reads them first. +- `config/inbox-to-repo.json` — Crisp account → GitHub repo routing. +- `prompts/crisp-triage-agent.md` — the Stage 2 investigation agent's actual instructions. + +## Hard-won rules, worth repeating + +- **Verify a pushed branch's actual remote content** (`gh api repos/OWNER/REPO/contents/PATH?ref=BRANCH`) before opening a PR from it — a stale local branch with the same name as an earlier, already-merged one can silently absorb your push instead of your new commits. This produced a real no-op PR (#93) that looked merged and normal until someone checked the diff. +- **A "fast test" dispatch of `crisp-triage.yml` (`skip_dedupe_check: true`) is not safe to leave unattended** if you're not sure the matrix will be empty — the `investigate` job auto-fires immediately after classify finishes, checking out real repos and potentially filing real issues. Check the classify step's log for `-> escalated to` lines before walking away. +- **`gh api`/`gh` can serve a cached response for a `GET` shortly after a mutation you just made** — see the `verify-github-actions-change` skill. +- Conversation-level metadata (`active.last`, `updated_at`) can lag real message activity — confirmed for real on at least one email-origin conversation. Any "has something new happened?" check in this codebase should be based on actual fetched message timestamps, never on this metadata alone (see `PHASE2-SETUP.md` § 4c's ⚠️ warning). diff --git a/PHASE2-SETUP.md b/PHASE2-SETUP.md index a1c1251..00f929d 100644 --- a/PHASE2-SETUP.md +++ b/PHASE2-SETUP.md @@ -8,7 +8,8 @@ Files added for this phase, all in `themegrill/.github`: - `scripts/crisp-fetch-transcript.mjs`, `scripts/build-prompt.mjs`, `scripts/crisp-post-note.mjs` — Stage 2 helpers - `prompts/crisp-triage-agent.md` — the agent's instructions - `state/cursor.json` — persisted "last checked" timestamp -- `state/investigated.json` — session_ids that already went through a full Stage 2 investigation at least once, so a conversation that resolves, gets reopened by the customer, and resolves again doesn't get reinvestigated (and re-noted) every cycle +- `state/escalated.json` — per-session `checkedThroughAt` (a real-message timestamp, not conversation metadata) and `manualNoteCount`; this is what actually gates re-investigation — see § 4c +- `state/investigated.json` — a write-only audit trail of every session_id that has ever completed a full Stage 2 investigation, from any path. Does **not** gate anything as of 2026-09-29 (see `CHANGELOG.md`) — kept for visibility only - `config/inbox-to-repo.json` — Crisp inbox → GitHub repo mapping (**you must populate this**) ## 1. Crisp API tokens — one per account, not one total @@ -103,16 +104,4 @@ ThemeIsle's own numbers: $0.0003 per conversation when Stage 1 (or a quick Stage ## Changelog -**2026-09-29 — permanent-block bug fix, real-timestamp freshness, cron/debug tooling** - -Two real chat sessions (`session_2fc63232`, `session_bd0acc7b`) stopped producing issues despite genuine, never-addressed problems. Root cause and fix: - -- `investigated.has(session_id)` / `escalated[session_id].autoEscalated` were permanent flags that never cleared, so a session auto-escalated once could never be auto-escalated again, ever — even after resolving and reopening with a brand new problem weeks later. Replaced with `checkedThroughAt` (timestamp from real fetched messages, not conversation metadata) — see § 4c above. -- A reopen with nothing new since a (possibly spurious) resolve took the reopen branch, found an empty delta, and skipped forever instead of falling through to the independently-stale check. Now falls back to a full-history stale check. -- Along the way, confirmed `active.last`/`updated_at` can lag real message activity for at least some conversations — the reason `checkedThroughAt` is deliberately message-based, not metadata-based. The 12h–720h staleness *window* itself still uses that metadata and is a known, unfixed residual gap (see the ⚠️ warning in § 4c). -- `seed-escalated.mjs` had the same metadata-vs-real-timestamp bug on first attempt (seeded from `updated_at`, which undercounted real activity for a large fraction of a backlog and caused it to immediately re-fire) — fixed to fetch real messages instead, and given a `--force` flag to recompute existing entries. -- All 3 accounts' current backlog was re-seeded with real-timestamp `checkedThroughAt` on 2026-09-29, so today's fix applies to new activity going forward rather than re-litigating months of history in one burst. -- Added `skip_dedupe_check` (workflow_dispatch input on `crisp-triage.yml`) to skip the slow "check active conversations for duplicates" step (~15–20 min, unrelated to classify logic) for fast manual debugging. **Caveat learned the hard way: if `matrix` isn't empty, the downstream `investigate` job still auto-fires immediately after classify finishes** — a "fast test" run isn't safe to leave unattended once real conversations match, and must be watched/cancelled if you don't want real issues filed. -- Cron cadence changed a few times this session while investigating a low signal-to-noise cadence: 5h (original) → 3h → 4h → 3h (temporary, as of this writing, to watch results more closely for a day). - -PRs: themegrill/.github#91, #92 (superseded, see below), #93 (accidentally a no-op — opened from a stale branch, merge commit had zero file changes, learned to always verify a branch's actual pushed content before opening a PR from it), #94, #95 (the real fix), #96. +Moved to `CHANGELOG.md` at the repo root. diff --git a/README.md b/README.md index 825b1c3..84f0a98 100644 --- a/README.md +++ b/README.md @@ -24,8 +24,9 @@ We use a machine user instead of a GitHub App because it keeps one identity acro | `propagate-shared-secret` | adding a brand-new secret/variable that needs to reach every private repo in both orgs | | `transfer-repo-across-orgs` | moving a repo between `wpeverest` and `themegrill` | | `debug-crisp-401-errors` | crisp-triage is failing with a Crisp API auth error | +| `debug-crisp-triage-not-investigated` | a specific conversation should have been auto-investigated and wasn't | | `write-safe-bot-workflow` | writing or editing any comment-triggered workflow | -| `verify-github-actions-change` | a change you just made looks like it didn't take effect | +| `verify-github-actions-change` | a change you just made looks like it didn't take effect, or before opening a PR from a branch you just pushed | ## Onboarding a new repo @@ -58,3 +59,4 @@ Both live as org secrets on `wpeverest`, since that's where every workflow here - A failed "Investigate" job in Crisp triage can be safely re-run on its own from the run page (**Re-run job**) — it retries the same conversation, no state to reset first. See [PHASE2-SETUP.md § 4d](PHASE2-SETUP.md#4d-investigate-job-concurrency-and-openai-rate-limits) for why this happens. - Committed state (`state/*.json`) is the pipeline's only memory of what it's already processed. If a run looks like it's reprocessing something it shouldn't, check whether "Commit advanced state" actually succeeded on the *prior* run, not just whether the investigation itself did. - If a mutation you just made (a secret, a workflow file, a PR reviewer) looks like it didn't take — see `verify-github-actions-change` before assuming the fix failed. +- Check [CHANGELOG.md](CHANGELOG.md) for recent policy changes before assuming current behavior matches what an older doc section (or your own memory of the code) describes — this pipeline's escalation policy has changed more than once. diff --git a/scripts/diagnose-active-cap.mjs b/scripts/diagnose-active-cap.mjs deleted file mode 100644 index bfc7b59..0000000 --- a/scripts/diagnose-active-cap.mjs +++ /dev/null @@ -1,21 +0,0 @@ -#!/usr/bin/env node -import { credsForAccount, crispGet } from "./crisp-client.mjs"; -const { ACCOUNT_KEY, TARGET_SESSION_ID } = process.env; -async function main() { - const creds = credsForAccount(ACCOUNT_KEY); - const conversations = []; - let rank = -1; - for (let page = 1; page <= 50; page++) { - const params = new URLSearchParams({ filter_not_resolved: "true", order_date_updated: "1" }); - const { data } = await crispGet(creds, `/website/${creds.websiteId}/conversations/${page}?${params}`); - if (!data || data.length === 0) break; - conversations.push(...data); - const idx = data.findIndex((c) => c.session_id === TARGET_SESSION_ID); - if (idx !== -1 && rank === -1) rank = conversations.length - data.length + idx + 1; - if (data.length < 20) break; - } - console.log(`Total unresolved fetched: ${conversations.length}`); - console.log(`Current 10-page/200-item cap covers ranks 1-200.`); - console.log(rank === -1 ? `NOT FOUND within ${conversations.length} fetched.` : `rank ${rank} -- ${rank <= 200 ? "WITHIN" : "OUTSIDE"} the 200-item cap.`); -} -main().catch((e) => { console.error(e.message); process.exit(1); }); diff --git a/scripts/diagnose-conversation-state.mjs b/scripts/diagnose-conversation-state.mjs deleted file mode 100644 index 7faf740..0000000 --- a/scripts/diagnose-conversation-state.mjs +++ /dev/null @@ -1,9 +0,0 @@ -#!/usr/bin/env node -import { credsForAccount, crispGet } from "./crisp-client.mjs"; -const { ACCOUNT_KEY, TARGET_SESSION_ID } = process.env; -async function main() { - const creds = credsForAccount(ACCOUNT_KEY); - const { data } = await crispGet(creds, `/website/${creds.websiteId}/conversation/${TARGET_SESSION_ID}`); - console.log(JSON.stringify({ state: data.state, created_at: data.created_at, updated_at: data.updated_at, active: data.active }, null, 2)); -} -main().catch((e) => { console.error(e.message); process.exit(1); }); diff --git a/scripts/diagnose-message-timestamps.mjs b/scripts/diagnose-message-timestamps.mjs deleted file mode 100644 index d259b5d..0000000 --- a/scripts/diagnose-message-timestamps.mjs +++ /dev/null @@ -1,16 +0,0 @@ -#!/usr/bin/env node -// Read-only, no content/sender exposed -- just type+timestamp, to check -// whether any message is newer than a given cutoff. -import { credsForAccount, crispGet } from "./crisp-client.mjs"; -const { ACCOUNT_KEY, TARGET_SESSION_ID, CUTOFF_MS } = process.env; -async function main() { - const creds = credsForAccount(ACCOUNT_KEY); - const { data } = await crispGet(creds, `/website/${creds.websiteId}/conversation/${TARGET_SESSION_ID}/messages`); - const cutoff = Number(CUTOFF_MS); - console.log(`Total messages: ${data.length}`); - console.log(`Cutoff: ${cutoff}`); - for (const m of data) { - console.log(JSON.stringify({ type: m.type, timestamp: m.timestamp, newer_than_cutoff: (m.timestamp ?? 0) > cutoff })); - } -} -main().catch((e) => { console.error(e.message); process.exit(1); }); From ca7e134ec2fed6f4096743a7383d6f77dfb06700 Mon Sep 17 00:00:00 2001 From: Aashil Date: Wed, 30 Sep 2026 10:44:59 +0545 Subject: [PATCH 2/2] docs: make CHANGELOG.md short and human-readable Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d48dc37..bd09cfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,28 +1,28 @@ # Changelog -Dated entries for notable fixes, redesigns, and policy changes to the bot. See `SETUP.md` (Phase 1: pr-build-zip) and `PHASE2-SETUP.md` (Phase 2: Crisp triage) for the design rationale these entries reference. +Short, dated summary of notable fixes and changes. For the full "why," see `PHASE2-SETUP.md` (Crisp triage design) or the linked PRs. -## 2026-09-30 — maintainability pass +## 2026-09-30 — housekeeping -- Added root `CLAUDE.md` indexing `.claude/skills/`, the doc split, and the hard-won rules below — none of this was being discovered by an agent working from a fresh clone instead of a checkout with `.claude/` loaded, which is exactly how the 2026-09-29 PR #93 mistake happened in the first place. -- Split the changelog out of `PHASE2-SETUP.md` into this file. -- Added `debug-crisp-triage-not-investigated` skill (the elimination checklist from the 2026-09-29 investigations, so a future "why didn't this fire" question doesn't start from zero). -- Extended `verify-github-actions-change` with the stale-branch-name lesson from PR #93. -- Removed 3 temporary diagnostic scripts (`diagnose-active-cap.mjs`, `diagnose-conversation-state.mjs`, `diagnose-message-timestamps.mjs`) and their workflow, left on master from the 2026-09-29 investigations and missed in that day's own cleanup pass — caught while reviewing `scripts/` for this same maintainability work, not before. -- Corrected `PHASE2-SETUP.md`'s file listing: `state/investigated.json`'s description still said it gates re-investigation, which stopped being true the moment `checkedThroughAt` shipped. `state/escalated.json` (the one that actually matters now) wasn't listed there at all. +- Added `CLAUDE.md` so future work here starts from the existing skills/docs instead of re-discovering things from scratch. +- Split this changelog out of `PHASE2-SETUP.md` into its own file. +- Added a skill for debugging "why wasn't this conversation investigated." +- Removed 3 leftover debug scripts that should've been cleaned up on 09-29. +- Fixed a couple of outdated file descriptions in `PHASE2-SETUP.md`. -## 2026-09-29 — permanent-block bug fix, real-timestamp freshness, cron/debug tooling, QA labels +PR: #100 -Two real chat sessions (`session_2fc63232`, `session_bd0acc7b`) stopped producing issues despite genuine, never-addressed problems. Root cause and fix: +## 2026-09-29 — fixed conversations getting permanently stuck -- `investigated.has(session_id)` / `escalated[session_id].autoEscalated` were permanent flags that never cleared, so a session auto-escalated once could never be auto-escalated again, ever — even after resolving and reopening with a brand new problem weeks later. Replaced with `checkedThroughAt` (timestamp from real fetched messages, not conversation metadata) — see § 4c of `PHASE2-SETUP.md`. -- A reopen with nothing new since a (possibly spurious) resolve took the reopen branch, found an empty delta, and skipped forever instead of falling through to the independently-stale check. Now falls back to a full-history stale check. -- Along the way, confirmed `active.last`/`updated_at` can lag real message activity for at least some conversations — the reason `checkedThroughAt` is deliberately message-based, not metadata-based. The 12h–720h staleness *window* itself still uses that metadata and is a known, unfixed residual gap (see the ⚠️ warning in `PHASE2-SETUP.md` § 4c). -- `seed-escalated.mjs` had the same metadata-vs-real-timestamp bug on first attempt (seeded from `updated_at`, which undercounted real activity for a large fraction of a backlog and caused it to immediately re-fire) — fixed to fetch real messages instead, and given a `--force` flag to recompute existing entries. -- All 3 accounts' current backlog was re-seeded with real-timestamp `checkedThroughAt` on 2026-09-29, so this fix applies to new activity going forward rather than re-litigating months of history in one burst. -- Added `skip_dedupe_check` (workflow_dispatch input on `crisp-triage.yml`) to skip the slow "check active conversations for duplicates" step (~15–20 min, unrelated to classify logic) for fast manual debugging. **Caveat learned the hard way: if `matrix` isn't empty, the downstream `investigate` job still auto-fires immediately after classify finishes** — a "fast test" run isn't safe to leave unattended once real conversations match, and must be watched/cancelled if you don't want real issues filed. -- Added `manual-qa-required`/`qa-verified` labels to AI-filed issues, decided per-item from the agent's own confidence score and reproduction method (see `prompts/crisp-triage-agent.md`). In practice, almost everything lands on `manual-qa-required` — this pipeline only has the target repo checked out, no live WordPress site to actually execute against, so genuine direct reproduction is the exception, not the rule. -- Cron cadence changed several times this session while chasing a real multi-day pattern of scheduled runs silently not firing (no error, no run object — confirmed not a billing/budget block by checking actual usage data; still unconfirmed root cause). Settled on `17 */3 * * *` (offset off the exact hour, a documented mitigation, not a proven fix) as of this writing. -- Found (not yet fixed): `crisp-dedupe-active.mjs` has no freshness gate at all — it re-runs a real OpenAI call for every still-open, still-unmatched conversation on every single run, forever, for as long as it stays open. Same class of bug as the `checkedThroughAt` fix above, just not yet applied here. Also calls `listOpenIssues(repo)` once per conversation instead of once per repo per run. +Some conversations stopped getting investigated at all, even with new, real problems. -PRs: themegrill/.github#91, #92 (superseded, see below), #93 (accidentally a no-op — opened from a stale branch, merge commit had zero file changes; see the `verify-github-actions-change` skill for the lesson), #94, #95 (the real fix), #96, #97 (QA labels), #98 (cron + docs), #99 (cron offset). +- **Root cause**: once a conversation was checked, it could never be auto-checked again — a one-way flag, no way to reset it. +- **Fix**: now tracks the actual last-seen message time instead, so it re-checks whenever something genuinely new comes in. +- Also fixed: a reopened conversation with nothing new in it used to get skipped entirely, instead of still being checked for staleness. +- Re-seeded all 3 Crisp accounts so this fix applies going forward, not to the whole backlog at once. +- Added `manual-qa-required` / `qa-verified` labels on AI-filed issues. +- Added a quick-test mode for `crisp-triage` (skips the slow duplicate-check step). +- Nudged the schedule off the exact hour (`17 */3 * * *`) — scheduled runs were silently not firing; this is a likely mitigation, not a confirmed fix. +- Known remaining issue: the duplicate-check script has no "don't recheck if nothing changed" logic yet, so it re-runs OpenAI checks on the same conversations every time. Not fixed yet. + +PRs: #91, #92 (superseded), #93 (accidental no-op, see the `verify-github-actions-change` skill), #94, #95 (the real fix), #96, #97, #98, #99