Skip to content

Preserve inner error codes through the skills lock-state wrap#5966

Open
samuv wants to merge 1 commit into
mainfrom
skills-nits/httperr-codes
Open

Preserve inner error codes through the skills lock-state wrap#5966
samuv wants to merge 1 commit into
mainfrom
skills-nits/httperr-codes

Conversation

@samuv

@samuv samuv commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Follow-up to the Record project-scope skill installs in the lock file #5894 panel review (deferred minor, promised in the review reply; tracked under Track: Skills lock file + Sigstore signing (RFC THV-0080) #5899). installAndRegister wrapped every recordLockState failure as HTTP 500 — but dependency materialization runs inside recordLockState, so a dependency's specific failure (502 from a git resolve, 404 from a registry miss) was masked to 500 at the API boundary, and classifySyncFailure reported unknown instead of the typed reason (registry-unreachable/digest-missing) that CI automation keys on.
  • The wrap now attaches 500 only when the inner chain carries no httperr code. Code-less failures — e.g. an actual lock file write error — keep defaulting to 500, and the errLockWrite sentinel classification is unchanged.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Test plan

  • New unit test: an install whose declared dependency fails to resolve surfaces the resolver's 502 through Install (was 500 before the fix)
  • Extended existing test: a pure lock-write failure still surfaces as 500
  • task test and task lint-fix pass locally

🤖 Generated with Claude Code

installAndRegister wrapped every recordLockState failure as HTTP 500,
but dependency materialization runs inside recordLockState — so a
dependency's specific failure (502 from a git resolve, 404 from a
registry miss) was masked at the API boundary and classified as
"unknown" instead of its typed sync failure reason. Attach 500 only
when the inner chain carries no code; code-less failures such as an
actual lock write error keep defaulting to 500. Deferred follow-up
from the #5894 panel review.
@samuv
samuv requested a review from JAORMX as a code owner July 24, 2026 13:03
@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label Jul 24, 2026
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.89%. Comparing base (e047b0d) to head (aa6dda6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5966      +/-   ##
==========================================
+ Coverage   71.85%   71.89%   +0.03%     
==========================================
  Files         714      715       +1     
  Lines       73415    73502      +87     
==========================================
+ Hits        52755    52846      +91     
+ Misses      16879    16872       -7     
- Partials     3781     3784       +3     

☔ 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 self-assigned this Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant