Skip to content

Implement skills REST handlers with project_root#3911

Merged
JAORMX merged 2 commits intomainfrom
skills-api-handlers-3651
Mar 3, 2026
Merged

Implement skills REST handlers with project_root#3911
JAORMX merged 2 commits intomainfrom
skills-api-handlers-3651

Conversation

@JAORMX
Copy link
Copy Markdown
Collaborator

@JAORMX JAORMX commented Feb 20, 2026

Summary

  • implement skills REST handlers behavior for list/install/uninstall/info (query + payload parsing) with client and project_root
  • enforce strict project_root validation (absolute path, exists, directory, no traversal/null bytes, git repo) at API and service layers
  • normalize project_root to keep DB records and locks consistent across equivalent paths
  • update handler and service tests to cover project_root validation failures and successful flows
  • regenerate swagger docs to reflect the new parameters

Rationale

  • issue REST API skills handlers #3651 is about completing the REST handlers; this change wires the query/payload params and error handling expected by the handlers
  • project_root is sensitive: without validation it can enable path traversal or arbitrary filesystem access; validating at both API and service layers prevents misuse from any entry point
  • requiring git roots keeps “project” scoping aligned with the intended workflow and reduces accidental use of unrelated directories
  • normalizing project_root avoids duplicate records for the same directory and prevents lock key mismatches

Issue

Reviewer Notes

  • expect new 400s when scope=project is used without project_root, or when project_root fails validation
  • list/install/uninstall/info now accept project_root; list also accepts client
  • docs are regenerated in docs/server/*

Testing

  • task test
  • task docs

@github-actions github-actions Bot added the size/L Large PR: 600-999 lines changed label Feb 20, 2026
@JAORMX JAORMX changed the title Validate project_root for skills APIs Implement skills REST handlers with project_root Feb 20, 2026
Comment thread pkg/skills/project_root.go Fixed
Comment thread pkg/skills/project_root.go Fixed
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 20, 2026

Codecov Report

❌ Patch coverage is 90.16393% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.83%. Comparing base (a7ec95a) to head (3c4a0b2).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
pkg/skills/project_root.go 83.33% 8 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3911      +/-   ##
==========================================
+ Coverage   67.76%   67.83%   +0.06%     
==========================================
  Files         439      440       +1     
  Lines       44130    44229      +99     
==========================================
+ Hits        29905    30001      +96     
- Misses      11881    11885       +4     
+ Partials     2344     2343       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread pkg/api/v1/skills.go Outdated
@JAORMX
Copy link
Copy Markdown
Collaborator Author

JAORMX commented Feb 20, 2026

@claude could you fix the listing errors?

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Feb 20, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@JAORMX JAORMX force-pushed the skills-api-handlers-3651 branch from 0efac8f to c392bc5 Compare February 21, 2026 08:31
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Feb 21, 2026
JAORMX

This comment was marked as duplicate.

Comment thread pkg/skills/project_root.go Fixed
Comment thread pkg/skills/project_root.go Fixed
JAORMX

This comment was marked as resolved.

JAORMX

This comment was marked as resolved.

@JAORMX JAORMX force-pushed the skills-api-handlers-3651 branch from c392bc5 to 3051750 Compare February 21, 2026 08:38
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Feb 21, 2026
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 2, 2026
aponcedeleonch
aponcedeleonch previously approved these changes Mar 2, 2026
Copy link
Copy Markdown
Member

@aponcedeleonch aponcedeleonch left a comment

Choose a reason for hiding this comment

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

looks good

jhrozek
jhrozek previously approved these changes Mar 2, 2026
Copy link
Copy Markdown
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

LGTM — solid validation with good defense-in-depth. One minor nit left as an inline comment.

Comment thread pkg/skills/project_root.go Outdated
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 2, 2026
JAORMX added 2 commits March 2, 2026 17:39
Add client/project_root filters to skills listing and enforce strict project root validation, including git repository checks, across list/install/uninstall/info. Update tests and swagger docs to match.
Reject symlinked project_root paths and centralize scope/project_root normalization in skills to keep API thin and reuse service validation. Update API tests for service-driven validation errors.
@JAORMX JAORMX dismissed stale reviews from jhrozek and aponcedeleonch via 3c4a0b2 March 2, 2026 15:43
@JAORMX JAORMX force-pushed the skills-api-handlers-3651 branch from 092863d to 3c4a0b2 Compare March 2, 2026 15:43
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 2, 2026
Comment thread pkg/skills/project_root.go Dismissed
Comment thread pkg/skills/project_root.go Dismissed
Copy link
Copy Markdown
Member

@aponcedeleonch aponcedeleonch left a comment

Choose a reason for hiding this comment

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

LGTM

@JAORMX JAORMX merged commit b17aa31 into main Mar 3, 2026
60 of 62 checks passed
@JAORMX JAORMX deleted the skills-api-handlers-3651 branch March 3, 2026 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

REST API skills handlers

5 participants