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..bd09cfd --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,28 @@ +# Changelog + +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 — housekeeping + +- 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`. + +PR: #100 + +## 2026-09-29 — fixed conversations getting permanently stuck + +Some conversations stopped getting investigated at all, even with new, real problems. + +- **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 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); });