Skip to content

fix(stack): harden service artifact resolution - #6041

Open
jgoux wants to merge 1 commit into
developfrom
lazy-stack/01-artifacts
Open

fix(stack): harden service artifact resolution#6041
jgoux wants to merge 1 commit into
developfrom
lazy-stack/01-artifacts

Conversation

@jgoux

@jgoux jgoux commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Stack layer 1 of 7.

Rebuilds the non-Fleet artifact foundation from #5819 as a focused change:

  • centralizes the complete managed and upstream service catalog
  • keeps imgproxy, Mailpit, and Vector Docker-only
  • adds Supabase registry candidates and native release descriptors as the seam for slim-services
  • makes native artifact caching safe under concurrent preparation

Fleet remains out of scope for this stack.

Stack: #6041#6042#6043#6044#6045#6046#6047

Supersedes #5819

@jgoux
jgoux force-pushed the lazy-stack/01-artifacts branch from 5cb141b to 39d03a3 Compare August 4, 2026 07:43
@jgoux
jgoux marked this pull request as ready for review August 4, 2026 07:43
@jgoux
jgoux requested a review from a team as a code owner August 4, 2026 07:43

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 39d03a3e68

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/stack/src/BinaryResolver.ts
Comment thread packages/stack/src/BinaryResolver.ts Outdated
Comment thread packages/stack/src/BinaryResolver.ts
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@8c2562536b9f9f5291acaae113b0cab66767ec51

Preview package for commit 8c25625.

@jgoux
jgoux force-pushed the lazy-stack/01-artifacts branch from 39d03a3 to 3bdb698 Compare August 4, 2026 08:04

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3bdb698f60

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/stack/src/BinaryResolver.ts Outdated
Comment thread packages/stack/src/BinaryResolver.ts Outdated
@jgoux
jgoux force-pushed the lazy-stack/01-artifacts branch from 3bdb698 to a8cc0a1 Compare August 4, 2026 09:32

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a8cc0a111e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/stack/src/versions.ts
@jgoux
jgoux force-pushed the lazy-stack/01-artifacts branch from a8cc0a1 to 8c25625 Compare August 4, 2026 09:41
@jgoux
jgoux force-pushed the lazy-stack/01-artifacts branch from 8c25625 to 462a1fb Compare August 4, 2026 10:20

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 462a1fbe91

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const ownerFiles = entries.value.filter(isCacheLockOwnerFile);
if (ownerFiles.length === 0) {
if (isPastOwnerGrace) {
yield* removeEmptyDirectory(lockPath);

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 Badge Tie lock reclamation to the observed directory

When a waiter stats an old lock, that lock can be removed and another contender can recreate lockPath before this call; because isPastOwnerGrace belongs to the old directory and the new contender may not have written its owner marker yet, this rmdir deletes the new lock and causes its marker write to fail with DownloadError (and StackPreparation to fall back to Docker). Fresh evidence beyond the earlier resolved thread is that the code captures the stale mtime before a separate readDirectory/rmdir sequence without verifying that the directory instance is unchanged; reclamation should atomically claim the observed lock rather than acting on the reused pathname.

Useful? React with 👍 / 👎.

Comment on lines +263 to +264
if (yield* isProcessAlive(ownerPid)) {
return;

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 Badge Verify lock owners beyond their process ID

When a resolver is hard-killed and its PID is later reused, such as after a stale cache lock survives a reboot, process.kill(ownerPid, 0) succeeds for the unrelated process and this branch treats the abandoned lock as live. Subsequent resolutions repeatedly exhaust the lock retry window and return DownloadError or fall back to Docker until that unrelated process exits. Fresh evidence beyond the earlier abandoned-lock fix is that the uniquely named marker still records only a PID, with no process-start or boot identity to distinguish reuse; persist and verify such an identity before retaining the lock.

Useful? React with 👍 / 👎.

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