Skip to content

fix(server): retry expired T3 Connect link proofs - #12077

Closed
Gigioxx wants to merge 2 commits into
pingdotgg:mainfrom
Gigioxx:t3code/fix-reported-issue-1
Closed

Gigioxx wants to merge 2 commits into
pingdotgg:mainfrom
Gigioxx:t3code/fix-reported-issue-1

Conversation

@Gigioxx

@Gigioxx Gigioxx commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Closed: this PR accidentally selected an unrelated existing fork branch after a branch-name collision. The verified fix for #12061 is in #12078.

Summary by CodeRabbit

  • Bug Fixes
    • Skill references with names beginning with numbers, such as $2spec, are now recognized and processed consistently across chat, markdown, and command dispatch.
    • Numeric dollar expressions such as $1 and $100 continue to be treated as plain text rather than skill references.
    • Skill labels and slash-command conversions now work correctly for numeric-prefix skill names.

Skill token patterns required an ASCII letter first even though discovery accepts digit-leading directory names. Allow digits at the first position in composer parsing, web and mobile rendering, and Claude dispatch.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 16, 2026
@Gigioxx Gigioxx closed this Sep 16, 2026
* dispatched skill are always the same set.
*/
const SKILL_MENTION_PATTERN = /(^|\s)\$([a-zA-Z][a-zA-Z0-9:_-]*)(?=\s|$)/g;
const SKILL_MENTION_PATTERN = /(^|\s)\$([a-zA-Z0-9][a-zA-Z0-9:_-]*)(?=\s|$)/g;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium Drivers/ClaudeSkillDispatch.ts:32

Use $1 now is dispatched as skill 1 and rewritten to /1 now, even though the composer treats $1 as plain text. The pattern permits a digit as the first character; require an alphabetic first character to keep numeric expressions from triggering skills.

Suggested change
const SKILL_MENTION_PATTERN = /(^|\s)\$([a-zA-Z0-9][a-zA-Z0-9:_-]*)(?=\s|$)/g;
const SKILL_MENTION_PATTERN = /(^|\s)\$([a-zA-Z][a-zA-Z0-9:_-]*)(?=\s|$)/g;
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Drivers/ClaudeSkillDispatch.ts around line 32:

`Use $1 now` is dispatched as skill `1` and rewritten to `/1 now`, even though the composer treats `$1` as plain text. The pattern permits a digit as the first character; require an alphabetic first character to keep numeric expressions from triggering skills.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: d9b59e0b-c605-4517-90b6-40aa47455baf

📥 Commits

Reviewing files that changed from the base of the PR and between ccf220b and 3fa1635.

📒 Files selected for processing (7)
  • apps/mobile/modules/t3-markdown-text/src/nativeMarkdownText.ts
  • apps/mobile/src/lib/nativeMarkdownText.test.ts
  • apps/server/src/provider/Drivers/ClaudeSkillDispatch.test.ts
  • apps/server/src/provider/Drivers/ClaudeSkillDispatch.ts
  • apps/web/src/components/chat/SkillInlineText.tsx
  • packages/shared/src/composerInlineTokens.test.ts
  • packages/shared/src/composerInlineTokens.ts

📝 Walkthrough

Walkthrough

Skill token and mention matching now accepts names that start with digits. Shared tokenization excludes purely numeric dollar expressions. Web, mobile, and Claude dispatch tests cover the updated behavior.

Changes

Numeric skill-name support

Layer / File(s) Summary
Shared token recognition
packages/shared/src/composerInlineTokens.ts, packages/shared/src/composerInlineTokens.test.ts
The shared tokenizer recognizes $2spec as a skill token and keeps $1 and $100 as plain text.
Client skill decoration
apps/web/src/components/chat/SkillInlineText.tsx, apps/mobile/modules/t3-markdown-text/src/nativeMarkdownText.ts, apps/mobile/src/lib/nativeMarkdownText.test.ts
Web and mobile matching accept digit-prefixed skill names. The mobile test uses 2spec.
Server skill dispatch
apps/server/src/provider/Drivers/ClaudeSkillDispatch.ts, apps/server/src/provider/Drivers/ClaudeSkillDispatch.test.ts
Claude skill mention matching accepts $2spec, and dispatch produces /2spec review this.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Composer
  participant SkillTokenizer
  participant ClientRenderer
  participant ClaudeDispatcher
  Composer->>SkillTokenizer: Submit "$2spec review this"
  SkillTokenizer-->>ClientRenderer: Return skill token "2spec"
  Composer->>ClaudeDispatcher: Submit "$2spec review this"
  ClaudeDispatcher-->>Composer: Return "/2spec review this"
Loading

Suggested reviewers: juliusmarminge

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@macroscopeapp

macroscopeapp Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This is a small, tested parsing fix that extends skill recognition to names beginning with digits across the composer, clients, and Claude dispatch path, without schema, infrastructure, or default changes. A remaining Medium correctness finding concerns numeric-only $1 dispatch inconsistency in the Claude path.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant