Add TypeScript code review skill

[phillipc]

• Created a TypeScript code review skill document outlining the review process, core categories, and output format.
• Introduced two comprehensive plan for addressing TypeScript code review findings in the TKO monorepo.
• Updated tsconfig.json to exclude new directories for skills and plans.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1a7a95bf-9a24-44c7-afcd-63e25bbcfc3c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pc/review_skill_and_demo_plans

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[phillipc]

I usually keep my skills and plans in a .agents/**/ folder. Thanks to the references in the agents.md file, all AI tools (Codex, Claude, Copilot) can find this folder. @brianmhunt: What do you think? Should I move it?

[brianmhunt]

Thanks for putting this together, Phillip. I'm going to take a proper pass on this later — leaving some observations now so they're on the record for when either of us comes back.

On placement (.agents/**/ vs repo root). My lean is to keep skills/ and plans/ at the repo root. TKO already treats plans/ as a first-class directory (the modernization migration has been landing work there, AGENTS.md has a "Plan Files" section referencing it, and several cross-plan links exist). If the motivation for .agents/**/ is cross-tool discoverability, that's fixable with pointers from AGENTS.md / CLAUDE.md rather than relocation. Happy to be overruled if there's a concrete benefit I'm missing.

Scope of this PR. It bundles three things:

1. The skill itself (skills/typescript-code-review/)
2. Two generated findings plans (plans/typescript-code-review-findings*.md, ~570 lines)
3. AGENTS.md + tsconfig wiring

My gut: land (1) + (3) first, treat (2) as separate follow-ups. Each finding is substantive work worthy of independent review — as-bundled, merging quietly accepts the findings as adopted plans. If you want them here for demo value, a one-line header labelling them "generated demo output, not adopted work" would do it.

Stale tooling references to refresh before merge:

• tko-conventions.md: "Karma + Electron" → should be Vitest + Playwright (chromium/firefox/webkit) per plans/modern-tooling.md.
• findings.md non-goals reference tools/build.mk and tools/karma.conf.js — both were deleted in Replace Make + lerna with Bun scripts #309.
• findings-2.md baselines: make tsc / make eslint → bunx tsc / bunx @biomejs/biome check.
• findings.md Phase 1.3 ("Add skills to tsconfig exclude") is self-referential with the tsconfig change in this PR.

Duplication risk with AGENTS.md. The TKO carve-outs in tko-conventions.md (factory-function-as-constructor, .fn prototype wiring, module-level side-effects, pervasive any) largely restate what AGENTS.md already documents. Worth deciding on a single source of truth — AGENTS.md canonical with the skill cross-referencing, or vice versa — otherwise they'll drift.

tsconfig.json excluding plans/ is (I think) a no-op — tsc doesn't pick up .md. If there's a reason I'm missing, a one-line comment in the PR would help future readers.

Critical findings worth verifying independently before we lock them in as plan items. Round-2 calls out five. I'd want to spot-check each against current main:

1. Proxy deleteProperty(property) signature — plausible, severe if real.
2. "?? behaves identically to ||" — extraordinary claim; want to see the case.
3. Parser operator-precedence inversion — fact-checkable.
4. TextInputLegacyFirefox dead code.
5. style binding referencing global jQuery.

Issue #235 (wrong binding-handler lookup in AttributeMustacheProvider) is flagged "confirmed" — still open, worth re-checking against current code before committing to a fix route here.

None of this is blocking — breadcrumbs for when I come back with a proper review. Good work overall; the skill itself is clean.

[brianmhunt]

Spot-checked the round-2 critical finding #1 (Proxy deleteProperty trap) — confirmed and fixed in #336 (draft, standalone 1-line patch + regression test). Credited the finding to this PR. Bug has been latent since 2017; no test exercised delete on a proxied object.

Still owe you proper reviews on the rest of the findings.

[brianmhunt]

Landed on a recommendation for the skills/ vs .agents/**/ question — keep skills at the repo root.

Reasoning:

• Consistency with plans/. Both are first-class, hand-curated, checked-in artifacts. Splitting one at root and one under .agents/**/ makes the repo structure schizophrenic.
• Discoverability. Dotted folders get hidden/muted in IDE trees and listings. For something the team is meant to extend, root visibility wins.
• Existing agent-docs home is tko.io/public/agents/. That directory already holds hand-written agent references (contract.md, guide.md, soul.md, testing.md, glossary.md, plus auto-generated verified-behaviors/). Moving skills/ under .agents/**/ would create a second "agents" root and split the mental model.
• Skills have structure that doesn't fit the prose-doc mold (SKILL.md frontmatter + references + possibly scripts), so they shouldn't live inside tko.io/public/agents/ either.

Proposed final layout:

plans/                          # planning docs (existing)
skills/                         # AI skills (this PR)
tko.io/public/agents/           # long-form agent references (existing)
AGENTS.md                       # short index, pointers to above

Example of how long-form references complement skills: I landed tko.io/public/agents/options.md in #337 as the canonical place for the defineOption pattern, with AGENTS.md holding only a one-line pointer. Same shape would work here — skills/typescript-code-review/SKILL.md can cross-link into tko.io/public/agents/ for pattern references rather than duplicating them in its own references/.

[phillipc]

@brianmhunt The challenge is that the project has seen some changes in the last weeks. We should discard this pr and creates some issues or separate pull-requests for the findings.

[phillipc]

The proposed final layout is fine for me.

[phillipc]

Closing this draft in favor of a new PR with updated findings plans. The review feedback from #297 has been incorporated — see the follow-up PR for details.