Skip to content

Commit 780ad88

Browse files
Surface early sticky disk commit denial and stop logging false commit success (#49)
The VM agent reports commit_early_deny/commit_early_deny_reason on GetStickyDisk when it already knows the commit will be denied (branch protection). Emit a notice when the git mirror is set up, save the reason to state, release the sticky disk without a commit in the post step, and stop logging a successful commit: the RPC only records intent and the host applies it at VM shutdown. Bumps @buf/blacksmith_vm-agent.{bufbuild_es,connectrpc_es} to the BSR build that includes the new fields, with the matching .licenses records. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent f505656 commit 780ad88

10 files changed

Lines changed: 327 additions & 79 deletions

.licenses/npm/@buf/blacksmith_vm-agent.bufbuild_es-1.10.0-20260115212136-f78b3d5805ba.1.dep.yml renamed to .licenses/npm/@buf/blacksmith_vm-agent.bufbuild_es-1.10.0-20260904133914-c6beb298c70c.1.dep.yml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.licenses/npm/@buf/blacksmith_vm-agent.bufbuild_es-1.9.0-20260115212136-f78b3d5805ba.2.dep.yml renamed to .licenses/npm/@buf/blacksmith_vm-agent.bufbuild_es-1.9.0-20260904133914-c6beb298c70c.2.dep.yml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.licenses/npm/@buf/blacksmith_vm-agent.connectrpc_es.dep.yml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/index.js

Lines changed: 238 additions & 40 deletions
Large diffs are not rendered by default.

package-lock.json

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@
3333
"@actions/github": "^6.0.0",
3434
"@actions/io": "^1.1.3",
3535
"@actions/tool-cache": "^2.0.1",
36-
"@buf/blacksmith_vm-agent.bufbuild_es": "1.9.0-20260115212136-f78b3d5805ba.2",
37-
"@buf/blacksmith_vm-agent.connectrpc_es": "1.6.1-20260115212136-f78b3d5805ba.2",
36+
"@buf/blacksmith_vm-agent.bufbuild_es": "1.9.0-20260904133914-c6beb298c70c.2",
37+
"@buf/blacksmith_vm-agent.connectrpc_es": "1.6.1-20260904133914-c6beb298c70c.2",
3838
"@bufbuild/protobuf": "^1.9.0",
3939
"@connectrpc/connect": "^1.6.1",
4040
"@connectrpc/connect-node": "^1.6.1",

src/blacksmith-cache.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ export interface CacheInfo {
8484
// performedHydration indicates that this job performed the initial git mirror clone.
8585
// Used to notify the backend on commit so it can mark hydration as complete.
8686
performedHydration: boolean
87+
// commitEarlyDenyReason is non-empty when the host already knows this job's
88+
// sticky disk commit will be denied (e.g. branch protection): the mirror can
89+
// be used but changes to it are discarded at VM teardown.
90+
commitEarlyDenyReason: string
8791
}
8892

8993
/**
@@ -326,7 +330,8 @@ export async function setupCache(
326330
mirrorPath: '',
327331
hydrationInProgress: true,
328332
hydrationMessage,
329-
performedHydration: false
333+
performedHydration: false,
334+
commitEarlyDenyReason: ''
330335
}
331336
}
332337
// Re-throw other errors
@@ -350,6 +355,15 @@ export async function setupCache(
350355
`[git-mirror] Got sticky disk device: ${device}, exposeId: ${exposeId}`
351356
)
352357

358+
const commitEarlyDenyReason = response.commitEarlyDeny
359+
? response.commitEarlyDenyReason || 'denied by host policy'
360+
: ''
361+
if (commitEarlyDenyReason) {
362+
core.notice(
363+
`[git-mirror] Sticky disk changes will not be committed for this job (${commitEarlyDenyReason}). The git mirror cache is used as-is and any changes to it are discarded.`
364+
)
365+
}
366+
353367
// Format if needed
354368
await waitForNonZeroDeviceSize(device, 10000)
355369
await maybeFormatDevice(device)
@@ -370,7 +384,8 @@ export async function setupCache(
370384
mountPoint,
371385
mirrorPath: getMirrorPath(owner, repo),
372386
hydrationInProgress: false,
373-
performedHydration: false // Will be set by ensureMirror if we do initial clone
387+
performedHydration: false, // Will be set by ensureMirror if we do initial clone
388+
commitEarlyDenyReason
374389
}
375390
}
376391

@@ -1996,7 +2011,13 @@ export async function cleanup(options: CleanupOptions): Promise<CleanupResult> {
19962011
vmHydratedGitMirror: vmHydratedGitMirror
19972012
})
19982013

1999-
core.info('[git-mirror] Successfully committed sticky disk')
2014+
// The host applies the commit at VM teardown, after this step has ended;
2015+
// the RPC only records whether a commit was requested.
2016+
core.info(
2017+
shouldCommit
2018+
? '[git-mirror] Sticky disk commit requested; applied at VM shutdown'
2019+
: '[git-mirror] Sticky disk released without commit'
2020+
)
20002021
} catch (error) {
20012022
core.warning(
20022023
`[git-mirror] Failed to commit sticky disk: ${(error as any)?.message ?? error}`

src/git-source-provider.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ export async function getSource(settings: IGitSourceSettings): Promise<void> {
138138
stateHelper.setBlacksmithCacheRepoName(cacheInfo.repoName)
139139
stateHelper.setBlacksmithCacheMirrorPath(cacheInfo.mirrorPath)
140140
stateHelper.setBlacksmithCacheMountPoint(cacheInfo.mountPoint)
141+
stateHelper.setBlacksmithCacheCommitEarlyDenyReason(
142+
cacheInfo.commitEarlyDenyReason
143+
)
141144

142145
const performedHydration = await blacksmithCache.ensureMirror(
143146
cacheInfo.mirrorPath,

src/main.ts

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -79,28 +79,38 @@ async function cleanup(): Promise<void> {
7979
let cleanupResult: blacksmithCache.CleanupResult | undefined
8080

8181
try {
82-
// Check for previous step failures by reading runner logs
83-
// This is the same approach used by setup-docker-builder (BPA)
84-
core.info(
85-
'[git-mirror] Checking for previous step failures before committing'
86-
)
87-
const failureCheck = await checkPreviousStepFailures()
88-
8982
let shouldCommit = true
9083
let skipReason = ''
9184

92-
if (failureCheck.error) {
93-
// If we can't determine failure status, skip commit to be safe
85+
const commitEarlyDenyReason =
86+
stateHelper.BlacksmithCacheCommitEarlyDenyReason
87+
if (commitEarlyDenyReason) {
88+
// The host already told us at mount time that this job's writes are
89+
// discarded, so there is nothing to persist and no need to inspect
90+
// step results.
9491
shouldCommit = false
95-
skipReason = `Unable to check for step failures: ${failureCheck.error}`
96-
} else if (failureCheck.hasFailures) {
97-
shouldCommit = false
98-
skipReason = `Found ${failureCheck.failedCount} failed/cancelled steps`
99-
if (failureCheck.failedSteps) {
100-
for (const step of failureCheck.failedSteps) {
101-
core.warning(
102-
`[git-mirror] - Step: ${step.stepName || step.action || 'unknown'} (${step.result})`
103-
)
92+
skipReason = `commit denied for this job (${commitEarlyDenyReason}); mirror changes are discarded`
93+
} else {
94+
// Check for previous step failures by reading runner logs
95+
// This is the same approach used by setup-docker-builder (BPA)
96+
core.info(
97+
'[git-mirror] Checking for previous step failures before committing'
98+
)
99+
const failureCheck = await checkPreviousStepFailures()
100+
101+
if (failureCheck.error) {
102+
// If we can't determine failure status, skip commit to be safe
103+
shouldCommit = false
104+
skipReason = `Unable to check for step failures: ${failureCheck.error}`
105+
} else if (failureCheck.hasFailures) {
106+
shouldCommit = false
107+
skipReason = `Found ${failureCheck.failedCount} failed/cancelled steps`
108+
if (failureCheck.failedSteps) {
109+
for (const step of failureCheck.failedSteps) {
110+
core.warning(
111+
`[git-mirror] - Step: ${step.stepName || step.action || 'unknown'} (${step.result})`
112+
)
113+
}
104114
}
105115
}
106116
}
@@ -109,7 +119,7 @@ async function cleanup(): Promise<void> {
109119
core.warning(`[git-mirror] Skipping cache commit: ${skipReason}`)
110120
if (performedHydration) {
111121
core.warning(
112-
'[git-mirror] Initial hydration was in progress but job failed - backend will delete entry for retry'
122+
'[git-mirror] Initial hydration was performed but is not being committed - backend will delete entry for retry'
113123
)
114124
}
115125
} else {

src/state-helper.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,14 @@ export const BlacksmithCacheRepoUrl = core.getState('blacksmithCacheRepoUrl')
9898
export const BlacksmithCacheVerbose =
9999
core.getState('blacksmithCacheVerbose') === 'true'
100100

101+
/**
102+
* Reason the host gave at mount time for denying this job's sticky disk commit
103+
* (e.g. branch protection). Empty when no denial was reported.
104+
*/
105+
export const BlacksmithCacheCommitEarlyDenyReason = core.getState(
106+
'blacksmithCacheCommitEarlyDenyReason'
107+
)
108+
101109
/**
102110
* Save the repository path so the POST action can retrieve the value.
103111
*/
@@ -217,6 +225,14 @@ export function setBlacksmithCacheVerbose(verbose: boolean) {
217225
core.saveState('blacksmithCacheVerbose', verbose ? 'true' : 'false')
218226
}
219227

228+
/**
229+
* Save the host's early commit denial reason so the POST action can skip the
230+
* commit and explain why.
231+
*/
232+
export function setBlacksmithCacheCommitEarlyDenyReason(reason: string) {
233+
core.saveState('blacksmithCacheCommitEarlyDenyReason', reason)
234+
}
235+
220236
// Publish a variable so that when the POST action runs, it can determine it should run the cleanup logic.
221237
// This is necessary since we don't have a separate entry point.
222238
if (!IsPost) {

0 commit comments

Comments
 (0)