Community Signal spec fixes, and template overrides wired through to storage

[NathanTarbert]

This PR carries two unrelated changes. 08cd06a is the Community Signal skill-spec work described below. 5427278 wires the template-override editor through to real storage and sandboxes its preview — its own description is on the commit, and review notes are in the comments. Two of those notes are worth settling before this leaves draft: a saved override does not reach a real invite email yet, and the slug route param is not validated before it reaches the filesystem.

---

Three rule changes to the Community Signal skill suite, from the Aug 14-21 run. Opening as a draft since more commits are likely.

Nathan spotted both of the things that drove the biggest change here: that the week's top issue had a fix PR the card never mentioned, and that the Fix plan line should lead with the PR link the same way Source: does. Both are in this commit, and the second turned out to be the more useful of the two, because it made the first kind of gap visible at a glance.

1. The Fix plan line leads with the linked PR

New shape, mirroring the Source: line:

**Fix plan:** [PR #6648](https://github.com/CopilotKit/CopilotKit/pull/6648) · In progress, awaiting a first review — …
**Fix plan:** No PR yet · Not started — the workaround is a request middleware.

Anchor text is PR #NNNN rather than a bare #NNNN, so it does not read as an issue number. When more than one PR is in play the live one leads and the closed or competing ones are linked in the prose. Same rule for Demand's Status: and Docs' Fix:.

The reason to put it first: the reader's next action after a Fix plan is to open the thread. A link sitting mid-sentence, or missing, stops that.

There is a matching check in the link-review pass so the format is verified on every run rather than remembered.

2. Fix-PR detection reads the authoritative field, and re-runs late

gh pr list --search "fixes #N OR closes #N" depends on GitHub's search indexing and on when in the run it executes, so a PR can exist and not appear. Detection now also reads the issue's own closedByPullRequestsReferences and its timeline, and re-runs on every ranked item at the final pre-publish refresh.

The timing matters more than it looks. On CopilotKit#3510, PR #6648 was opened 56 seconds after the contributor's comment claiming the issue. Any pass that reads the comment and stops there will miss the PR that follows it.

Four cards on this week's report were in that position, and the field would have caught all four:

• ag-ui#2447 — PR #2448, open since 2026-08-18. The reporter, a Microsoft engineer, filed the bug and the fix on the same day.
• ag-ui#167 — PR #2210, an open Python implementation since 2026-07-19, while a second contributor builds the TypeScript version. Neither has had an answer, so two people are currently building the same integration.
• CopilotKit#6507 — PR #6628 implemented the request in full and closed unmerged the same day, unreviewed.
• CopilotKit#6526 — two PRs open, one per competing root cause, with no call made between them.

Also worth stating plainly, because the three states get collapsed: a claim comment is not a PR, and a PR is not a merge. All three are recorded distinctly now.

The good news is what this surfaced. Once those four were corrected, seventeen fixes are written and waiting on review across both repos, several of them one-line changes on the worst open bugs. That is a healthier problem to have than a shortage of contributions, and it is now the report's lead finding.

3. Reporter attribution comes from the author field

Reported by: is taken from gh issue view --json author, not from whoever commented most recently.

This one is load-bearing rather than cosmetic. The last Top-issue tie-break is community-filed over staff-filed, and the enterprise counts and prospect roster read the same field, so the author determines a rank. Where the filer and the person doing the work differ, both are named and the implementer belongs on the Fix plan line.

Two supporting rules:

• Staff-filed cards are marked as such. Our own team carries no org membership and an empty company on their GitHub profile, so gh search commits --author <login> and the @copilotkit.ai address is what identifies them.
• A "community authors: N" figure always states which definition it uses. Distinct authors, distinct excluding staff, and distinct excluding contracted partner maintainers gave 6, 5 and 3 for the same week.

4. Gating and tier claims are checked against the raw payload

For any claim that a feature is Premium, Enterprise-only or gated, the check is now a grep of the raw page payload, with confirmation that the words sit in body copy rather than in navigation, a sidebar tree or a <meta> tag.

docs.copilotkit.ai/premium/headless-ui is the worked example: 264KB of payload, and no licence or Cloud requirement anywhere in it. A URL under /premium/ describes how the docs are organised, not what the feature costs.

Where a text extraction and a raw grep disagree, the grep decides. Absence claims verified the same way hold up well, and one did this week: "Coming Soon" is genuinely gone from /copilotkit-intelligence, confirmed by grepping the full page twice.

Also here

This run's Reddit dedup entry. The prune is still skipped, because earlier entries store bare id strings with no dates and a run window can span the 90-day cutoff, so ageing one out risks re-reporting a post. Carrying extra ids costs nothing by comparison.

Report content is not in this commit. That lives only in Notion, per the standing rule.

Two open questions

Happy to go either way on both, and neither is changed here:

1. Should the leading-link format extend to the ✅ Resolved table rows? Right now it applies to cards only, and the rows carry their PR in prose.
2. Is a severity tier worth adding for "a safety control silently switches itself off"? Two approval-bypass bugs scored 11 and 13 this week and both needed a priority override to land where they belonged, which suggests the axis is pricing them low rather than that the overrides were wrong. Left alone for now since it changes the rubric for every future run.

[NathanTarbert]

Review notes on the templates commit (5427278), so they are tracked somewhere other than a terminal. Tests pass, 35 across the two new files, and tsc --noEmit is clean.

The good news first, because most of this holds up: the sandboxed-iframe replacement for dangerouslySetInnerHTML is right, including leaving out both allow-scripts and allow-same-origin (either alone is survivable, together they let the frame drop its own sandbox). The type-vs-truthiness validation in PUT, deleteMany for an idempotent reset, the single batched findMany in the list route, the editedBy email-to-memberId fallback, and the draft-wins lookup injected into renderTemplate all check out.

Two worth settling before this leaves draft.

A saved override does not reach a real invite email yet. sendEmail only consults an override when it is handed a dbLookup (packages/outpost/shared/src/email/sender.ts:166), and neither call site passes one — apps/web/src/app/api/team/invite/route.ts:72 and .../invite/resend/route.ts:52. So the loop is closed for GET, list and preview, but an admin who edits the invite template sees "Template saved successfully" and a Custom badge, and the next invite still goes out with the on-disk copy. Passing the same findOverride helper into both call sites is probably all it needs.

The slug route param reaches the filesystem unvalidated. loadFromFilesystem builds join(dir, slug + '.md') (packages/outpost/shared/src/templates/loader.ts:75) and join normalises .., so a crafted slug reads an arbitrary .md off disk and returns its subject and body in the JSON. The .md suffix bounds it and this predates the commit, but the commit widens the surface — preview now calls it too, and PUT keys rows on the same string. listTemplateSlugs() already produces the allowlist, so testing membership in it would close this and make the new "must exist on disk" checks cheaper at the same time.

Smaller things, all optional:

• The bodiless-POST guard in preview/route.ts keys on content-length !== '0', but a bodiless POST can arrive with no content-length at all (chunked, or a server-side new Request(url, {method:'POST'})), and that path returns 400 rather than previewing the stored template. The comment above it describes the intended behaviour correctly; the check is narrower than the comment.
• Same file: const hasBody = await Promise.resolve(true); if (hasBody) is always true, so it reads as a condition without being one.
• Preview applies no length bound while PUT applies SUBJECT_MAX/BODY_MAX, and preview only needs a session rather than ADMIN, so a large draft.body runs the interpolate and markdown chain unbounded. Reusing the same two constants would line them up.
• If the bound on subject is there for header safety, a CR/LF check probably belongs next to it — length alone still allows Welcome\r\nBcc: ….
• GET returns an override even when the filesystem template is gone (loadTemplate synthesises the meta), while PUT and preview both 404 on that slug. Reachable only by deleting a .md with a row still present, so low stakes, but it is the one case where preview and save disagree.
• On the page: Reset now genuinely deletes and has no confirm step, and it renders whether or not isOverride is true. Also setSuccess after the post-reset reload can sit next to an error banner if that reload fails, and handlePreview is the only handler that does not clear error on entry, so a stale failure can stay on screen beside a good preview.

Happy to be argued out of any of these, particularly the content-length one, which may be unreachable in practice given how the editor calls it.

[Anmol-Baranwal]

LGTM

[jerelvelarde]

Reviewed both halves. The skill-spec commit (08cd06a) is accurate and matches its description. The code commit (5427278) is where the problems are, and I verified the two biggest ones against the tree myself rather than taking the review at face value.

The sandboxed-iframe work is correct and genuinely closes the stored-XSS sink: sandbox="" with neither allow-scripts nor allow-same-origin, srcDoc, asserted structurally with an exact-value test, and no other dangerouslySetInnerHTML or rehype-raw sink left in apps/web. The tests are real — the loader/renderer/markdown pipeline actually execute; only session and Prisma are mocked.

Blocker — a saved override never reaches a real email, and the UI says it did

apps/web/src/app/api/team/invite/route.ts:72 and .../invite/resend/route.ts:52.

The write path works: PUT upserts templateOverride, the row lands, and the page does await fetchTemplates(); setSuccess('Template saved successfully'). Every read surface then confirms it — the list flips the row from Default to Custom, GET returns the override, the preview renders it.

The one surface that matters does not. sendEmail only consults an override when handed a dbLookup (packages/outpost/shared/src/email/sender.ts:166, :171), and neither invite call site passes one. The invitee gets the on-disk copy.

So an admin edits a template, is told it saved, sees a Custom badge, sends an invite, and the edit is silently ignored — with no trace anywhere. That is exactly the silent-success failure this commit's own test header says it is fixing. You flagged it yourself in the PR body as "worth settling before this leaves draft" — the PR is now out of draft with it still open, which is why I am gating on it rather than filing it.

Either export findOverride (or move it to a shared lib) and pass it as dbLookup at both call sites, with a test that a stored override changes the subject/body handed to the transport — or, if it is genuinely deferred, the editor has to stop claiming otherwise: no Custom badge and no unqualified success banner until the override is actually wired.

Also blocking, and undisclosed — the feature is dead in the production image

I checked this because it seemed too big to be true. It holds:

• templates/ is seven .md files at the repo root.
• .dockerignore:7 excludes *.md, so they never enter the build context — the pruner's COPY . . cannot pick them up.
• apps/web/Dockerfile's runner stage copies .next/standalone, .next/static, public, the Prisma bits and start.sh. Nothing copies templates/.

So in production findTemplatesDir() finds nothing, loadFromFilesystem returns null for every slug, listTemplateSlugs() returns [], and every new fail-closed gate 404s. The 35 tests pass only because they run from apps/web, where the repo-root templates/ exists. Worth confirming against a built image before this merges — if I have the packaging wrong I would rather be corrected than have you take my word for it.

The slug still reaches the filesystem unvalidated

loader.ts:73-75 does join(dir, ${slug}.md) with no validation, and Next decodes percent-encoding in a dynamic segment. ..%2F..%2F..%2Fsomething becomes a traversal. It is admin-gated and bounded to .md files, so the blast radius is small — but it is the second item your own PR body listed as unsettled, and a /^[a-z0-9-]+$/ test at the route boundary closes it in one line.

Two things to fix in the skill commit

1. reddit-pulse-seen.json:389 changes shape mid-file. The new run's seen_ids are {id, created} objects; all twelve prior runs are bare strings. weekly-report/SKILL.md:75 says to "skip any post id already listed", and any check shaped like seen.includes(post.id) or new Set(entry.seen_ids) matches nothing against the new shape — so this run's posts will be re-reported next week. This is the one file in the repo whose entire job is dedup state.

2. weekly-report/SKILL.md:150 — the Coverage bullet lost its three-space indent, so it escapes the sub-list under step 14 and renders as a top-level item. Every sibling above and below keeps the indent.

Process, not code

This PR carries two unrelated changes. That is already awkward for review, and it is how both of the items you flagged as unsettled ended up shipping out of draft — they are buried under a skill-spec description. Worth splitting the code commit out; I will re-review it on its own immediately.

The fix-PR detection change (reading closedByPullRequestsReferences and re-running at the final refresh) is a genuinely good catch, and the four cards it would have corrected make the case on their own.

[NathanTarbert]

Thanks Jerel — the split you asked for is done, and the two skill findings are fixed in 724c5ac. This PR is now Community Signal only: three files, all docs.

.claude/skills/product-surface-scan/SKILL.md
.claude/skills/weekly-report/SKILL.md
docs/community-signal/reddit-pulse-seen.json

The web half is #248, commit 5427278 unchanged and rebased onto current main. Your three code findings are recorded in that PR's body against the code they concern, and CPK-8188 tracks the review. You were right that bundling them under a skill-spec description is how two items you'd already flagged as unsettled got out of draft.

The dedup shape — you were right, with one wrinkle worth recording

The commit did already carry a line saying readers must tolerate both shapes. The problem is where it sat: about 200 words below the Skip any post id already listed instruction a reader actually acts on, at the tail of the paragraph about pruning. So someone could follow the instruction faithfully and still write entry.seen_ids.includes(post.id) — which is exactly the failure you described.

The normalisation now lives in the dedup instruction itself, with the flatten expression and the consequence spelled out:

Entries come in two shapes — bare "1abc234" strings (runs up to 2026-08-14) and {"id": "1abc234", "created": "YYYY-MM-DD"} objects (2026-08-21 onward) — so normalise before comparing: flatten with ids = entry.seen_ids.map(x => typeof x === 'string' ? x : x.id).

One judgement call, flagging it in case you'd rather go the other way: I kept the {id, created} shape rather than flattening the new run back to bare strings. The dates are what makes the >90d prune possible at all, and the skill says the prune stays skipped until every entry carries them — so restoring uniformity would trade a real capability for a tidier file. Happy to switch if you'd prefer one shape now and dates reintroduced later.

Verified the file still parses: 10 runs, both shapes present by design.

The Coverage bullet

Indent restored, so it sits with Claim support and Correctness under step 14 instead of escaping to top level. Confirmed all three siblings now at three spaces (:148, :150, :151).

On the fix-PR detection change

Glad that one reads well — the closedByPullRequestsReferences re-check at the final refresh is the part that catches a PR landing after the report is drafted, which is how those four cards went stale.

[jerelvelarde]

Approving the content. Both findings from my last pass are fixed, and the split is clean — #248 carries 5427278 with my three code findings recorded against the code they concern. That was the right call.

Verified rather than taken on trust:

• Dedup normalisation now sits in the instruction a reader acts on, with the flatten expression and the consequence. That was the actual defect — the old line was correct but 200 words from the point of use.
• Coverage bullet indent — all five step-14 siblings (Claim support, Fix-plan PR links, Coverage, Correctness, Prospect LinkedIn) are at matching indent.
• File still parses — 10 runs, both id shapes present.

On the {id, created} judgement call: keep it. Restoring uniformity would trade the only thing that makes the prune possible for a tidier file, and the skill already tells readers to tolerate both shapes at the point they compare. Right trade.

Blocking merge, but not your bug

Format check (changed files) is red on all three files. I checked out origin/main's versions of the same three files and ran prettier against them — they fail identically. The check arrived with #250 and this branch's merge of main pulled it in; it gates any PR that touches a file that was never formatted, regardless of what the PR changed.

Please don't fix it here. Prettier wants ~1,200 lines: it reindents the SKILL.md list nesting, repads every table, and 4-space-reindents the ledger. On these files that is a content change — this PR's own review turned up an indentation bug where the Coverage bullet escaped step 14, which is exactly the class of damage a wholesale reindent causes silently.

.prettierignore already carries the argument, for /templates/:

Prettier reflows markdown — wrapping lines, renumbering lists, normalising emphasis — and every one of those changes what lands in a customer's inbox.

Skill specs are read by agents as instructions, so the same reasoning holds with the same force. Suggest a separate one-line PR adding .claude/skills/ (and docs/community-signal/*.json) to .prettierignore — that unblocks this and every future skill-spec PR. Happy to open it if you'd rather not context-switch.

Worth fixing before merge (cheap)

1. Title and body are stale. The title still ends "and template overrides wired through to storage", and the body still opens with the two-commit note describing 5427278 and its two unsettled items — none of which are in this PR any more. It also says "Opening as a draft" while the PR is not a draft. This becomes the permanent merge-commit record, so it's worth the edit.

2. _comment in the ledger contradicts the new rule. It still instructs "prune ids older than 90 days", while the run note and the SKILL.md now say the prune is skipped and explain why bare-string entries make it unsafe. A reader following _comment does the thing the spec forbids. One line, in a file this PR already edits.

3. Minor: the two pre-existing _comment/_note lines got their em dashes rewritten to — escapes. Valid JSON and harmless, but it is unexplained churn in a hand-read file and suggests something other than a plain editor wrote it. Worth knowing which tool did it, in case it touches more next run.

Open questions from the body

1. Leading-link on Resolved rows — leave as-is. The rows are scanned, not acted on; the format earns its cost on cards where the next action is opening the thread.
2. Severity tier for a safety control silently disabling itself — worth adding. Two items needing a priority override in one week is the rubric mispricing the axis, not the overrides being wrong. Separate PR, since it changes every future run.

Approving so this isn't waiting on me — merge once CI is green.