feat(skills): add typescript-patterns skill (#543)#717
Conversation
There was a problem hiding this comment.
affaan-m has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Analysis CompleteGenerated ECC bundle from 500 commits | Confidence: 100% View Pull Request #718Repository Profile
Detected Workflows (9)
Generated Instincts (17)
After merging, import with: Files
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
|
Closing — CI failures. TypeScript skill needs catalog count updates to pass validators. |
There was a problem hiding this comment.
1 issue found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/claw.js">
<violation number="1" location="scripts/claw.js:19">
P2: Default timeout was reduced from 5 minutes to 30 seconds, which can cause premature failures for normal Claude responses.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const SESSION_NAME_RE = /^[a-zA-Z0-9][-a-zA-Z0-9]*$/; | ||
| const DEFAULT_MODEL = process.env.CLAW_MODEL || 'sonnet'; | ||
| const DEFAULT_COMPACT_KEEP_TURNS = 20; | ||
| const DEFAULT_CLAUDE_TIMEOUT_MS = 30000; |
There was a problem hiding this comment.
P2: Default timeout was reduced from 5 minutes to 30 seconds, which can cause premature failures for normal Claude responses.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/claw.js, line 19:
<comment>Default timeout was reduced from 5 minutes to 30 seconds, which can cause premature failures for normal Claude responses.</comment>
<file context>
@@ -16,6 +16,7 @@ const readline = require('readline');
const SESSION_NAME_RE = /^[a-zA-Z0-9][-a-zA-Z0-9]*$/;
const DEFAULT_MODEL = process.env.CLAW_MODEL || 'sonnet';
const DEFAULT_COMPACT_KEEP_TURNS = 20;
+const DEFAULT_CLAUDE_TIMEOUT_MS = 30000;
function isValidSessionName(name) {
</file context>
| const DEFAULT_CLAUDE_TIMEOUT_MS = 30000; | |
| const DEFAULT_CLAUDE_TIMEOUT_MS = 300000; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a327f981cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "scripts/session-inspect.js", | ||
| "scripts/uninstall.js", | ||
| "skills/", | ||
| "skills/agent-harness-construction/", |
There was a problem hiding this comment.
Preserve all existing skills in the npm package
Replacing the single "skills/" entry with a hard-coded allowlist drops 10 existing root skills from npm pack/registry installs. I verified the packed tarball now omits, for example, skills/context-budget/SKILL.md and skills/rules-distill/SKILL.md, even though commands/context-budget.md and commands/rules-distill.md still point at those paths. That means users installing this release from npm will get commands/docs that reference missing skills, so those workflows break outside a source checkout.
Useful? React with 👍 / 👎.
| ## Reference | ||
|
|
||
| This repo does not yet ship a dedicated `typescript-patterns` skill. For detailed TypeScript and JavaScript patterns, use `coding-standards` plus `frontend-patterns` or `backend-patterns` based on the code being reviewed. | ||
| For detailed TypeScript and JavaScript patterns, use `typescript-patterns` alongside `frontend-patterns` or `backend-patterns` based on the code being reviewed. |
There was a problem hiding this comment.
Publish
typescript-patterns to Codex skill installs
This adds the new skill only under skills/typescript-patterns, but the Codex path loads from .agents/skills (.codex/AGENTS.md) and scripts/sync-ecc-to-codex.sh copies only that directory into ~/.codex/skills. In other words, after this change Codex users still cannot load typescript-patterns, even though this reviewer guide now tells them to use it. Please add the corresponding .agents/skills/typescript-patterns copy (and harness metadata) before advertising the skill here.
Useful? React with 👍 / 👎.
Adds TypeScript patterns skill with type safety, generics, async patterns, and project configuration. Matches existing skill format and passes catalog validators.
Summary by cubic
Adds a new
typescript-patternsskill with clear patterns for type modeling, runtime validation, and async design. Also makes the CLAW timeout configurable (default 30s) and standardizes MCP startup timeouts.New Features
skills/typescript-patternscovering unions, generics, type guards,zodvalidation, result types,AbortSignal, and React prop typing.typescript-reviewerto use the new skill.manifests/install-modules.jsonandpackage.jsonfor install/publish.Refactors
CLAW_TIMEOUT_MS(default 30,000 ms; was 300,000); tests updated to exercise the env-based path.startup_timeout_secto 30s in.codex/config.tomland the sync script.package.jsonfile list explicit to include the new skill and tighten the published surface.Written for commit a327f98. Summary will update on new commits.
Summary by CodeRabbit
New Features
Chores