Skip to content

fix: use toolset key for PTC eligibility checks#2268

Merged
alchemistklk merged 1 commit intomainfrom
ptc-toolset-video-pr
Mar 12, 2026
Merged

fix: use toolset key for PTC eligibility checks#2268
alchemistklk merged 1 commit intomainfrom
ptc-toolset-video-pr

Conversation

@nettee
Copy link
Contributor

@nettee nettee commented Mar 11, 2026

Summary

This PR fixes PTC eligibility checks to use the actual toolset key instead of only relying on toolset IDs.

Changes

  • Updated toolset key extraction in SkillInvokerService to read toolset.key first
  • Added safe fallback handling for missing nested fields when building the PTC toolset list
  • Prevented incorrect PTC blacklist/allowlist matching caused by mismatched identifiers

Summary by CodeRabbit

  • Bug Fixes
    • Improved toolset configuration handling to safely process nested or missing toolset data during platform evaluation, preventing potential errors and enhancing system reliability.

- Use each toolset's nested key when building the PTC evaluation list instead of relying on id only.
- Add safe fallback handling for missing toolset fields to prevent incorrect blacklist matching.
@coderabbitai
Copy link

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 99b00b84-e494-4343-8f7c-8e803e6f443c

📥 Commits

Reviewing files that changed from the base of the PR and between 76329f7 and 54c5b1c.

📒 Files selected for processing (1)
  • apps/api/src/modules/skill/skill-invoker.service.ts

📝 Walkthrough

Walkthrough

This change modifies toolset key extraction logic in the skill invoker service. Previously, the code directly accessed t.id; now it safely retrieves t?.toolset?.key with fallbacks to t?.id and an empty string, improving null-safety for PTC enablement calculation.

Changes

Cohort / File(s) Summary
PTC Enablement Logic
apps/api/src/modules/skill/skill-invoker.service.ts
Updated toolset key extraction to use optional chaining (t?.toolset?.key ?? t?.id ?? '') instead of direct property access, enabling safe handling of nested or missing toolset objects in PTC evaluation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • alchemistklk
  • mrcfps

Poem

🐰 A whisker-twitch for safer keys,
Where nested toolsets were not pleased,
Now fallbacks flow with grace so free,
No null-pointer errors we shall see! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: use toolset key for PTC eligibility checks' directly and accurately summarizes the main change—switching from using toolset IDs to toolset keys for PTC evaluation logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ptc-toolset-video-pr

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

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

@cloudflare-workers-and-pages
Copy link

Deploying refly-branch-test with  Cloudflare Pages  Cloudflare Pages

Latest commit: 54c5b1c
Status: ✅  Deploy successful!
Preview URL: https://6dfc88c8.refly-branch-test.pages.dev
Branch Preview URL: https://ptc-toolset-video-pr.refly-branch-test.pages.dev

View logs

@nettee
Copy link
Contributor Author

nettee commented Mar 11, 2026

/cr 修复 PTC toolset 黑名单判断

@slack-code-review-channel
Copy link

✅ CR summary sent to Slack channel #code-reviews.

@alchemistklk alchemistklk merged commit d33b72d into main Mar 12, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants