Skip to content

fix(ci): partition pnpm caches by runner#56

Merged
Jesssullivan merged 1 commit intomainfrom
fix/self-hosted-pnpm-cache
Apr 16, 2026
Merged

fix(ci): partition pnpm caches by runner#56
Jesssullivan merged 1 commit intomainfrom
fix/self-hosted-pnpm-cache

Conversation

@Jesssullivan
Copy link
Copy Markdown
Owner

Summary:

  • isolate workspace-local pnpm store caches by runner name on self-hosted honey runners
  • stop honey-sk-1 and honey-sk-2 from restoring each others pnpm store snapshots
  • keep the existing workspace-local store model introduced for the self-hosted lane

Root cause:
The publish workflow test job restored a cache snapshot created on another runner, and pnpm then tried to copy from an absolute store path under honey-sk-1 while running on honey-sk-2. Partitioning the cache key by runner name removes that cross-runner contamination.

Validation:

  • observed publish run 24519258595 fail in Install dependencies with ERR_PNPM_ENOENT referencing honey-sk-1 from a honey-sk-2 job
  • patched CI and publish cache keys to include runner.name

@Jesssullivan Jesssullivan merged commit 331d6fc into main Apr 16, 2026
4 checks passed
@Jesssullivan Jesssullivan deleted the fix/self-hosted-pnpm-cache branch April 16, 2026 15:42
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 16, 2026

Greptile Summary

This PR fixes cross-runner pnpm cache contamination on self-hosted runners (honey-sk-1, honey-sk-2) by adding runner.name to the cache key and restore-keys in both ci.yml and publish.yml. The root cause — a job on honey-sk-2 restoring a cache snapshot written by honey-sk-1 with a different absolute store path — is directly addressed.

Confidence Score: 5/5

Safe to merge — the fix correctly isolates self-hosted runner caches and only remaining feedback is a P2 suggestion to add a runner-agnostic restore-key fallback tier.

The targeted bug (cross-runner store path contamination) is fully fixed. The two comments are P2 style suggestions about improving cache hit rate on GitHub-hosted fallback runners, which is a minor efficiency concern that does not affect correctness or self-hosted CI.

No files require special attention — both workflow changes are consistent and focused.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Cache keys in both the test (matrix) and build jobs now include runner.name, correctly isolating self-hosted runner stores; however, on GitHub-hosted fallback runners runner.name varies per job, causing full cache misses.
.github/workflows/publish.yml The test job's pnpm cache key gains runner.name consistently with ci.yml; publish/download jobs are unaffected as they use artifact upload/download instead of pnpm caching.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Job starts on runner] --> B{runner.name in cache key}
    B -->|Before fix - key has no runner.name| C[Cache lookup: OS + node + pnpm + hash]
    B -->|After fix - key includes runner.name| D[Cache lookup: OS + runner.name + node + pnpm + hash]

    C --> E{Cache hit?}
    E -->|Yes - may be from another runner| F[Restore alien store abs path = honey-sk-1]
    E -->|No| G[Cold install]
    F --> H[pnpm copy from wrong abs path ERR_PNPM_ENOENT]

    D --> I{Exact hit on same runner?}
    I -->|Yes| J[Restore own store abs path matches]
    I -->|No - restore-keys prefix match| K{Runner type?}
    K -->|Self-hosted stable runner.name| L[Partial hit from same runner]
    K -->|GitHub-hosted unique runner.name per run| M[No hit at all - cold install]

    J --> N[pnpm install fast]
    L --> N
    G --> N
    M --> N
Loading

Reviews (1): Last reviewed commit: "fix(ci): partition pnpm caches by runner" | Re-trigger Greptile

