Skip to content

Resolve relative --project-root before the API call - #6223

Merged
samuv merged 1 commit into
mainfrom
fix/6211-relative-project-root
Aug 6, 2026
Merged

Resolve relative --project-root before the API call#6223
samuv merged 1 commit into
mainfrom
fix/6211-relative-project-root

Conversation

@samuv

@samuv samuv commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

--project-root . is the natural thing to write when scripting thv skill commands, but it does not work. The CLI forwards the flag verbatim to the API server, which requires an absolute path, so the command fails with a validation error naming the wire field rather than the flag the user typed:

Error: failed to upgrade skills: project_root must be absolute, got "."

Two things made this worse than a cosmetic message problem:

  • The auto-detected path (used when --project-root is omitted) is absolute, so the flag behaved differently from its own default — surprising, and only at the point of failure.
  • thv skill upgrade exited 1, outside its documented 0/2/3/4 exit-code contract. A CI job branching on those codes sees an unexpected value instead of a clear failure.

What changed:

  • New absProjectRoot helper in skill_helpers.go resolves a user-supplied value with filepath.Abs, and leaves an empty value empty — for most skill commands empty means "not project-scoped", so substituting the working directory would silently change the scope.
  • resolveProjectRoot (sync, upgrade) now absolutizes the explicit branch, which previously short-circuited untouched.
  • install, info, list, and uninstall normalize before building their options.

Closes #6211

Type of change

  • Bug fix

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)
  • Manual testing (describe below)

New table-driven TestAbsProjectRoot covers empty / . / relative child / already-absolute / traversal-cleaning, plus TestResolveProjectRootAbsolutizesExplicit pinning the specific branch that regressed.

Manually, against a local thv serve:

$ thv skill list --scope project --project-root .
NAME                  SCOPE     STATUS      REFERENCE
gha-security-review   project   installed   ghcr.io/stacklok/dockyard/skills/gha-security-review:0.2.0

and confirmed the server-side validation is unchanged — it still rejects a relative root, so the normalization is genuinely happening client-side:

$ curl -s 'http://127.0.0.1:18080/api/v1beta/skills?scope=project&project_root=.'
project_root must be absolute, got "."

Does this introduce a user-facing change?

Yes. thv skill commands now accept a relative --project-root (including .), resolving it against the working directory. Absolute paths behave exactly as before.

Special notes for reviewers

Deliberately scoped to thv skill, matching the issue. The thv ai-plugin commands take the same flag and validate through the parallel pkg/plugins/validator.go, so they almost certainly have the identical bug — happy to fix them here instead if you'd rather it were one sweep, but it seemed better not to widen a bug fix into a package the issue never mentioned.

Generated with Claude Code

A relative --project-root travelled verbatim to the API server, which
requires an absolute path, so the command failed with a validation error
naming the wire field (project_root) rather than the flag the user typed.
For upgrade this also surfaced as exit 1, outside its documented 0/2/3/4
exit-code contract, so CI branching on those codes saw an unexpected value.

Empty stays empty: for most skill commands that means "not project-scoped",
and substituting the working directory would silently change the scope.

Closes #6211
@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Aug 6, 2026
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.50%. Comparing base (8863eb0) to head (1ee4bde).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6223      +/-   ##
==========================================
+ Coverage   72.48%   72.50%   +0.02%     
==========================================
  Files         739      739              
  Lines       76728    76728              
==========================================
+ Hits        55613    55635      +22     
+ Misses      17151    17110      -41     
- Partials     3964     3983      +19     

☔ View full report in Codecov by Harness.
📢 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.

@samuv
samuv merged commit 12ffd7a into main Aug 6, 2026
78 of 79 checks passed
@samuv
samuv deleted the fix/6211-relative-project-root branch August 6, 2026 08:36
@samuv samuv self-assigned this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Relative --project-root is rejected server-side with a confusing error

2 participants