feat(flow-webhook-triage): add AppSignal error triage plugin - #8
Open
AmishaBisht wants to merge 5 commits into
Open
feat(flow-webhook-triage): add AppSignal error triage plugin#8AmishaBisht wants to merge 5 commits into
AmishaBisht wants to merge 5 commits into
Conversation
Adds a plugin that triages Glific flow-webhook errors: pulls incidents from
the flow_webhooks and flow_webhook_config_errors namespaces, diagnoses each
against the glific/glific code, and upserts a row into a shared Google Sheet.
The sheet is the point. A single incident is noise; a month of diagnosed ones
shows which failures are worth engineering away — what is accumulating in the
:unknown bucket, and which config errors repeat across enough orgs to be a
product defect rather than a support ticket.
Notes on the design:
- Dedup key is incident_id, enforced server-side under a lock. Recurrence
updates the row and bumps times_seen rather than appending, so repeat
offenders sort to the top and concurrent runs cannot double-write.
- Sheet writes go through an Apps Script Web App deployed by the sheet owner
("execute as: me"), so no Google credentials are shared. Sheets API writes
require OAuth regardless of link-sharing, so a hardcoded sheet id alone
cannot work.
- Incident data comes from the AppSignal GraphQL API, which needs a Personal
API token. The push key the app uses to send errors is write-only and 401s.
- The classification rules in references/error-taxonomy.md are read from the
code (errors.ex, error_type.ex, error_reporter.ex, instrumentation.ex), not
inferred. Notably rate_limited/service_unavailable are :system, not
suppressed, because there is no retry.
Code.gs cannot run outside Google, so test-upsert.js stubs SpreadsheetApp and
exercises it in Node (17 checks). It already caught a real bug: an in-batch
duplicate id took the update path and wrote to row -1, failing the batch.
The GraphQL query itself is unverified against a live token — an introspect
command is included to correct it on first run, and the limitation is flagged
in the script and the setup doc.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds and registers the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs`:
- Around line 169-179: Update the recurrence patch construction in the
existing-row flow so last_seen, occurrences, and http_status are included only
when present in the incoming payload, preserving their existing sheet values
when omitted. Use nullish checks rather than truthiness checks so valid zero
values are retained, while continuing to update the other recurrence fields.
- Around line 169-179: Sanitize every untrusted string value before it reaches
either the patch loop’s setValue call or any setValues call in the surrounding
write flow, prefixing values that could be interpreted as formulas while
preserving non-string values and existing data semantics. Update the shared
write/sanitization path rather than only the visible patch object, and use it
consistently for all AppSignal fields.
- Around line 47-50: Make expectedToken_ treat a missing TRIAGE_TOKEN as a
configuration error instead of returning an open state, and update the request
validation around Code.gs lines 127-130 to reject requests unless the configured
token matches. In
plugins/flow-webhook-triage/skills/triage-flow-webhooks/references/sheet-setup.md
lines 34-36, remove the documented fail-open/test exception.
- Around line 160-180: Update the batch-processing logic around pendingIds and
the index branch to track every processed incident ID, including IDs found in
index, before performing updates. Skip subsequent occurrences of any ID within
the same batch so an existing incident is updated and times_seen incremented
only once. Add a regression test covering duplicate persisted IDs in one batch.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs`:
- Around line 42-60: Update the response handling in fetchIncidents so the
existing 401/authentication-specific error check runs first, then reject every
response where res.ok is false, including JSON 403, 429, and 5xx responses
without body.errors. Preserve the existing GraphQL error handling and successful
body.data return path.
- Around line 36-40: Bound both external requests with a finite abort timeout:
update fetch-incidents.mjs around the fetch call to pass an AbortController
signal, and apply the same timeout to the fetch call in append-to-sheet.mjs.
Ensure append-to-sheet.mjs reports timeout failures clearly while preserving
existing error handling for other failures.
- Around line 118-123: Update the output object constructed in the incidents
loop of fetch-incidents to map each returned incident’s id to the
downstream-required incident_id field. Preserve the namespace fallback and all
other incident properties while ensuring every emitted row includes incident_id.
- Around line 118-121: The incident retrieval loop around gql in
fetch-incidents.mjs must not treat a 100-item response as complete: add
pagination using the API’s supported cursor/offset fields and continue fetching
until a page is shorter than the limit, while preserving the existing namespace
mapping and output behavior.
In `@plugins/flow-webhook-triage/skills/triage-flow-webhooks/SKILL.md`:
- Around line 65-67: Update the incident capture guidance in the
triage-flow-webhooks skill to explicitly map AppSignal fields to the downstream
sheet columns: incident number to incident_id, count to occurrences,
organization_id to org_id, and include appsignal_url. Preserve the existing
exception, timing, and tag fields while naming the exact JSON keys expected by
Code.gs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 675c1e47-93eb-4904-a841-982c4dd73d77
📒 Files selected for processing (10)
.claude-plugin/marketplace.jsonplugins/flow-webhook-triage/.claude-plugin/plugin.jsonplugins/flow-webhook-triage/README.mdplugins/flow-webhook-triage/skills/triage-flow-webhooks/SKILL.mdplugins/flow-webhook-triage/skills/triage-flow-webhooks/references/error-taxonomy.mdplugins/flow-webhook-triage/skills/triage-flow-webhooks/references/sheet-setup.mdplugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/append-to-sheet.mjsplugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gsplugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/test-upsert.jsplugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs
Comment on lines
+47
to
+50
| /** Shared secret, set in Script Properties. Absent = open, which is fine for a private URL. */ | ||
| function expectedToken_() { | ||
| return PropertiesService.getScriptProperties().getProperty('TRIAGE_TOKEN'); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail closed when TRIAGE_TOKEN is not configured.
The public Web App currently grants unauthenticated write access whenever the property is accidentally omitted.
plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L47-L50: treat a missing token as a configuration error.plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L127-L130: reject requests unless a configured token matches.plugins/flow-webhook-triage/skills/triage-flow-webhooks/references/sheet-setup.md#L34-L36: remove the documented fail-open/test exception.
📍 Affects 2 files
plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L47-L50(this comment)plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L127-L130plugins/flow-webhook-triage/skills/triage-flow-webhooks/references/sheet-setup.md#L34-L36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs`
around lines 47 - 50, Make expectedToken_ treat a missing TRIAGE_TOKEN as a
configuration error instead of returning an open state, and update the request
validation around Code.gs lines 127-130 to reject requests unless the configured
token matches. In
plugins/flow-webhook-triage/skills/triage-flow-webhooks/references/sheet-setup.md
lines 34-36, remove the documented fail-open/test exception.
Comment on lines
+169
to
+179
| const patch = { | ||
| last_seen: row.last_seen || '', | ||
| last_run_date: today, | ||
| times_seen: prev + 1, | ||
| occurrences: row.occurrences || '', | ||
| http_status: row.http_status || '', | ||
| }; | ||
| for (const key of Object.keys(patch)) { | ||
| const col = COLUMNS.indexOf(key); | ||
| if (col >= 0) sh.getRange(existing, col + 1).setValue(patch[key]); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve existing values when recurrence fields are omitted.
The client only requires incident_id, but omitted last_seen, occurrences, or http_status are written as empty strings. This can erase the watermark and other previously captured data. Only patch fields present in the payload, using nullish checks so valid zero values survive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs`
around lines 169 - 179, Update the recurrence patch construction in the
existing-row flow so last_seen, occurrences, and http_status are included only
when present in the incoming payload, preserving their existing sheet values
when omitted. Use nullish checks rather than truthiness checks so valid zero
values are retained, while continuing to update the other recurrence fields.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs'
echo '### Relevant excerpt (around lines 130-230)'
sed -n '130,230p' "$FILE" | cat -n
echo
echo '### All setValue/setValues occurrences in Code.gs'
rg -n 'setValue|setValues' "$FILE"Repository: glific/claude-plugins
Length of output: 3414
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs'
echo '### Sanitization helpers / formula handling in Code.gs'
rg -n "sanitize|formula|escape|apostrophe|='|^\s*const .*safe|replace\(" "$FILE" || true
echo
echo '### Full file length'
wc -l "$FILE"
echo
echo '### File header / helper definitions'
sed -n '1,140p' "$FILE" | cat -nRepository: glific/claude-plugins
Length of output: 5722
Escape untrusted Sheet values before writing them.
AppSignal fields are written raw via both setValue() and setValues(), so any string starting with = can become a formula in Google Sheets. Prefix/sanitize all string cell values before both write paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs`
around lines 169 - 179, Sanitize every untrusted string value before it reaches
either the patch loop’s setValue call or any setValues call in the surrounding
write flow, prefixing values that could be interpreted as formulas while
preserving non-string values and existing data semantics. Update the shared
write/sanitization path rather than only the visible patch object, and use it
consistently for all AppSignal fields.
Comment on lines
+36
to
+40
| const res = await fetch(`${ENDPOINT}?token=${encodeURIComponent(TOKEN)}`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ query, variables }), | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound both external requests with timeouts.
A stalled AppSignal or Apps Script connection can hang the CLI indefinitely.
plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs#L36-L40: attach an abort signal with a finite timeout.plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/append-to-sheet.mjs#L38-L40: apply the same timeout and report timeout failures clearly.
📍 Affects 2 files
plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs#L36-L40(this comment)plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/append-to-sheet.mjs#L38-L40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs`
around lines 36 - 40, Bound both external requests with a finite abort timeout:
update fetch-incidents.mjs around the fetch call to pass an AbortController
signal, and apply the same timeout to the fetch call in append-to-sheet.mjs.
Ensure append-to-sheet.mjs reports timeout failures clearly while preserving
existing error handling for other failures.
Comment on lines
+42
to
+60
| const text = await res.text(); | ||
| let body; | ||
| try { | ||
| body = JSON.parse(text); | ||
| } catch { | ||
| die(`non-JSON response (HTTP ${res.status}): ${text.slice(0, 300)}`); | ||
| } | ||
|
|
||
| if (res.status === 401 || body.errors?.some((e) => /authenticat/i.test(e.message || ''))) { | ||
| die( | ||
| 'AppSignal rejected the token (401).\n' + | ||
| ' This is almost always a PUSH key rather than a Personal API token.\n' + | ||
| ' Push keys live on the app settings page and can only SEND data.\n' + | ||
| ' Get a Personal API token from your AppSignal *user* settings, and put it\n' + | ||
| ' in APPSIGNAL_API_KEY.' | ||
| ); | ||
| } | ||
| if (body.errors) die(`GraphQL: ${JSON.stringify(body.errors).slice(0, 500)}`); | ||
| return body.data; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject every unsuccessful HTTP response.
A JSON 403, 429, or 5xx response without body.errors returns undefined; fetchIncidents() then silently reports zero incidents. Handle the 401 message first, then fail on !res.ok.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs`
around lines 42 - 60, Update the response handling in fetchIncidents so the
existing 401/authentication-specific error check runs first, then reject every
response where res.ok is false, including JSON 403, 429, and 5xx responses
without body.errors. Preserve the existing GraphQL error handling and successful
body.data return path.
Comment on lines
+118
to
+121
| const data = await gql(QUERY, { appId: APP_ID, namespace, limit: 100, start: since }); | ||
| const incidents = data?.app?.exceptionIncidents || []; | ||
| for (const i of incidents) out.push({ ...i, namespace: i.namespace || namespace }); | ||
| console.error(`${namespace}: ${incidents.length} incident(s)`); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the reported lines
sed -n '1,220p' plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs
printf '\n--- SEARCH exceptionIncidents ---\n'
rg -n "exceptionIncidents|pageInfo|cursor|pagination|limit: 100|start: since|QUERY" plugins/flow-webhook-triage -S
printf '\n--- FILE LISTING IN SCRIPT DIR ---\n'
git ls-files plugins/flow-webhook-triage/skills/triage-flow-webhooks/scriptsRepository: glific/claude-plugins
Length of output: 6358
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '100,150p' plugins/flow-webhook-triage/skills/triage-flow-webhooks/references/sheet-setup.md
printf '\n--- SEARCH TRIAGE DOCS ---\n'
rg -n "fetch-incidents|incident(s)?|limit: 100|pagination|watermark|since" plugins/flow-webhook-triage/skills/triage-flow-webhooks -SRepository: glific/claude-plugins
Length of output: 18120
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,90p' plugins/flow-webhook-triage/skills/triage-flow-webhooks/SKILL.md
printf '\n--- WATERMARK / FETCH REFERENCES ---\n'
sed -n '90,150p' plugins/flow-webhook-triage/skills/triage-flow-webhooks/SKILL.mdRepository: glific/claude-plugins
Length of output: 7213
Avoid truncating incident fetches at 100. plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs:118-121 always requests limit: 100 and treats the page as complete. If AppSignal returns a full page, later incidents in the watermark window can be skipped permanently. Paginate, or fail when a page hits 100 until pagination is added.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs`
around lines 118 - 121, The incident retrieval loop around gql in
fetch-incidents.mjs must not treat a 100-item response as complete: add
pagination using the API’s supported cursor/offset fields and continue fetching
until a page is shorter than the limit, while preserving the existing namespace
mapping and output behavior.
Comment on lines
+118
to
+123
| const data = await gql(QUERY, { appId: APP_ID, namespace, limit: 100, start: since }); | ||
| const incidents = data?.app?.exceptionIncidents || []; | ||
| for (const i of incidents) out.push({ ...i, namespace: i.namespace || namespace }); | ||
| console.error(`${namespace}: ${incidents.length} incident(s)`); | ||
| } | ||
| console.log(JSON.stringify(out, null, 2)); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Emit the downstream deduplication key as incident_id.
The query returns id, while append-to-sheet.mjs rejects every row without incident_id. Normalize it when constructing the output.
Proposed fix
- for (const i of incidents) out.push({ ...i, namespace: i.namespace || namespace });
+ for (const i of incidents) {
+ out.push({
+ ...i,
+ incident_id: String(i.id),
+ namespace: i.namespace || namespace,
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const data = await gql(QUERY, { appId: APP_ID, namespace, limit: 100, start: since }); | |
| const incidents = data?.app?.exceptionIncidents || []; | |
| for (const i of incidents) out.push({ ...i, namespace: i.namespace || namespace }); | |
| console.error(`${namespace}: ${incidents.length} incident(s)`); | |
| } | |
| console.log(JSON.stringify(out, null, 2)); | |
| const data = await gql(QUERY, { appId: APP_ID, namespace, limit: 100, start: since }); | |
| const incidents = data?.app?.exceptionIncidents || []; | |
| for (const i of incidents) { | |
| out.push({ | |
| ...i, | |
| incident_id: String(i.id), | |
| namespace: i.namespace || namespace, | |
| }); | |
| } | |
| console.error(`${namespace}: ${incidents.length} incident(s)`); | |
| } | |
| console.log(JSON.stringify(out, null, 2)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs`
around lines 118 - 123, Update the output object constructed in the incidents
loop of fetch-incidents to map each returned incident’s id to the
downstream-required incident_id field. Preserve the namespace fallback and all
other incident properties while ensuring every emitted row includes incident_id.
Comment on lines
+65
to
+67
| Per incident, capture: number, exception class, count, first/last seen, and the tags — | ||
| `webhook_name`, `error_type`, `kaapi_error_type`, `http_status`, `organization_id`, `flow_id`, | ||
| `contact_id`, `reason`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Map AppSignal fields to expected sheet columns explicitly.
The downstream Code.gs script relies on specific column names (incident_id, occurrences, org_id, appsignal_url). To prevent the LLM from dropping data or hallucinating keys when building the JSON payload, explicitly map the captured data fields to the expected column names and include the missing appsignal_url.
💡 Proposed fix
-Per incident, capture: number, exception class, count, first/last seen, and the tags —
-`webhook_name`, `error_type`, `kaapi_error_type`, `http_status`, `organization_id`, `flow_id`,
-`contact_id`, `reason`.
+Per incident, capture: incident_id (number), exception class, occurrences (count), first/last seen, appsignal_url, and the tags —
+`webhook_name`, `error_type`, `kaapi_error_type`, `http_status`, `org_id` (from organization_id), `flow_id`,
+`contact_id`, `reason`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Per incident, capture: number, exception class, count, first/last seen, and the tags — | |
| `webhook_name`, `error_type`, `kaapi_error_type`, `http_status`, `organization_id`, `flow_id`, | |
| `contact_id`, `reason`. | |
| Per incident, capture: incident_id (number), exception class, occurrences (count), first/last seen, appsignal_url, and the tags — | |
| `webhook_name`, `error_type`, `kaapi_error_type`, `http_status`, `org_id` (from organization_id), `flow_id`, | |
| `contact_id`, `reason`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/flow-webhook-triage/skills/triage-flow-webhooks/SKILL.md` around
lines 65 - 67, Update the incident capture guidance in the triage-flow-webhooks
skill to explicitly map AppSignal fields to the downstream sheet columns:
incident number to incident_id, count to occurrences, organization_id to org_id,
and include appsignal_url. Preserve the existing exception, timing, and tag
fields while naming the exact JSON keys expected by Code.gs.
The deploy dialog offers "Anyone", not "Anyone with the link" — the latter is Google's older label and does not appear in the current UI. Found while following the setup for real. Also spells out why the other three options fail: they all require the *caller* to be signed into Google, and the script POSTs with a plain fetch that has no session, so it gets a login page (HTML) instead of JSON. The append client's error for that case now says so directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e sheet Raised in review: the sheet is written by a Web App deployed with "Who has access: Anyone", so what ends up in it deserves more care than I gave it. Two fixes. **doGet was unauthenticated.** doPost checked TRIAGE_TOKEN; doGet did not. Since "Anyone" is what lets an unauthenticated client call the endpoint at all, that left the watermark — and every incident id in the sheet — readable by anyone who obtained the URL. Reads now require the token, with a regression guard so it cannot be dropped silently. **Free text is now redacted at the boundary.** Glific carries WhatsApp traffic, so a provider error string can quote a contact's phone number or echo a credential, and the sheet outlives AppSignal's access controls. Phone numbers, emails, credentials, JWTs, UUIDs and opaque blobs are scrubbed in append-to-sheet.mjs before send. Writing the tests immediately found a real leak: "Authorization: Bearer sk-xxx" matched the label, consumed "Bearer" as the value, and left the actual secret in the string — output that looked redacted but was not. Fixed with an explicit `(?:bearer\s+)?` step, and the test now covers it. Redaction is a backstop, not a guarantee, and the docs say so plainly. A regex cannot catch a name in prose or a sentence a contact typed; test-redact.mjs keeps those cases visible under "KNOWN LIMITS" rather than implying coverage the code does not have. The controls that actually hold are documented instead: keep sheet sharing tight, paraphrase rather than quote, and link the incident when the raw text is genuinely needed. contact_id is deliberately not a column. Also documents that the token and the sheet are separate doors — the token grants the endpoint, not the data. That distinction is easy to conflate and worth stating. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 401 handler asserted "this is almost always a PUSH key rather than a Personal API token". That was a hunch hard-coded as a diagnosis, and it cost real time: it sent the user to the wrong AppSignal page repeatedly while the actual failure was elsewhere, and I kept quoting the message back as if it were evidence. It now prints AppSignal's own error and lists candidate causes in order — wrong token type, no access to the app, truncated paste, regenerated token — without claiming to know which. An error string that names a cause gets trusted like a measurement; a wrong one manufactures confident false leads. Docs confirm the auth method itself is correct: personal API token as a query parameter against https://appsignal.com/graphql. Also adds the exact URL for the token (https://appsignal.com/users/edit) rather than "your user settings", which was not findable enough to act on. Renames GLIFIC_TRIAGE_SHEET_URL -> GLIFIC_TRIAGE_WEBAPP_URL. The old name reads as "the URL of the sheet", and predictably got set to the docs.google.com URL instead of the Apps Script /exec deployment URL. The value is the deployment, not the document. Documents the one-shared-sheet model, which the setup docs had backwards: the team writes to a single sheet via the owner's deployment, so most people set three env vars and never touch Apps Script. Per-person sheets would defeat the dedup and the trend analysis that are the entire point. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Running this against prod for the first time invalidated the core assumption.
**One AppSignal incident is not one failure.** AppSignal groups by exception
class, and this subsystem deliberately uses low-cardinality classes (the webhook
name lives in the message, not the class — which is why the AppSignal trigger had
to exist at all). So incident #511 "SystemError", 8958 occurrences, held samples
from text_to_speech, filesearch-gpt, voice-filesearch-gpt AND speech_to_text.
A row per incident would have been precise-looking and useless.
The unit of triage is now a SIGNATURE mined from samples:
(namespace, exception_class, webhook_name, error_type, reason_shape), hashed to
signature_key. reason_shape normalises ids out ("6367551 does not have any active
flows" -> "<N> does not have any active flows") so the same failure groups across
contacts. The incident number is demoted to a link. Today's 3 incidents resolve
to 8 distinct, diagnosable signatures.
Also adds org_count — distinct orgs per signature. That is the product-defect
signal the whole sheet exists for: one org repeating is a support conversation,
many orgs on one config error is something to fix at source.
**Query corrected against the live schema** (it had never been run):
`namespaces` is plural and takes [String]; exceptionIncidents has no time filter
but samples(start:, end:) does; tags are key/value pairs under sample.overview,
not fields. Counts are sample counts, not true volume — named sample_count, with
incident_count kept alongside for context, and the sample cap logged rather than
silently truncating.
**Header is no longer write-once.** It was written only when the sheet was empty,
so changing COLUMNS left a 24-column header above 28-column data: `occurrences`
sat above sample_count, `org_id` above incident_count. Correct data, every label
wrong, no error anywhere — it rendered perfectly. sheet_() now reconciles the
header on every write, with a regression test reproducing the stale-header case.
Verified end to end against the live deployment: 8 rows inserted, re-run gives
updated 8 / total 8, tokenless read rejected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/flow-webhook-triage/skills/triage-flow-webhooks/SKILL.md (1)
29-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUpdate deduplication key terminology to match the signature-based grouping.
The PR changed the deduplication strategy from
incident_idto normalized failure signatures (signature_key), andappend-to-sheet.mjsnow enforcessignature_keyas the required deduplication key. However, this documentation still referencesincident_idandincidentIds.💡 Proposed fixes for stale terminology
Update line 29:
-Sheet: dedup key is incident_id; recurrence bumps times_seen +Sheet: dedup key is signature_key; recurrence bumps times_seenYou should also update the references across the rest of the file:
- Line 43:
Returns { lastSeen, incidentIds, rowCount }.→signatureKeys- Line 46:
- incidentIds — already in the sheet.→signatureKeys- Line 85:
Only for incidents not in incidentIds.→signatureKeys- Line 159:
Recurring incidents need only the volatile fields (incident_id, ...→signature_key- Line 168:
The script dedups server-side on incident_id under a lock→signature_key- Line 216:
If incidentIds has it,→signatureKeys🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/flow-webhook-triage/skills/triage-flow-webhooks/SKILL.md` at line 29, Update the triage-flow-webhooks documentation to consistently use signature-based deduplication terminology: replace incident_id and incidentIds references with signature_key and signatureKeys in the deduplication description, return documentation, filtering guidance, recurring-incident payload guidance, server-side deduplication note, and final lookup instruction.
♻️ Duplicate comments (4)
plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs (2)
39-43: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound external requests with a timeout.
A stalled connection to AppSignal can cause the CLI to hang indefinitely because Node's
fetchdoes not have a default timeout. Please apply anAbortControllerwith a finite timeout to this request (and similarly inappend-to-sheet.mjs).💡 Proposed fix
+ const controller = new AbortController(); + const id = setTimeout(() => controller.abort(), 15000); const res = await fetch(`${ENDPOINT}?token=${encodeURIComponent(TOKEN)}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ query, variables }), + signal: controller.signal, }); + clearTimeout(id);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs` around lines 39 - 43, Update the fetch request in fetch-incidents.mjs and the corresponding request in append-to-sheet.mjs to use an AbortController with a finite timeout, passing its signal to fetch and ensuring the timer is cleaned up after completion. Preserve the existing request behavior while preventing stalled external calls from hanging indefinitely.
53-70: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject all unsuccessful HTTP responses.
Currently, if the AppSignal API returns a non-401 error with a JSON body that lacks a
body.errorsarray (e.g., a custom 429 Too Many Requests or a 5xx response), the script will silently returnbody.data(which is likelyundefined), leading to downstream failures.Check
res.okto catch all HTTP-level failures before returning.💡 Proposed fix
' 4. Has the token been regenerated since you copied it?' ); } + if (!res.ok) { + die(`HTTP ${res.status}: ${JSON.stringify(body).slice(0, 500)}`); + } if (body.errors) die(`GraphQL: ${JSON.stringify(body.errors).slice(0, 500)}`); return body.data; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs` around lines 53 - 70, Update the response handling before the existing authentication and GraphQL error checks to reject every HTTP response where res.ok is false, including responses without body.errors. Preserve the current detailed handling for 401/authentication failures, then use die with the HTTP status and available response message for other unsuccessful responses; only return body.data after all HTTP-level and GraphQL checks pass.plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs (2)
204-213: 🗄️ Data Integrity & Integration | 🟠 MajorPreserve existing values when recurrence fields are omitted.
Omitted fields in the payload (like
sample_countorhttp_status) will evaluate to''and overwrite the existing values in the sheet. Only include fields in thepatchobject if they are present in the payload, and use nullish checks to ensure valid0values are retained.Proposed fix
const patch = { - last_seen: row.last_seen || '', last_run_date: today, times_seen: prev + 1, - sample_count: row.sample_count || '', - incident_count: row.incident_count || '', - org_count: row.org_count || '', - org_ids: row.org_ids || '', - http_status: row.http_status || '', }; + if (row.last_seen != null) patch.last_seen = row.last_seen; + if (row.sample_count != null) patch.sample_count = row.sample_count; + if (row.incident_count != null) patch.incident_count = row.incident_count; + if (row.org_count != null) patch.org_count = row.org_count; + if (row.org_ids != null) patch.org_ids = row.org_ids; + if (row.http_status != null) patch.http_status = row.http_status;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs` around lines 204 - 213, Update the patch construction in the recurrence update flow to include optional recurrence fields only when present in the payload, preserving existing sheet values when they are omitted. Replace truthiness fallbacks for fields such as sample_count, incident_count, org_count, org_ids, and http_status with nullish checks so valid 0 values are retained, and conditionally add omitted fields rather than assigning empty strings.
214-234: 🔒 Security & Privacy | 🟠 MajorEscape untrusted Sheet values before writing them.
AppSignal fields are written raw via both
setValue()andsetValues(). Any string starting with=will be executed as an unintended formula in Google Sheets. Prefix untrusted string values with an apostrophe (') before writing them to the sheet to prevent formula injection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs` around lines 214 - 234, Sanitize all untrusted string values before writing them to the sheet in both the patch update loop and the pending-row construction. Update the setValue path and the values generated by COLUMNS.map to prefix strings beginning with “=” (or otherwise use the established apostrophe escaping) so they are stored as text rather than formulas, while preserving non-string values and existing defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs`:
- Around line 125-128: Fail closed when no token is configured by updating the
token checks in doGet (Code.gs lines 125-128) and doPost (Code.gs lines 162-165)
to reject requests unless the expected token exists and matches params.token,
using the strict !token || params.token !== token validation in both sites.
- Around line 72-88: In Code.gs lines 72-88, update the sheet initialization
flow to expand columns with insertColumnsAfter when sh.getMaxColumns() is below
COLUMNS.length, before reading or writing the header. In Code.gs lines 232-234,
expand rows with insertRowsAfter when sh.getMaxRows() is below sh.getLastRow() +
pending.length, before batch insertion; both sites require direct changes.
- Around line 195-197: Update the batch processing logic around the pendingIds
guard and existing-incident update path to track every processed ID, not only
newly queued rows. Record an ID after successfully handling either a new or
persisted incident, while preserving first-occurrence-wins behavior so duplicate
existing IDs are not updated or counted twice.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs`:
- Around line 250-261: The fetch-incidents flow currently limits results to 100
without handling a full page. Add pagination using the GraphQL response’s cursor
fields to retrieve all incidents before calling toSignatureRows, or explicitly
warn/fail when data.app.exceptionIncidents reaches 100; ensure the watermark
cannot advance while incidents remain unfetched.
---
Outside diff comments:
In `@plugins/flow-webhook-triage/skills/triage-flow-webhooks/SKILL.md`:
- Line 29: Update the triage-flow-webhooks documentation to consistently use
signature-based deduplication terminology: replace incident_id and incidentIds
references with signature_key and signatureKeys in the deduplication
description, return documentation, filtering guidance, recurring-incident
payload guidance, server-side deduplication note, and final lookup instruction.
---
Duplicate comments:
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs`:
- Around line 204-213: Update the patch construction in the recurrence update
flow to include optional recurrence fields only when present in the payload,
preserving existing sheet values when they are omitted. Replace truthiness
fallbacks for fields such as sample_count, incident_count, org_count, org_ids,
and http_status with nullish checks so valid 0 values are retained, and
conditionally add omitted fields rather than assigning empty strings.
- Around line 214-234: Sanitize all untrusted string values before writing them
to the sheet in both the patch update loop and the pending-row construction.
Update the setValue path and the values generated by COLUMNS.map to prefix
strings beginning with “=” (or otherwise use the established apostrophe
escaping) so they are stored as text rather than formulas, while preserving
non-string values and existing defaults.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs`:
- Around line 39-43: Update the fetch request in fetch-incidents.mjs and the
corresponding request in append-to-sheet.mjs to use an AbortController with a
finite timeout, passing its signal to fetch and ensuring the timer is cleaned up
after completion. Preserve the existing request behavior while preventing
stalled external calls from hanging indefinitely.
- Around line 53-70: Update the response handling before the existing
authentication and GraphQL error checks to reject every HTTP response where
res.ok is false, including responses without body.errors. Preserve the current
detailed handling for 401/authentication failures, then use die with the HTTP
status and available response message for other unsuccessful responses; only
return body.data after all HTTP-level and GraphQL checks pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 680ceeed-b4df-4cd3-9b22-08658632dcc2
📒 Files selected for processing (8)
plugins/flow-webhook-triage/README.mdplugins/flow-webhook-triage/skills/triage-flow-webhooks/SKILL.mdplugins/flow-webhook-triage/skills/triage-flow-webhooks/references/sheet-setup.mdplugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/append-to-sheet.mjsplugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gsplugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/test-upsert.jsplugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjsplugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/test-redact.mjs
Comment on lines
+72
to
+88
| if (sh.getLastRow() === 0) { | ||
| sh.getRange(1, 1, 1, COLUMNS.length).setValues([COLUMNS]); | ||
| sh.setFrozenRows(1); | ||
| return sh; | ||
| } | ||
|
|
||
| // The header is NOT write-once. Rows are written positionally against COLUMNS, so a | ||
| // header left over from an older deployment silently mislabels every column — the data | ||
| // is right and every name is wrong, which reads fine and is therefore worse than an | ||
| // error. (This happened for real: a 24-column header survived a change to 28 columns, | ||
| // leaving `occurrences` above sample_count and `org_id` above incident_count.) | ||
| const header = sh.getRange(1, 1, 1, COLUMNS.length).getValues()[0]; | ||
| if (COLUMNS.some((c, i) => header[i] !== c)) { | ||
| sh.getRange(1, 1, 1, COLUMNS.length).setValues([COLUMNS]); | ||
| sh.setFrozenRows(1); | ||
| } | ||
| return sh; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Auto-expand sheet dimensions before writing.
Unlike appendRow(), getRange() and setValues() do not automatically expand a sheet's dimensions. If a requested range exceeds the physical grid, it throws a fatal The coordinates or dimensions of the range are invalid exception. Google Sheets creates new sheets with 26 columns, while COLUMNS.length is 28, guaranteeing a crash on a fresh deployment. Similarly, bulk inserting rows can exceed getMaxRows().
plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L72-L88: Ensuresh.getMaxColumns()is at leastCOLUMNS.length, expanding it viash.insertColumnsAfter()if necessary, before reading or writing the header.plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L232-L234: Ensuresh.getMaxRows()is at leastsh.getLastRow() + pending.length, expanding it viash.insertRowsAfter()if necessary, before batch inserting the new rows.
📍 Affects 1 file
plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L72-L88(this comment)plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L232-L234
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs`
around lines 72 - 88, In Code.gs lines 72-88, update the sheet initialization
flow to expand columns with insertColumnsAfter when sh.getMaxColumns() is below
COLUMNS.length, before reading or writing the header. In Code.gs lines 232-234,
expand rows with insertRowsAfter when sh.getMaxRows() is below sh.getLastRow() +
pending.length, before batch insertion; both sites require direct changes.
Comment on lines
+125
to
+128
| const token = expectedToken_(); | ||
| if (token && params.token !== token) { | ||
| return json_({ error: 'unauthorized' }); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
Fail closed when TRIAGE_TOKEN is not configured.
The public Web App still grants unauthenticated access whenever the token script property is accidentally omitted, as token evaluates to falsy and the authorization check is bypassed.
plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L125-L128: Update thedoGetvalidation to reject requests unless a configured token is present and matches (if (!token || params.token !== token)).plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L162-L165: Apply the same strict token validation indoPost.
📍 Affects 1 file
plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L125-L128(this comment)plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs#L162-L165
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs`
around lines 125 - 128, Fail closed when no token is configured by updating the
token checks in doGet (Code.gs lines 125-128) and doPost (Code.gs lines 162-165)
to reject requests unless the expected token exists and matches params.token,
using the strict !token || params.token !== token validation in both sites.
Comment on lines
+195
to
+197
| // Same signature twice in one batch: the first wins. Must be checked before the | ||
| // `index` lookup — a queued row has no row number to update yet. | ||
| if (pendingIds[id]) continue; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
Deduplicate existing incidents within the current batch.
This check skips subsequent occurrences of an ID, but it relies on pendingIds which is only updated for new rows on line 227. If an already-persisted ID appears twice in the same request, it will skip this guard, update the existing row twice, and increment times_seen twice. Track every processed ID regardless of whether it's new or existing.
Proposed fix
// Same signature twice in one batch: the first wins. Must be checked before the
// `index` lookup — a queued row has no row number to update yet.
if (pendingIds[id]) continue;
+ pendingIds[id] = true;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/apps-script/Code.gs`
around lines 195 - 197, Update the batch processing logic around the pendingIds
guard and existing-incident update path to track every processed ID, not only
newly queued rows. Record an ID after successfully handling either a new or
persisted incident, while preserving first-occurrence-wins behavior so duplicate
existing IDs are not updated or counted twice.
Comment on lines
+250
to
+261
| const data = await gql(QUERY, { | ||
| appId: APP_ID, | ||
| namespaces: NAMESPACES, | ||
| limit: 100, | ||
| start, | ||
| end: null, | ||
| sampleLimit: SAMPLES_PER_INCIDENT, | ||
| }); | ||
|
|
||
| const incidents = (data?.app?.exceptionIncidents || []).filter((i) => (i.samples || []).length); | ||
| const rows = toSignatureRows(incidents); | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Avoid silent truncation of incidents when the 100-item limit is reached.
The GraphQL query requests a maximum of 100 incidents. If the time window contains more than 100 incidents, the remaining ones are silently dropped, causing them to be permanently skipped as the watermark advances.
Either add pagination using the API's cursor fields to fetch all incidents, or fail/warn explicitly when data.app.exceptionIncidents.length === 100 so the user knows they are missing data.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/flow-webhook-triage/skills/triage-flow-webhooks/scripts/fetch-incidents.mjs`
around lines 250 - 261, The fetch-incidents flow currently limits results to 100
without handling a full page. Add pagination using the GraphQL response’s cursor
fields to retrieve all incidents before calling toSignatureRows, or explicitly
warn/fail when data.app.exceptionIncidents reaches 100; ensure the watermark
cannot advance while incidents remain unfetched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
flow-webhook-triageplugin: pulls incidents from theflow_webhooksandflow_webhook_config_errorsnamespaces, diagnoses each against theglific/glificcode, and upserts a row into a shared Google Sheet.
Why a sheet
A single incident is noise. A month of diagnosed incidents answers two questions
AppSignal can't:
:unknownbucket?:unknownis the fail-safe, soanything a webhook can't name pages on-call unclassified. The pattern only shows up
over weeks.
Many NGOs making the same mistake is a product defect — validate it at the source
instead of triaging it forever.
Each incident gets one row that persists, with a
times_seencounter that incrementsevery run it reappears, so repeat offenders sort straight to the top.
Design notes
Dedup is on
incident_id, server-side, under a lock. A recurrence updates theexisting row and bumps
times_seenrather than appending — so re-runs are idempotent,two people triaging the same day can't double-write, and the diagnosis isn't
re-litigated on every run.
No Google credentials are shared. Sheet writes go through an Apps Script Web App
that the sheet owner deploys as themselves. This is load-bearing: the Sheets API
requires OAuth for any write regardless of link-sharing, so "share the sheet and
hardcode the id" cannot work — an API key only ever grants read access to published
sheets. The deployment URL is the only secret and lives in an env var.
The taxonomy is read from the code, not inferred —
errors.ex,error_type.ex,error_reporter.ex,instrumentation.ex. Worth knowing:rate_limited/service_unavailableare:system, not suppressed, because there is noretry — an upstream blip is a real page.
references/error-taxonomy.mdcites files andasks the reader to re-verify, since this subsystem has moved fast (#5351, #5357, #5388).
AppSignal needs a Personal API token, not the push key the app uses to send errors.
Push keys are write-only and 401 on read. The script says so explicitly on a 401 — this
is a very easy mistake to make and the error is otherwise opaque.
Verified vs not
test-upsert.js— stubsSpreadsheetAppsoCode.gscan run in NodeThat last row is the main review risk. Field names may be wrong;
fetch-incidents.mjs introspectdumps the live schema to correctQUERYon first run. The output contract(one object per incident, with tags) is what the rest of the skill depends on. The
limitation is flagged in the script and in
sheet-setup.mdrather than left to looktested.
test-upsert.jsalready earned its keep — it caught an in-batch duplicateincident_idtaking the update path and writing to row
-1, which throws in real Apps Script andfails the whole batch.
Overlap worth resolving
There's an unmerged
glific-monitoringskill (on thesystem-monitoring-skillbranch,also present untracked in some local marketplace checkouts) doing AppSignal → classify →
Discord → autofix PRs. It overlaps this on "pull AppSignal, classify" but differs on
everything else: prod-wide vs these two namespaces, Discord vs sheet, no incremental
resume, no month-scale analysis. Kept separate deliberately rather than entangling
with an unmerged branch — but whoever owns it should weigh in before both land.
Setup (both one-time, in
references/sheet-setup.md)APPSIGNAL_API_KEY— verify withnode scripts/fetch-incidents.mjs verifyGLIFIC_TRIAGE_SHEET_URL+GLIFIC_TRIAGE_TOKENBoth scripts dry-run without their env vars, so you can see the shape before creating
anything.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation