Skip to content

Conversation

@leodido
Copy link
Contributor

@leodido leodido commented Nov 13, 2025

Summary

Fixes critical logging issues that made debugging cache operations and SLSA verification failures impossible. Status is now reported after operations complete, and verification failures are properly distinguished and tracked.

Fixes https://linear.app/ona-team/issue/CLC-2082/fix-misleading-cache-status-reporting-and-improve-build-logging

Changes

Critical Fixes (P0)

Status Reporting After Operations

  • Set package status after download completes, not before
  • Check actual cache state to determine if download succeeded
  • Prevents misleading "downloaded" status for packages that failed verification

Verification Error Handling

  • Add VerificationFailedError type to distinguish verification failures from missing artifacts
  • Return proper errors when SLSA verification fails
  • Enable build system to detect and report verification failures accurately

High Priority Improvements (P1)

Dependency Build Logging

  • Log when building dependencies
  • Log when packages are already cached
  • Log when packages were built as dependencies
  • Log when starting local builds

Build Summary

  • Add comprehensive summary showing cached/downloaded/built package counts
  • Track and report packages that failed verification/download
  • Provide clear overview of build outcomes

Medium Priority Improvements (P2)

Status Indicators

  • Add PackageVerificationFailed and PackageDownloadFailed status constants
  • Update reporter to show warning indicators (⚠️) for failures
  • Clear visual distinction between success and failure states

SLSA Verification Logging

  • Add structured logging before/after verification
  • Log verification duration
  • Enhanced error messages for debugging

Example Output

Before

📥 cached remotely (downloaded)  frontend/shared:lib
[frontend/shared:lib] build started
[frontend/shared:lib] package build succeeded

Problem: Says "downloaded" but actually rebuilt due to verification failure!

After

🌎 cached remotely (available)  frontend/shared:lib
[DEBUG] Starting SLSA verification: frontend/shared:lib
[WARN]  SLSA verification failed, artifact rejected
[INFO]  building package locally: frontend/shared:lib
[frontend/shared:lib] build started
[frontend/shared:lib] package build succeeded

Build completed:
  Total: 5, Cached: 2, Downloaded: 1, Built: 2
[WARN] Some packages failed verification or download and were rebuilt locally (count: 1)
[WARN]   - failed to download/verify, rebuilt locally: frontend/shared:lib

Solution: Clear indication of what actually happened!

Testing

  • ✅ All existing tests pass
  • ✅ Code compiles without errors
  • ✅ No breaking changes
  • ✅ Backward compatible

Notes

  • Changes improve logging for all builds, not just SLSA-related ones
  • Graceful fallback behavior is maintained (download errors don't fail builds)
  • TODO added for future improvement: distinguish download failures from verification failures (requires interface changes)

Related Issues

Addresses logging and debuggability issues identified in internal reviews.

- Set package status after download completes, not before
- Add VerificationFailedError to distinguish verification failures from missing artifacts
- Add comprehensive dependency build logging
- Add build summary showing cached/downloaded/built package counts
- Track and report packages that failed verification/download
- Add new status indicators for verification and download failures
- Enhance SLSA verification logging with structured fields

This addresses critical issues where status was reported before operations
completed, making it impossible to debug SLSA verification failures. Users
previously saw 'downloaded' for packages that failed verification and were
rebuilt locally.

Co-authored-by: Ona <no-reply@ona.com>
} else {
// Package was in cache but we're not sure how it got there
// This could happen if it was built as a dependency
alreadyCached++

Choose a reason for hiding this comment

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

Nit: we should log if we get here I think, not blocking just an idea

} else {
// Package was in cache but we're not sure how it got there
// This could happen if it was built as a dependency
alreadyCached++

Choose a reason for hiding this comment

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

Nit: we should log if we get here I think, not blocking just an idea

@leodido leodido merged commit a8ac383 into main Nov 13, 2025
6 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.

3 participants