Comment thread .github/workflows/ci.yml
Comment on lines +47 to +49
key: ${{ runner.os }}-${{ runner.name }}-node-${{ matrix.node-version }}-pnpm-${{ env.PNPM_VERSION }}-${{ hashFiles('pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-node-${{ matrix.node-version }}-pnpm-${{ env.PNPM_VERSION }}-
${{ runner.os }}-${{ runner.name }}-node-${{ matrix.node-version }}-pnpm-${{ env.PNPM_VERSION }}-
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 GitHub-hosted runner cache cold-start on fallback path

When PRIMARY_LINUX_RUNNER_LABELS_JSON is unset and jobs fall back to ubuntu-latest, GitHub provisions a fresh VM each run with a distinct runner.name (e.g. GitHub Actions 3, GitHub Actions 17). Because both the exact key and the restore-keys prefix embed that unique name, neither will match any prior run's entry — every job on GitHub-hosted runners will see a complete cache miss. Before this change the key was runner-agnostic, so restore-keys provided partial hits. The fix is correct for self-hosted runners; just be aware the fallback lane loses cache re-use entirely unless a runner-agnostic restore-key tier is added, e.g.:

restore-keys: |
  ${{ runner.os }}-${{ runner.name }}-node-${{ matrix.node-version }}-pnpm-${{ env.PNPM_VERSION }}-
  ${{ runner.os }}-node-${{ matrix.node-version }}-pnpm-${{ env.PNPM_VERSION }}-

Comment on lines +46 to +48
key: ${{ runner.os }}-${{ runner.name }}-node-${{ env.DEFAULT_NODE_VERSION }}-pnpm-${{ env.PNPM_VERSION }}-${{ hashFiles('pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-node-${{ env.DEFAULT_NODE_VERSION }}-pnpm-${{ env.PNPM_VERSION }}-
${{ runner.os }}-${{ runner.name }}-node-${{ env.DEFAULT_NODE_VERSION }}-pnpm-${{ env.PNPM_VERSION }}-
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Same GitHub-hosted fallback cache miss (publish test job)

Mirrors the ci.yml concern: the single restore-keys entry includes runner.name, so GitHub-hosted fallback runs will always have a cold cache. Adding a runner-agnostic restore-key tier would preserve partial hits on that path:

restore-keys: |
  ${{ runner.os }}-${{ runner.name }}-node-${{ env.DEFAULT_NODE_VERSION }}-pnpm-${{ env.PNPM_VERSION }}-
  ${{ runner.os }}-node-${{ env.DEFAULT_NODE_VERSION }}-pnpm-${{ env.PNPM_VERSION }}-

Jesssullivan added a commit to tinyland-inc/scheduling-kit that referenced this pull request Apr 16, 2026
* refactor!: remove middleware code (belongs in acuity-middleware) (#10)

Removed: src/middleware/ (33 files), modal-app.py, Dockerfile,
live tests, playwright deps. Version 0.3.1 to 0.4.0.

* chore: bump version to 0.5.0

* refactor!: remove acuity-scraper adapter

Scraper belongs in acuity-middleware, not the scheduling library.
Deprecated since extract-business.ts + middleware wizard steps
replaced all scraper functionality.

BREAKING: AcuityScraper, createScraperAdapter, scrapeServicesOnce,
scrapeAvailabilityOnce removed from @tummycrypt/scheduling-kit/adapters.

* build: add Bazel 8 configuration with subpackage targets

- MODULE.bazel: bzlmod config with rules_js 2.9.1, rules_ts 3.8.4, SWC, pnpm 9
- BUILD.bazel: svelte-package build, npm_package, 6 subpackage ts_project
  targets (core, adapters, payments, reconciliation, lib, testing), vitest,
  svelte-check typecheck
- .bazelrc: build/CI/debug/release configs with disk cache
- .bazelversion: pin to 8.1.1
- .npmrc: hoist=false (required by rules_js)

* feat: v0.5.0 - remove acuity-scraper, add Bazel 8 config (#11)

* chore: bump version to 0.5.0

* refactor!: remove acuity-scraper adapter

Scraper belongs in acuity-middleware, not the scheduling library.
Deprecated since extract-business.ts + middleware wizard steps
replaced all scraper functionality.

BREAKING: AcuityScraper, createScraperAdapter, scrapeServicesOnce,
scrapeAvailabilityOnce removed from @tummycrypt/scheduling-kit/adapters.

* build: add Bazel 8 configuration with subpackage targets

- MODULE.bazel: bzlmod config with rules_js 2.9.1, rules_ts 3.8.4, SWC, pnpm 9
- BUILD.bazel: svelte-package build, npm_package, 6 subpackage ts_project
  targets (core, adapters, payments, reconciliation, lib, testing), vitest,
  svelte-check typecheck
- .bazelrc: build/CI/debug/release configs with disk cache
- .bazelversion: pin to 8.1.1
- .npmrc: hoist=false (required by rules_js)

* feat(venmo): add payeeEmail option to route payments to practitioner

When payeeEmail is set in VenmoAdapterConfig, the PayPal order
creation includes payee.email_address in purchase_units. This
routes payments directly to the practitioner's PayPal account
without requiring their API credentials.

Ref: PayPal "Pay another account" docs

* chore: bump version to 0.5.1 (payee-email support)

* fix(ci): use @Jesssullivan scope for GitHub Packages mirror (Jesssullivan#18)

* feat(venmo): add returnUrl/cancelUrl to experience_context (Jesssullivan#19)

* fix(ci): use @Jesssullivan scope for GitHub Packages mirror

* feat(venmo): add returnUrl/cancelUrl to experience_context

PayPal requires return_url and cancel_url in the Venmo payment source
experience_context for proper popup handling. Without them, PayPal may
force additional buyer verification loops or block the popup flow.

New optional fields on VenmoAdapterConfig: returnUrl, cancelUrl.

* chore: bump to 0.5.2 (PayPal return URLs) (Jesssullivan#20)

* fix(ci): use @Jesssullivan scope for GitHub Packages mirror

* feat(venmo): add returnUrl/cancelUrl to experience_context

PayPal requires return_url and cancel_url in the Venmo payment source
experience_context for proper popup handling. Without them, PayPal may
force additional buyer verification loops or block the popup flow.

New optional fields on VenmoAdapterConfig: returnUrl, cancelUrl.

* chore: bump to 0.5.2 (PayPal return URLs)

* feat: onboarding subpackage — provider credential management (Jesssullivan#21-Jesssullivan#27) (Jesssullivan#28)

New @tummycrypt/scheduling-kit/onboarding subpackage:

Interfaces:
- CredentialStore: app-provided key-value storage (PG, Redis, etc.)
- EncryptionProvider: app-provided encryption (AES, Vault, etc.)
- StripeConnectConfig, StripeAccountStatus, WebhookSetupResult types

Stripe:
- buildStripeAuthorizeUrl() + exchangeStripeCode() — Connect OAuth
- getStripeAccountStatus() — account onboarding status
- validateStripeKeys() — key validation against Stripe API
- createStripeWebhook() + deleteStripeWebhooks() — webhook CRUD

PayPal:
- validatePayPalCredentials() — OAuth token validation
- createPayPalWebhook() — webhook creation

Build:
- Bazel //src/onboarding target (deps: :core, :payments, effect)
- Package.json ./onboarding export

Pattern: library defines interfaces + helpers, application provides
CredentialStore implementation. Same pattern as HomegrownAdapter's
getDb callback — scheduling-kit doesn't know about databases.

Closes Jesssullivan#21, Jesssullivan#22, Jesssullivan#23, Jesssullivan#24, Jesssullivan#27. Partial Jesssullivan#25, Jesssullivan#26.

* chore: bump to 0.6.0 (onboarding subpackage) (Jesssullivan#29)

* feat: adapter factory pattern + 21 onboarding tests (Jesssullivan#25, Jesssullivan#26) (Jesssullivan#30)

- createAdapterFactory(): settings-driven singleton with cache,
  promise dedup, reset, and disable lifecycle
- 21 tests: Stripe OAuth URL, key validation, account status,
  PayPal credential validation, factory lifecycle (cache, reset,
  disable, store passthrough)
- Updated vitest.config.ts to include onboarding test glob

Closes Jesssullivan#25, Jesssullivan#26.

* chore: strip sourcemaps from npm package (Jesssullivan#31)

* feat: adapter factory pattern + 21 onboarding tests (Jesssullivan#25, Jesssullivan#26)

- createAdapterFactory(): settings-driven singleton with cache,
  promise dedup, reset, and disable lifecycle
- 21 tests: Stripe OAuth URL, key validation, account status,
  PayPal credential validation, factory lifecycle (cache, reset,
  disable, store passthrough)
- Updated vitest.config.ts to include onboarding test glob

Closes Jesssullivan#25, Jesssullivan#26.

* chore: strip sourcemaps from npm package (2,711 .map files excluded)

* feat: provider status helpers + SetupStep type (Jesssullivan#32) (Jesssullivan#33)

* chore: bump to 0.6.1 (status helpers) (Jesssullivan#34)

* align build truth and package boundaries (Jesssullivan#39)

* ci: enforce Bazel release metadata truth (Jesssullivan#40)

* docs: add agent and llm operating brief (Jesssullivan#42)

* feat(payments)!: converge PaymentCapabilities contract from tinyland-inc (Jesssullivan#45)

* feat(payments)!: converge PaymentCapabilities contract from tinyland-inc

Cherry-pick tinyland-inc/main squash (v0.7.0) onto Jesssullivan/main.
Keeps Jess's CI/publish workflows and Bazel structure.
Bumps all version references to 0.7.0.

- PaymentCapabilities, StripeCapability, VenmoCapability types
- getDefaultCapabilities() factory
- HybridCheckoutDrawer: capabilities prop replaces individual payment props
- Cash at Visit structurally removed (cash: false)

* fix(ci): skip prepublish scripts in gh packages mirror

* docs(release): clarify scheduling-kit authority (Jesssullivan#46)

* ci(publish): validate bazel package artifact (Jesssullivan#47)

* build(bazel): publish scheduling-kit from bazel artifact

* fix(ui): dark-mode skeleton shimmer and border parity (Jesssullivan#49)

Replace hardcoded hex CSS with light-dark() for 8 components:
skeleton loading shimmer, border colors, scrollbar tracks.
Ensures proper dark-mode rendering when consumed by host apps.

* ci: support honey self-hosted runner stopgap (Jesssullivan#50)

* ci: isolate pnpm store on self-hosted runners (Jesssullivan#51)

* fix: make publish workflow self-hosted-safe (Jesssullivan#52)

* fix(ci): make github package artifact writable (Jesssullivan#53)

* perf(components): drop zod from browser client form (Jesssullivan#54)

* fix(ci): ignore npm scripts when publishing bazel pkg (Jesssullivan#55)

* fix(ci): ignore npm scripts when publishing bazel pkg

* fix(ci): clean stale bazel publish artifacts on runners

* fix(ci): partition pnpm caches by runner (Jesssullivan#56)
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.

1 participant