-
-
Notifications
You must be signed in to change notification settings - Fork 307
feat(orchestrator): build reliability features — git integrity, reserved filenames, archival #808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
4f07508
47670cf
40dd436
81ba9c3
e4fd1e6
3b8874f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -182,8 +182,8 @@ inputs: | |||||||||||||||||
| required: false | ||||||||||||||||||
| default: '' | ||||||||||||||||||
| description: | ||||||||||||||||||
| '[Orchestrator] Run a custom job instead of the standard build automation for orchestrator (in yaml format with the | ||||||||||||||||||
| keys image, secrets (name, value object array), command line string)' | ||||||||||||||||||
| '[Orchestrator] Run a custom job instead of the standard build automation for orchestrator (in yaml format with | ||||||||||||||||||
| the keys image, secrets (name, value object array), command line string)' | ||||||||||||||||||
| awsStackName: | ||||||||||||||||||
| default: 'game-ci' | ||||||||||||||||||
| required: false | ||||||||||||||||||
|
|
@@ -279,6 +279,30 @@ inputs: | |||||||||||||||||
| description: | ||||||||||||||||||
| '[Orchestrator] Specifies the repo for the unity builder. Useful if you forked the repo for testing, features, or | ||||||||||||||||||
| fixes.' | ||||||||||||||||||
| gitIntegrityCheck: | ||||||||||||||||||
| description: 'Run git integrity checks before build (fsck, lock cleanup, submodule validation)' | ||||||||||||||||||
| required: false | ||||||||||||||||||
| default: 'false' | ||||||||||||||||||
| gitAutoRecover: | ||||||||||||||||||
| description: 'Attempt automatic recovery if git corruption is detected' | ||||||||||||||||||
| required: false | ||||||||||||||||||
| default: 'false' | ||||||||||||||||||
|
Comment on lines
+286
to
+289
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Set At Line 289, the default is 🔧 Suggested change gitAutoRecover:
description: 'Attempt automatic recovery if git corruption is detected'
required: false
- default: 'false'
+ default: 'true'📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| cleanReservedFilenames: | ||||||||||||||||||
| description: 'Remove Windows reserved filenames that cause Unity import loops' | ||||||||||||||||||
| required: false | ||||||||||||||||||
| default: 'false' | ||||||||||||||||||
| buildArchiveEnabled: | ||||||||||||||||||
| description: 'Archive build output after successful build' | ||||||||||||||||||
| required: false | ||||||||||||||||||
| default: 'false' | ||||||||||||||||||
| buildArchivePath: | ||||||||||||||||||
| description: 'Path to store build archives' | ||||||||||||||||||
| required: false | ||||||||||||||||||
| default: './build-archives' | ||||||||||||||||||
| buildArchiveRetention: | ||||||||||||||||||
| description: 'Days to retain build archives before cleanup' | ||||||||||||||||||
| required: false | ||||||||||||||||||
| default: '30' | ||||||||||||||||||
|
|
||||||||||||||||||
| outputs: | ||||||||||||||||||
| volume: | ||||||||||||||||||
|
|
||||||||||||||||||
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { Action, BuildParameters, Cache, Orchestrator, Docker, ImageTag, Output | |
| import { Cli } from './model/cli/cli'; | ||
| import MacBuilder from './model/mac-builder'; | ||
| import PlatformSetup from './model/platform-setup'; | ||
| import { BuildReliabilityService } from './model/orchestrator/services/reliability'; | ||
|
|
||
| async function runMain() { | ||
| try { | ||
|
|
@@ -14,11 +15,38 @@ async function runMain() { | |
| Action.checkCompatibility(); | ||
| Cache.verify(); | ||
|
|
||
| // Always configure git environment for CI reliability | ||
| BuildReliabilityService.configureGitEnvironment(); | ||
|
|
||
| const { workspace, actionFolder } = Action; | ||
|
|
||
| const buildParameters = await BuildParameters.create(); | ||
| const baseImage = new ImageTag(buildParameters); | ||
|
|
||
| // Pre-build reliability checks | ||
| if (buildParameters.gitIntegrityCheck) { | ||
| core.info('Running git integrity checks...'); | ||
|
|
||
| const isHealthy = BuildReliabilityService.checkGitIntegrity(workspace); | ||
| BuildReliabilityService.cleanStaleLockFiles(workspace); | ||
| BuildReliabilityService.validateSubmoduleBackingStores(workspace); | ||
|
|
||
|
Comment on lines
+30
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Submodule validation result is ignored in the recovery gate. At Line 32, 🔧 Suggested update- const isHealthy = BuildReliabilityService.checkGitIntegrity(workspace);
- BuildReliabilityService.cleanStaleLockFiles(workspace);
- BuildReliabilityService.validateSubmoduleBackingStores(workspace);
+ BuildReliabilityService.cleanStaleLockFiles(workspace);
+ const brokenSubmodules = BuildReliabilityService.validateSubmoduleBackingStores(workspace);
+ const isHealthy =
+ BuildReliabilityService.checkGitIntegrity(workspace) && brokenSubmodules.length === 0;Also applies to: 38-44 🤖 Prompt for AI Agents |
||
| if (buildParameters.cleanReservedFilenames) { | ||
| BuildReliabilityService.cleanReservedFilenames(buildParameters.projectPath); | ||
| } | ||
|
|
||
| if (!isHealthy && buildParameters.gitAutoRecover) { | ||
| core.info('Git corruption detected, attempting automatic recovery...'); | ||
| const recovered = BuildReliabilityService.recoverCorruptedRepo(workspace); | ||
| if (!recovered) { | ||
| core.warning('Automatic recovery failed. Build may encounter issues.'); | ||
| } | ||
| } | ||
| } else if (buildParameters.cleanReservedFilenames) { | ||
| // cleanReservedFilenames can run independently of gitIntegrityCheck | ||
| BuildReliabilityService.cleanReservedFilenames(buildParameters.projectPath); | ||
| } | ||
|
|
||
| let exitCode = -1; | ||
|
|
||
| if (buildParameters.providerStrategy === 'local') { | ||
|
|
@@ -37,6 +65,13 @@ async function runMain() { | |
| exitCode = 0; | ||
| } | ||
|
|
||
| // Post-build: archive and enforce retention | ||
| if (buildParameters.buildArchiveEnabled && exitCode === 0) { | ||
| core.info('Archiving build output...'); | ||
| BuildReliabilityService.archiveBuildOutput(buildParameters.buildPath, buildParameters.buildArchivePath); | ||
| BuildReliabilityService.enforceRetention(buildParameters.buildArchivePath, buildParameters.buildArchiveRetention); | ||
| } | ||
|
|
||
| // Set output | ||
| await Output.setBuildVersion(buildParameters.buildVersion); | ||
| await Output.setAndroidVersionCode(buildParameters.androidVersionCode); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -282,6 +282,38 @@ class Input { | |||||||||||||||
| return Input.getInput('skipActivation')?.toLowerCase() ?? 'false'; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| static get gitIntegrityCheck(): boolean { | ||||||||||||||||
| const input = Input.getInput('gitIntegrityCheck') ?? 'false'; | ||||||||||||||||
|
|
||||||||||||||||
| return input === 'true'; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| static get gitAutoRecover(): boolean { | ||||||||||||||||
| const input = Input.getInput('gitAutoRecover') ?? 'false'; | ||||||||||||||||
|
|
||||||||||||||||
| return input === 'true'; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| static get cleanReservedFilenames(): boolean { | ||||||||||||||||
| const input = Input.getInput('cleanReservedFilenames') ?? 'false'; | ||||||||||||||||
|
|
||||||||||||||||
| return input === 'true'; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| static get buildArchiveEnabled(): boolean { | ||||||||||||||||
| const input = Input.getInput('buildArchiveEnabled') ?? 'false'; | ||||||||||||||||
|
|
||||||||||||||||
| return input === 'true'; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| static get buildArchivePath(): string { | ||||||||||||||||
| return Input.getInput('buildArchivePath') ?? './build-archives'; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| static get buildArchiveRetention(): number { | ||||||||||||||||
| return Number.parseInt(Input.getInput('buildArchiveRetention') ?? '30', 10); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+313
to
+315
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If the user provides a non-numeric string (e.g., 🛡️ Suggested fix with fallback static get buildArchiveRetention(): number {
- return Number.parseInt(Input.getInput('buildArchiveRetention') ?? '3', 10);
+ const parsed = Number.parseInt(Input.getInput('buildArchiveRetention') ?? '3', 10);
+ return Number.isNaN(parsed) || parsed < 0 ? 3 : parsed;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
|
|
||||||||||||||||
| public static ToEnvVarFormat(input: string) { | ||||||||||||||||
| if (input.toUpperCase() === input) { | ||||||||||||||||
| return input; | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent error handling across platform workflows.
Adding
continue-on-error: trueat the job level means macOS build failures won't fail the overall workflow, while Windows and Ubuntu workflows (per the context snippets) will still fail on errors. This asymmetry could mask real macOS build failures and allow PRs to merge with broken builds.This change also doesn't appear related to the PR's stated objectives (git integrity checks, reserved filename cleanup, build archival). If this is intentional for testing the new reliability features, consider:
🤖 Prompt for AI Agents