feat: Linux CI-first packaging, preview, and modular policy groundwork#118
feat: Linux CI-first packaging, preview, and modular policy groundwork#118oladapodev wants to merge 15 commits into
Conversation
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis PR adds Linux media-policy contracts, Debian packaging, artifact metadata and validation scripts, Linux CI and release workflows, upstream synchronization automation, and architecture, acceptance, distribution, and release documentation. ChangesLinux media-policy contracts
Linux packaging and delivery
CI and releases
Documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant ElectronBuilder
participant LinuxHealthSmoke
participant GitHubRelease
GitHubActions->>ElectronBuilder: Build Linux Debian package
ElectronBuilder->>LinuxHealthSmoke: Provide packaged executable and artifacts
LinuxHealthSmoke->>GitHubActions: Validate recording artifacts and metadata
GitHubActions->>GitHubRelease: Upload preview, prerelease, or immutable release artifacts
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR lays groundwork for a CI-first Linux packaging/release pipeline (Debian .deb) and introduces initial Linux acceptance/metadata tooling plus shared “media policy” protocol contract scaffolding across Rust + TypeScript.
Changes:
- Added GitHub Actions workflows for Linux gates, PR preview packaging, main prerelease publishing, tagged Linux releases, and upstream sync automation.
- Added Linux packaging support scripts (preflight, artifact validation, build metadata, capabilities, notices) and a packaged-app health smoke.
- Added Linux port planning/acceptance documentation and introduced new media-policy protocol fixture plumbing and type/schema scaffolding.
Reviewed changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/validate-linux-artifacts.mjs | Validates presence/readability of required Linux release artifacts. |
| scripts/smoke-packaged-linux-health.mjs | Runs a packaged Linux health smoke and validates output media via ffprobe. |
| scripts/preflight-linux-package.mjs | CLI entrypoint for Linux packaging preflight. |
| scripts/lib/linux-package-preflight.test.mjs | Tests Linux package preflight validation behavior. |
| scripts/lib/linux-package-preflight.mjs | Validates required binaries/resources for Linux packaging. |
| scripts/lib/linux-capabilities.test.mjs | Tests generation of Linux capabilities payload. |
| scripts/lib/linux-capabilities.mjs | Generates Linux capability manifest payload + runtime probe reader. |
| scripts/lib/linux-build-info.test.mjs | Tests Linux build-info generation and writing. |
| scripts/lib/linux-build-info.mjs | Generates Linux build metadata + checksum writing helpers. |
| scripts/lib/linux-acceptance-gate.test.mjs | Tests Linux acceptance manifest validator and CLI gate. |
| scripts/lib/linux-acceptance-gate.mjs | Validates Linux acceptance evidence manifest and provides CLI gate. |
| scripts/build-linux-notices.mjs | Builds THIRD-PARTY-NOTICES artifact for Linux packages. |
| scripts/build-linux-capabilities.mjs | Writes linux-capabilities JSON (+ checksum). |
| scripts/build-linux-build-info.mjs | CLI wrapper to write linux build-info JSON. |
| protocol-fixtures/high-risk-contracts.json | Adds mediaPolicyProfile wire/normalized fixtures. |
| package.json | Adds Linux packaging/dist scripts and Linux validation/smoke commands. |
| docs/releases/release-runbook.md | Extends runbook to include Linux release flow via workflows. |
| docs/linux-port-tasks.md | Adds detailed Linux slice/task checklist and verification matrix. |
| docs/linux-port-plan.md | Adds master Linux delivery plan, phases, and gates. |
| docs/linux-architecture.md | Adds Linux architecture spec and modular policy/capture/preview/encode plan. |
| docs/distribution.md | Documents Linux distribution channel via workflows and artifact requirements. |
| docs/compose/plans/2026-07-13-linux-slice-1-shared-settings-contract.md | Slice plan for shared settings/protocol contract work. |
| docs/compose/plans/2026-07-13-linux-slice-0-acceptance-contract.md | Slice plan for Linux acceptance contract/gate work. |
| docs/acceptance/linux-app-acceptance-template.md | Adds Linux acceptance evidence template. |
| crates/videorc-backend/src/protocol.rs | Adds media_policy to StartSessionParams + fixture test. |
| crates/videorc-backend/src/media_policy/settings.rs | Defines Rust media-policy types used by protocol/fixtures. |
| crates/videorc-backend/src/media_policy/selection.rs | Adds preset normalization helper and test scaffold. |
| crates/videorc-backend/src/media_policy/mod.rs | Exposes new media_policy module surface. |
| crates/videorc-backend/src/media_policy/fallback.rs | Adds initial fallback policy enum/helpers scaffold. |
| crates/videorc-backend/src/main.rs | Wires media_policy module into backend crate. |
| apps/desktop/src/shared/protocol-contract-fixtures.test.ts | Extends fixture typing and validates mediaPolicy fixture round-trip. |
| apps/desktop/src/shared/backend.ts | Adds TS media-policy types and StartSessionParams field. |
| apps/desktop/src/shared/backend-rpc-contract.ts | Adds runtime schema for mediaPolicy in session.start params. |
| apps/desktop/src/shared/backend-rpc-contract.test.ts | Adds contract test for mediaPolicy normalization. |
| apps/desktop/electron-builder.yml | Adds Linux deb packaging config + bundles backend/ffmpeg resources. |
| .gitignore | Ignores Linux acceptance artifacts directory. |
| .github/workflows/upstream-sync.yml | Scheduled/dispatch workflow to sync upstream into a PR. |
| .github/workflows/release-linux.yml | Workflow to build, validate, and publish tagged Linux releases. |
| .github/workflows/linux.yml | Workflow for Linux gates + PR packaging + main prerelease flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct MediaPolicySelection { | ||
| #[serde(default)] | ||
| pub requested: MediaPolicyRequest, | ||
| #[serde(default)] | ||
| pub selected: MediaBackendPolicy, | ||
| #[serde(default)] | ||
| pub fallback_mode: MediaPolicyFallbackMode, | ||
| #[serde(default)] | ||
| pub fallback_reason: Option<String>, | ||
| #[serde(default)] | ||
| pub capability_verdict: MediaPolicyCapabilityVerdict, | ||
| #[serde(default)] | ||
| pub observed_runtime_path: Option<String>, | ||
| #[serde(default)] | ||
| pub hardware_fingerprint: Option<String>, | ||
| #[serde(default)] | ||
| pub benchmark_recommendation: Option<BenchmarkPreset>, | ||
| } |
| const ffprobe = process.env.VIDEORC_SMOKE_FFPROBE_PATH || 'ffprobe' | ||
| if (!existsSync(ffprobe)) { | ||
| console.error(`ffprobe not found: ${ffprobe}`) | ||
| process.exit(1) | ||
| } |
| const fallback = [ | ||
| 'THIRD-PARTY-NOTICES.txt', | ||
| '', | ||
| 'No packaged FFmpeg notice files were discoverable at build time.', | ||
| 'The project still includes source and license files in vendor/ffmpeg/current for audit.', | ||
| `Generated at: ${new Date().toISOString()}` | ||
| ] | ||
|
|
||
| const content = [ | ||
| 'Videorc Linux third-party notices', | ||
| 'Includes bundled FFmpeg notices and configured build artifacts when available.', | ||
| '', | ||
| ...ffmpegNotices, | ||
| '', | ||
| ...fallback | ||
| ].join('\n') |
| export function validateLinuxPackagePreflight(manifest = {}) { | ||
| const failures = [] | ||
|
|
||
| const backendPath = manifest.backendPath || resolveBackendBinaryPath({ repoRoot: manifest.repoRoot }) |
| const shaPath = `${outDir}.sha256` | ||
| try { | ||
| const binary = readFileSync(artifactPath) | ||
| const { createHash } = await import('node:crypto') | ||
| const checksum = createHash('sha256') | ||
| checksum.update(binary) | ||
| await writeFile(shaPath, `${checksum.digest('hex')} ${artifactPath}\\n`, 'utf8') | ||
| } catch { | ||
| // Optional checksum file. Build pipelines treat this as informational. | ||
| } |
| - name: Run full Linux gates | ||
| run: | | ||
| cargo fmt --check --all | ||
| cargo audit --deny warnings | ||
| cargo clippy -p videorc-backend -- -D warnings | ||
| cargo test -p videorc-backend | ||
| pnpm audit:js | ||
| pnpm format:check | ||
| pnpm typecheck | ||
| pnpm --filter @videorc/desktop test | ||
|
|
| exit 0 | ||
| fi | ||
|
|
||
| OPEN_PR_NUMBER="$(gh pr list --head sync/upstream- --state open --json number --jq '.[0].number' || true)" |
| "package:desktop": "pnpm package:backend:macos && pnpm ffmpeg:build:macos && pnpm package:preflight:macos && pnpm --filter @videorc/desktop package", | ||
| "package:desktop:linux": "pnpm package:backend && pnpm package:preflight:linux && pnpm --filter @videorc/desktop package --linux", | ||
| "package:desktop:signed-local": "pnpm package:desktop && pnpm sign:desktop:local", | ||
| "dist:desktop:signed-local": "pnpm package:desktop:signed-local && cd apps/desktop && pnpm exec electron-builder --mac dmg --prepackaged release/mac-arm64 --config.mac.identity=null --config.mac.notarize=false --config.dmg.sign=false && cd ../.. && pnpm release:manifest:macos", | ||
| "package:desktop:windows": "pnpm package:backend && pnpm ffmpeg:fetch:windows && pnpm package:preflight:windows && pnpm --filter @videorc/desktop package", | ||
| "dist:desktop:windows": "pnpm package:backend && pnpm ffmpeg:fetch:windows && pnpm package:preflight:windows && pnpm --filter @videorc/desktop dist", | ||
| "dist:desktop:linux": "pnpm package:desktop:linux && pnpm --filter @videorc/desktop dist --linux --publish never", |
| linux: | ||
| icon: build-resources/icon.icns | ||
| category: AudioVideo | ||
| executableName: videorc |
7a18a90 to
215563f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (7)
apps/desktop/src/shared/backend-rpc-contract.ts (2)
325-329: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
requestedandselectedoverrides inmediaPolicySelectionSchemareturn.
return { ...record, requested: record.requested, selected: record.selected }is a no-op — the spread already includes both fields. This is dead code that may confuse future readers into thinking a transformation is happening.♻️ Remove redundant overrides
return { ...record, - requested: record.requested, - selected: record.selected }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/shared/backend-rpc-contract.ts` around lines 325 - 329, In the mediaPolicySelectionSchema return, remove the redundant requested and selected property overrides and return the spread record directly, preserving all existing fields and behavior.
197-282: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
normalizeEnumfallback reasons are silently discarded.
normalizeEnumreturns{ value, fallbackReason? }, but every call site inmediaBackendPolicySchemaandmediaPolicyRequestSchemauses only.value, discardingfallbackReason. When a backend field is unrecognized and silently replaced with a default, no diagnostic is propagated. This violates the guideline to prefer explicit diagnostics over silent fallbacks.Consider aggregating fallback reasons and surfacing them, or at minimum logging when normalization occurs.
As per coding guidelines: "Prefer explicit status and diagnostics over silent fallbacks in TypeScript UI and application code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/shared/backend-rpc-contract.ts` around lines 197 - 282, Propagate the fallbackReason values returned by normalizeEnum through mediaBackendPolicySchema and the related mediaPolicyRequestSchema normalization flow instead of discarding them. Aggregate diagnostics for every unrecognized backend field and expose or log them through the existing schema/request result mechanism, while preserving the current fallback values and successful validation behavior.Source: Coding guidelines
apps/desktop/src/shared/backend-rpc-contract.test.ts (1)
333-345: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStore validation result instead of calling
validateBackendRpcParamstwice.The test calls
validateBackendRpcParams('session.start', validSessionStartParams)twice — once for thetoMatchObjectassertion and once for thetoBeassertion. Store the result in a variable for clarity and to avoid redundant validation.♻️ Store result in a variable
- expect( - validateBackendRpcParams('session.start', validSessionStartParams) - ).toMatchObject({ - mediaPolicy: { - requested: { - intent: 'automatic' - } - } - }) - expect( - (validateBackendRpcParams('session.start', validSessionStartParams) as { mediaPolicy?: { selected?: { capture?: string } } }) - .mediaPolicy?.selected?.capture - ).toBe('pipewire-portal') + const result = validateBackendRpcParams('session.start', validSessionStartParams) as { + mediaPolicy?: { requested?: { intent?: string }; selected?: { capture?: string } } + } + expect(result).toMatchObject({ + mediaPolicy: { requested: { intent: 'automatic' } } + }) + expect(result.mediaPolicy?.selected?.capture).toBe('pipewire-portal')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/shared/backend-rpc-contract.test.ts` around lines 333 - 345, Update the test around validateBackendRpcParams by storing its session.start validation result in a local variable, then reuse that variable for both the toMatchObject and selected.capture assertions instead of invoking validation twice.scripts/build-linux-capabilities.mjs (1)
28-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRead/parse failures are silently swallowed across the Linux capabilities metadata pipeline. Both sites catch all errors and fall back to defaults with no logged diagnostic, so a malformed
package.jsonor runtime-capabilities probe file quietly produces an incomplete release artifact instead of surfacing the failure in CI logs.
scripts/build-linux-capabilities.mjs#L28-L45: log a warning (including the caught error) before falling back toruntimeCapabilities.scripts/lib/linux-capabilities.mjs#L75-L85: log a warning (including the caught error) before returningnullfromreadRuntimeCapabilities.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/build-linux-capabilities.mjs` around lines 28 - 45, Log a warning with the caught error before each fallback: in scripts/build-linux-capabilities.mjs lines 28-45, update the buildInfo catch block to warn before returning runtimeCapabilities; in scripts/lib/linux-capabilities.mjs lines 75-85, update readRuntimeCapabilities to warn before returning null. Preserve both existing fallback values and use the surrounding logging mechanism.crates/videorc-backend/src/media_policy/fallback.rs (1)
20-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing unit tests for this pure helper.
select_benchmark_recommendationhas no accompanying tests, unlike the sibling helpers inselection.rs. Given the branching logic (three outcomes across two booleans), a small test module would catch regressions and clarify intended behavior once the type/wiring issue above is resolved.As per coding guidelines, "prefer small pure helpers for behavior that needs tests."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/videorc-backend/src/media_policy/fallback.rs` around lines 20 - 31, Add unit tests for the pure helper select_benchmark_recommendation, covering all boolean combinations and asserting the expected FallbackPolicy: AskFirst when requested is true but allowed is false, SafeAuto when allowed is true, and Custom when both are false. Follow the existing sibling-helper test structure in selection.rs.scripts/lib/linux-build-info.mjs (1)
66-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse static import for
writeFileinstead of repeated dynamic imports.
readFileis already statically imported fromnode:fs/promisesat line 1, butwriteFileis fetched via two separate dynamicimport('node:fs/promises').then(...)calls. AddingwriteFileto the static import eliminates the dynamic import overhead and the duplicated pattern.♻️ Proposed refactor
-import { readFile } from 'node:fs/promises' +import { readFile, writeFile } from 'node:fs/promises'export async function writeLinuxBuildInfo({ artifactPath, outputPath, repoRoot = process.cwd() }) { const buildInfo = await buildLinuxBuildInfo({ artifactPath, repoRoot }) - await import('node:fs/promises').then(async ({ writeFile }) => { - await writeFile(outputPath, JSON.stringify(buildInfo, null, 2) + '\n', 'utf8') - }) + await writeFile(outputPath, JSON.stringify(buildInfo, null, 2) + '\n', 'utf8') if (buildInfo.artifacts.mainDeb?.sha256) { - await import('node:fs/promises').then(async ({ writeFile }) => { - await writeFile( - buildInfo.artifacts.mainDeb.sha256File, - `${buildInfo.artifacts.mainDeb.sha256} ${artifactPath}\n`, - 'utf8' - ) - }) + await writeFile( + buildInfo.artifacts.mainDeb.sha256File, + `${buildInfo.artifacts.mainDeb.sha256} ${artifactPath}\n`, + 'utf8' + ) } return buildInfo }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/linux-build-info.mjs` around lines 66 - 81, Update the static node:fs/promises import to include writeFile, then replace both dynamic import blocks in writeLinuxBuildInfo with direct writeFile calls while preserving their existing paths, contents, encoding, and ordering.scripts/lib/linux-build-info.test.mjs (1)
37-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for the
.sha256sidecar file.
writeLinuxBuildInfowrites both the JSON output and a${artifactPath}.sha256sidecar file, but the test only verifies the JSON. Adding a sidecar assertion would confirm the checksum file is created with the expected"<hash> <path>"format.♻️ Proposed addition
await writeLinuxBuildInfo({ artifactPath, outputPath, repoRoot: root }) const written = JSON.parse(await readFile(outputPath, 'utf8')) assert.equal(written.artifacts.mainDeb.sha256.length > 0, true) + + const sidecar = await readFile(`${artifactPath}.sha256`, 'utf8') + assert.ok(sidecar.includes(written.artifacts.mainDeb.sha256)) + assert.ok(sidecar.includes(artifactPath)) } finally {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/linux-build-info.test.mjs` around lines 37 - 52, Add coverage in the “writes build-info and checksum files” test for the `${artifactPath}.sha256` sidecar produced by writeLinuxBuildInfo. Read the sidecar and assert it contains the expected “<hash> <path>” format, using the recorded artifacts.mainDeb.sha256 value and artifact path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-linux.yml:
- Around line 112-125: Update the release-notes header expression in the
workflow block to use GitHub Actions’ positional format placeholder `{0}`
instead of `%s`, so the provided `github.event.inputs.release_id` is
interpolated correctly while preserving the existing conditional suffix
behavior.
- Around line 128-142: Update the “Publish immutable Linux release” step to
avoid using GITHUB_REF_NAME as a release tag for workflow_dispatch runs. Require
an explicit release-tag input for manual dispatches, or skip publishing when the
ref is not a tag; preserve the existing immutability check and gh release create
flow for valid release tags.
- Around line 112-117: Update the release-notes step around the shell block to
pass github.event.inputs.release_id through an env entry named RELEASE_ID, then
reference $RELEASE_ID in the release ID portion of the echoed title instead of
interpolating it directly in run. Leave the other GitHub metadata and notes_path
handling unchanged.
- Around line 27-28: Update the checkout step using actions/checkout@v4 to set
persist-credentials to false, ensuring the repository token is not retained in
git configuration for subsequent workflow steps.
In @.github/workflows/upstream-sync.yml:
- Around line 35-53: Move the no-op detection in the “Open or update upstream
sync PR” flow ahead of creating and pushing the sync branch, so the workflow
exits before the `git checkout -b`, merge, and `git push` commands when
`upstream/main` is already contained in `HEAD`; retain the existing branch
creation and merge behavior only when a sync is needed.
In `@apps/desktop/electron-builder.yml`:
- Around line 135-143: Update the Linux FFmpeg extraResources filter under the
ffmpeg mapping to include NOTICE.txt alongside the existing LICENSE.txt and
other metadata files, matching the macOS resource set.
In `@crates/videorc-backend/src/media_policy/fallback.rs`:
- Around line 5-31: Update MediaPolicySelection in selection.rs to derive its
benchmark recommendation from the requested and allowed inputs instead of always
using BenchmarkPreset::Balanced. Modify select_benchmark_recommendation in
fallback.rs to return the benchmark preset and reason pair required by that
field, or remove it if no longer needed, and wire the resulting recommendation
through every selection path. Apply the changes in
crates/videorc-backend/src/media_policy/fallback.rs lines 5-31 and
crates/videorc-backend/src/media_policy/selection.rs line 61.
In `@crates/videorc-backend/src/media_policy/selection.rs`:
- Around line 47-69: Fix normalized_selection_from_wire by avoiding the second
access to result.selected_fallback_reason after moving it into reason. Clone the
fallback reason once before constructing MediaPolicySelection, then use that
value directly for both policy.fallback_reason and the returned Option, removing
the redundant if/else branch.
In `@crates/videorc-backend/src/media_policy/settings.rs`:
- Around line 162-199: Unknown JSON fields are currently accepted by the media
policy wire structs. Add serde deny_unknown_fields to MediaPolicyRequest and
MediaPolicySelection in crates/videorc-backend/src/media_policy/settings.rs; the
protocol.rs test site at lines 3540-3575 requires no direct change and should
now pass through the struct-level validation.
- Around line 135-199: Add serde skip_serializing_if attributes to every
optional Option field in MediaBackendPolicy and MediaPolicySelection so absent
values are omitted rather than serialized as null; leave MediaPolicyRequest
unchanged, and verify the protocol.rs round-trip matches the fixture
expectations.
In `@crates/videorc-backend/src/protocol.rs`:
- Around line 3540-3575: Update the serde definitions in settings.rs: add
skip_serializing_if = "Option::is_none" to optional fields so normalized
serialization omits None values, and add deny_unknown_fields to
MediaPolicySelection so unexpected wire fields are rejected. In
shared_high_risk_contract_fixture_includes_media_policy_contract, clarify the
missing-requested case by separating it from the invalid capabilityVerdict enum
case; preserve #[serde(default)] behavior unless missing requested is explicitly
intended to be rejected.
In `@docs/acceptance/linux-app-acceptance-template.md`:
- Around line 127-129: Update the Rollback command in the acceptance template to
use a valid local .deb installation invocation, either by prefixing the package
path with ./ for apt or by using dpkg -i consistently with the install command.
Keep the existing /opt package-cache restoration alternative unchanged.
- Around line 21-26: Update the Linux acceptance template around the verifier
command and checklist to document the required manifest schema: add or link a
canonical minimal manifest.json showing named sections such as gnome-wayland,
portal, and pipewire, each with non-empty evidence arrays, and map every
checklist item to its exact manifest key.
In `@docs/compose/plans/2026-07-13-linux-slice-0-acceptance-contract.md`:
- Line 19: Fix the heading hierarchy by changing the Task 1 heading to an H2
(`##`) or by adding an intervening `## Tasks` parent section before the existing
task heading, ensuring no H3 directly follows the document’s H1.
In `@docs/compose/plans/2026-07-13-linux-slice-1-shared-settings-contract.md`:
- Line 18: Update the Task 1 heading in the document to use the correct
hierarchy by changing it from an H3 to an H2, or place it under a new parent H2
Tasks section while preserving the existing task content.
In `@docs/linux-port-tasks.md`:
- Around line 62-87: Update the checklist in the task section so it does not
claim completion while “Verify RED” remains unchecked. Either add the required
RED verification evidence and mark it [x], or uncheck the later GREEN, template,
ignore, command, and final verification items until RED verification is
complete.
In `@docs/releases/release-runbook.md`:
- Around line 112-119: Update the tagged Linux release flow in release-linux.yml
so build-info.json, linux-capabilities.json, and THIRD-PARTY-NOTICES.txt are
generated before scripts/validate-linux-artifacts.mjs runs. Preserve validation
of all required metadata before upload, and keep the release-runbook sequence
aligned with the corrected workflow.
In `@scripts/build-linux-capabilities.mjs`:
- Around line 61-67: Update the checksum content written by the checksum
generation flow around `shaPath` and `checksum.digest` so it appends an actual
newline character rather than the literal backslash-n sequence. Preserve the
existing SHA-256 format and artifact path.
In `@scripts/build-linux-notices.mjs`:
- Around line 26-41: Update the content assembly in the build notice generation
flow so the fallback messages are included only when ffmpegNotices is empty.
Preserve the discovered FFmpeg notices and existing header when notices are
present, while retaining the fallback output for the no-notices case.
---
Nitpick comments:
In `@apps/desktop/src/shared/backend-rpc-contract.test.ts`:
- Around line 333-345: Update the test around validateBackendRpcParams by
storing its session.start validation result in a local variable, then reuse that
variable for both the toMatchObject and selected.capture assertions instead of
invoking validation twice.
In `@apps/desktop/src/shared/backend-rpc-contract.ts`:
- Around line 325-329: In the mediaPolicySelectionSchema return, remove the
redundant requested and selected property overrides and return the spread record
directly, preserving all existing fields and behavior.
- Around line 197-282: Propagate the fallbackReason values returned by
normalizeEnum through mediaBackendPolicySchema and the related
mediaPolicyRequestSchema normalization flow instead of discarding them.
Aggregate diagnostics for every unrecognized backend field and expose or log
them through the existing schema/request result mechanism, while preserving the
current fallback values and successful validation behavior.
In `@crates/videorc-backend/src/media_policy/fallback.rs`:
- Around line 20-31: Add unit tests for the pure helper
select_benchmark_recommendation, covering all boolean combinations and asserting
the expected FallbackPolicy: AskFirst when requested is true but allowed is
false, SafeAuto when allowed is true, and Custom when both are false. Follow the
existing sibling-helper test structure in selection.rs.
In `@scripts/build-linux-capabilities.mjs`:
- Around line 28-45: Log a warning with the caught error before each fallback:
in scripts/build-linux-capabilities.mjs lines 28-45, update the buildInfo catch
block to warn before returning runtimeCapabilities; in
scripts/lib/linux-capabilities.mjs lines 75-85, update readRuntimeCapabilities
to warn before returning null. Preserve both existing fallback values and use
the surrounding logging mechanism.
In `@scripts/lib/linux-build-info.mjs`:
- Around line 66-81: Update the static node:fs/promises import to include
writeFile, then replace both dynamic import blocks in writeLinuxBuildInfo with
direct writeFile calls while preserving their existing paths, contents,
encoding, and ordering.
In `@scripts/lib/linux-build-info.test.mjs`:
- Around line 37-52: Add coverage in the “writes build-info and checksum files”
test for the `${artifactPath}.sha256` sidecar produced by writeLinuxBuildInfo.
Read the sidecar and assert it contains the expected “<hash> <path>” format,
using the recorded artifacts.mainDeb.sha256 value and artifact path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b863818a-5bae-4506-a9b3-28c6c11fa809
📒 Files selected for processing (40)
.github/workflows/ci.yml.github/workflows/linux.yml.github/workflows/release-linux.yml.github/workflows/upstream-sync.yml.gitignoreapps/desktop/electron-builder.ymlapps/desktop/src/shared/backend-rpc-contract.test.tsapps/desktop/src/shared/backend-rpc-contract.tsapps/desktop/src/shared/backend.tsapps/desktop/src/shared/protocol-contract-fixtures.test.tscrates/videorc-backend/src/main.rscrates/videorc-backend/src/media_policy/fallback.rscrates/videorc-backend/src/media_policy/mod.rscrates/videorc-backend/src/media_policy/selection.rscrates/videorc-backend/src/media_policy/settings.rscrates/videorc-backend/src/protocol.rsdocs/acceptance/linux-app-acceptance-template.mddocs/compose/plans/2026-07-13-linux-slice-0-acceptance-contract.mddocs/compose/plans/2026-07-13-linux-slice-1-shared-settings-contract.mddocs/distribution.mddocs/linux-architecture.mddocs/linux-port-plan.mddocs/linux-port-tasks.mddocs/releases/release-runbook.mdpackage.jsonprotocol-fixtures/high-risk-contracts.jsonscripts/build-linux-build-info.mjsscripts/build-linux-capabilities.mjsscripts/build-linux-notices.mjsscripts/lib/linux-acceptance-gate.mjsscripts/lib/linux-acceptance-gate.test.mjsscripts/lib/linux-build-info.mjsscripts/lib/linux-build-info.test.mjsscripts/lib/linux-capabilities.mjsscripts/lib/linux-capabilities.test.mjsscripts/lib/linux-package-preflight.mjsscripts/lib/linux-package-preflight.test.mjsscripts/preflight-linux-package.mjsscripts/smoke-packaged-linux-health.mjsscripts/validate-linux-artifacts.mjs
| - name: Check out repository | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the workflow and inspect the relevant section with line numbers.
git ls-files '.github/workflows/release-linux.yml'
wc -l .github/workflows/release-linux.yml
sed -n '1,220p' .github/workflows/release-linux.yml | cat -n
# Show nearby usage of checkout-related settings and any token-sensitive steps.
rg -n "checkout|persist-credentials|contents: write|gh release|actions/|cargo|pnpm|token" .github/workflows/release-linux.ymlRepository: TheOrcDev/videorc
Length of output: 7376
Disable persisted checkout credentials
actions/checkout@v4 leaves the repo token in git config by default. With contents: write and several dependency/install steps afterward, a compromised step could reuse that token. Add persist-credentials: false here.
Suggested change
- name: Check out repository
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Check out repository | |
| uses: actions/checkout@v4 | |
| - name: Check out repository | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 27-28: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-linux.yml around lines 27 - 28, Update the
checkout step using actions/checkout@v4 to set persist-credentials to false,
ensuring the repository token is not retained in git configuration for
subsequent workflow steps.
Source: Linters/SAST tools
| { | ||
| echo "# Linux release ${{ github.ref_name }}${{ github.event.inputs.release_id != '' && format(' (%s)', github.event.inputs.release_id) || '' }}" | ||
| echo | ||
| echo "- Commit: ${{ github.sha }}" | ||
| echo "- Ref: ${{ github.ref }}" | ||
| echo "- Actor: ${{ github.actor }}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files '.github/workflows/release-linux.yml'
wc -l .github/workflows/release-linux.yml
cat -n .github/workflows/release-linux.yml | sed -n '100,150p'Repository: TheOrcDev/videorc
Length of output: 2643
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n .github/workflows/release-linux.yml | sed -n '1,110p'Repository: TheOrcDev/videorc
Length of output: 4439
Move the user-controlled release ID out of the shell expression In the release-notes step, github.event.inputs.release_id is interpolated directly into run:. Bind it via env: and use $RELEASE_ID instead; that’s the only untrusted value here. The other GitHub metadata and notes_path output are constrained and don’t need the same treatment.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 113-113: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 113-113: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 116-116: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 117-117: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-linux.yml around lines 112 - 117, Update the
release-notes step around the shell block to pass github.event.inputs.release_id
through an env entry named RELEASE_ID, then reference $RELEASE_ID in the release
ID portion of the echoed title instead of interpolating it directly in run.
Leave the other GitHub metadata and notes_path handling unchanged.
Source: Linters/SAST tools
| { | ||
| echo "# Linux release ${{ github.ref_name }}${{ github.event.inputs.release_id != '' && format(' (%s)', github.event.inputs.release_id) || '' }}" | ||
| echo | ||
| echo "- Commit: ${{ github.sha }}" | ||
| echo "- Ref: ${{ github.ref }}" | ||
| echo "- Actor: ${{ github.actor }}" | ||
| echo "- Channel: tagged release" | ||
| echo | ||
| echo "## Commit / PR changes" | ||
| git log --no-merges --pretty=format:'- %h %s' "$RANGE" | ||
| echo | ||
| echo "## Artifact checksums" | ||
| sha256sum apps/desktop/release/*.deb apps/desktop/release/*.sha256 | ||
| } > "$NOTES" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant workflow section with line numbers.
sed -n '100,135p' .github/workflows/release-linux.ymlRepository: TheOrcDev/videorc
Length of output: 1686
🌐 Web query:
GitHub Actions expressions format function placeholders {0} {1} documentation
💡 Result:
In GitHub Actions, the format function allows you to insert variable values into a string using positional placeholders [1][2]. The syntax for the function is format(string, replaceValue0, replaceValue1,..., replaceValueN) [1][3]. Key details regarding placeholders: - Placeholders are specified within the string using the syntax {N}, where N is an integer corresponding to the index of the replaceValue provided (starting from 0) [1][2]. - There is no theoretical maximum number of variables (replaceValueN) you can use [1][3]. - You must provide at least one replaceValue and the string itself [1][3]. Example usage: ${{ format('Hello {0} {1} {2}', 'Mona', 'the', 'Octocat') }} This returns the string: 'Hello Mona the Octocat' [4][1]. Escaping curly braces: If you need to include literal curly braces in your string, you can escape them by doubling them [1][3]. For example: ${{ format('{{Hello {0}}}', 'World') }} This returns the string: '{Hello World}' [1][3].
Citations:
- 1: https://docs.github.com/en/enterprise-cloud@latest/actions/reference/workflows-and-actions/expressions
- 2: https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/expressions.md
- 3: https://docs.github.com/en/enterprise-server@2.22/actions/learn-github-actions/expressions
- 4: https://docs.github.com/actions/reference/evaluate-expressions-in-workflows-and-actions
Use {0} in format()
GitHub Actions format() uses {0}, {1}, … placeholders, so format(' (%s)', github.event.inputs.release_id) leaves the literal %s in the release notes header instead of inserting release_id.
Proposed fix
- echo "# Linux release ${{ github.ref_name }}${{ github.event.inputs.release_id != '' && format(' (%s)', github.event.inputs.release_id) || '' }}"
+ echo "# Linux release ${{ github.ref_name }}${{ github.event.inputs.release_id != '' && format(' ({0})', github.event.inputs.release_id) || '' }}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| echo "# Linux release ${{ github.ref_name }}${{ github.event.inputs.release_id != '' && format(' (%s)', github.event.inputs.release_id) || '' }}" | |
| echo | |
| echo "- Commit: ${{ github.sha }}" | |
| echo "- Ref: ${{ github.ref }}" | |
| echo "- Actor: ${{ github.actor }}" | |
| echo "- Channel: tagged release" | |
| echo | |
| echo "## Commit / PR changes" | |
| git log --no-merges --pretty=format:'- %h %s' "$RANGE" | |
| echo | |
| echo "## Artifact checksums" | |
| sha256sum apps/desktop/release/*.deb apps/desktop/release/*.sha256 | |
| } > "$NOTES" | |
| echo "# Linux release ${{ github.ref_name }}${{ github.event.inputs.release_id != '' && format(' ({0})', github.event.inputs.release_id) || '' }}" |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 113-113: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 113-113: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 116-116: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 117-117: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-linux.yml around lines 112 - 125, Update the
release-notes header expression in the workflow block to use GitHub Actions’
positional format placeholder `{0}` instead of `%s`, so the provided
`github.event.inputs.release_id` is interpolated correctly while preserving the
existing conditional suffix behavior.
Source: Linters/SAST tools
| - name: Publish immutable Linux release | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| TAG_NAME="${GITHUB_REF_NAME}" | ||
| NOTES_PATH="${{ steps.release-notes.outputs.notes_path }}" | ||
| if gh release view "$TAG_NAME" --repo "${{ github.repository }}" >/dev/null 2>&1; then | ||
| echo "release exists for $TAG_NAME; refusing to overwrite for immutable release policy" | ||
| exit 1 | ||
| fi | ||
| gh release create "$TAG_NAME" \ | ||
| --repo "${{ github.repository }}" \ | ||
| --notes-file "$NOTES_PATH" \ | ||
| --title "Linux ${{ github.ref_name }}" \ | ||
| --target "$GITHUB_SHA" \ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- workflow triggers and publish step ---'
sed -n '1,220p' .github/workflows/release-linux.ymlRepository: TheOrcDev/videorc
Length of output: 5396
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- locate workflow_dispatch / release_id usage ---'
rg -n "workflow_dispatch|release_id|GITHUB_REF_NAME|Publish immutable Linux release|release exists" .github/workflows/release-linux.ymlRepository: TheOrcDev/videorc
Length of output: 673
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
import pathlib, re, sys
p = pathlib.Path('.github/workflows/release-linux.yml')
text = p.read_text()
# Print relevant lines with numbers for triggers and publish step.
for i, line in enumerate(text.splitlines(), 1):
if 1 <= i <= 220 and (re.search(r'workflow_dispatch|release_id|GITHUB_REF_NAME|Publish immutable Linux release|release exists|tags:', line) or 120 <= i <= 150):
print(f"{i:4d}: {line}")
PYRepository: TheOrcDev/videorc
Length of output: 1909
🌐 Web query:
GitHub Actions workflow_dispatch GITHUB_REF_NAME selected branch reference behavior documentation
💡 Result:
When a workflow is triggered via workflow_dispatch, the GITHUB_REF_NAME environment variable (and the equivalent github.ref_name context) is populated with the name of the branch or tag that was selected when the workflow was triggered [1][2]. Key behavioral points regarding workflow_dispatch and branch references: 1. Selection: When you manually trigger a workflow using the GitHub UI, you are prompted to select a branch or tag from which the workflow should run [3]. This selection determines the value of GITHUB_REF and GITHUB_REF_NAME [4]. 2. Value: GITHUB_REF_NAME contains the short name of the selected reference [1]. For a branch, it excludes the refs/heads/ prefix; for a tag, it excludes the refs/tags/ prefix [2]. 3. API and CLI: When triggering the workflow via the GitHub API or CLI, the reference is determined by the ref parameter [3]. If the parameter is omitted, the workflow defaults to the repository's default branch [3]. 4. Workflow Requirements: For the workflow_dispatch event to be available for selection, the workflow file must exist on the default branch of the repository [4][3]. Once that condition is met, you can trigger it against any branch or tag that exists in the repository [3]. In summary, GITHUB_REF_NAME is not empty when using workflow_dispatch; it accurately reflects the specific branch or tag reference you choose at the time of execution [1][2].
Citations:
- 1: https://docs.github.com/en/actions/reference/workflows-and-actions/variables
- 2: https://www.codegenes.net/blog/how-to-get-the-current-branch-within-github-actions/
- 3: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/manually-run-a-workflow?tool=webui
- 4: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows
Use an explicit release tag for manual dispatches workflow_dispatch can run against a branch, so GITHUB_REF_NAME here becomes a branch name like main. That would create branch-named releases and make the immutability check block later manual runs on the same branch. Derive the release tag from an explicit input or skip publishing unless the ref is a tag.
🧰 Tools
🪛 zizmor (1.26.1)
[info] 133-133: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 141-141: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-linux.yml around lines 128 - 142, Update the
“Publish immutable Linux release” step to avoid using GITHUB_REF_NAME as a
release tag for workflow_dispatch runs. Require an explicit release-tag input
for manual dispatches, or skip publishing when the ref is not a tag; preserve
the existing immutability check and gh release create flow for valid release
tags.
| - name: Create sync branch and apply upstream merge | ||
| run: | | ||
| SYNC_BRANCH="sync/upstream-$(date -u +%Y%m%d-%H%M%S)" | ||
| git checkout -b "$SYNC_BRANCH" | ||
| git merge --no-ff --no-edit upstream/main | ||
| if [ $? -ne 0 ]; then | ||
| echo "merge with upstream/main failed" | ||
| exit 1 | ||
| fi | ||
| git push -u origin "$SYNC_BRANCH" | ||
|
|
||
| - name: Open or update upstream sync PR | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| if git merge-base --is-ancestor HEAD origin/main; then | ||
| echo "upstream/main is already in HEAD; no PR needed" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Sync branch is pushed before checking whether a sync is actually needed.
The branch is created and pushed (lines 37-44) unconditionally. Only afterward (line 50) does the workflow check whether there's anything new to merge and bail out. On a daily cron with no upstream changes, this still leaves a new, unused sync/upstream-<timestamp> branch pushed to origin every run, accumulating stale branches indefinitely.
♻️ Proposed reordering
- name: Create sync branch and apply upstream merge
run: |
+ if git merge-base --is-ancestor upstream/main origin/main; then
+ echo "upstream/main already merged into origin/main; nothing to sync"
+ exit 0
+ fi
SYNC_BRANCH="sync/upstream-$(date -u +%Y%m%d-%H%M%S)"
git checkout -b "$SYNC_BRANCH"
git merge --no-ff --no-edit upstream/main
if [ $? -ne 0 ]; then
echo "merge with upstream/main failed"
exit 1
fi
git push -u origin "$SYNC_BRANCH"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Create sync branch and apply upstream merge | |
| run: | | |
| SYNC_BRANCH="sync/upstream-$(date -u +%Y%m%d-%H%M%S)" | |
| git checkout -b "$SYNC_BRANCH" | |
| git merge --no-ff --no-edit upstream/main | |
| if [ $? -ne 0 ]; then | |
| echo "merge with upstream/main failed" | |
| exit 1 | |
| fi | |
| git push -u origin "$SYNC_BRANCH" | |
| - name: Open or update upstream sync PR | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| run: | | |
| if git merge-base --is-ancestor HEAD origin/main; then | |
| echo "upstream/main is already in HEAD; no PR needed" | |
| exit 0 | |
| fi | |
| - name: Create sync branch and apply upstream merge | |
| run: | | |
| if git merge-base --is-ancestor upstream/main origin/main; then | |
| echo "upstream/main already merged into origin/main; nothing to sync" | |
| exit 0 | |
| fi | |
| SYNC_BRANCH="sync/upstream-$(date -u +%Y%m%d-%H%M%S)" | |
| git checkout -b "$SYNC_BRANCH" | |
| git merge --no-ff --no-edit upstream/main | |
| if [ $? -ne 0 ]; then | |
| echo "merge with upstream/main failed" | |
| exit 1 | |
| fi | |
| git push -u origin "$SYNC_BRANCH" | |
| - name: Open or update upstream sync PR | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| run: | | |
| if git merge-base --is-ancestor HEAD origin/main; then | |
| echo "upstream/main is already in HEAD; no PR needed" | |
| exit 0 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/upstream-sync.yml around lines 35 - 53, Move the no-op
detection in the “Open or update upstream sync PR” flow ahead of creating and
pushing the sync branch, so the workflow exits before the `git checkout -b`,
merge, and `git push` commands when `upstream/main` is already contained in
`HEAD`; retain the existing branch creation and merge behavior only when a sync
is needed.
|
|
||
| --- | ||
|
|
||
| ### Task 1: Rust contracts and fixtures scaffold |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the heading hierarchy.
### Task 1 follows the document’s H1 without an intervening H2, matching the reported MD001 warning. Change task headings to ## or add a parent ## Tasks section.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 18-18: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/compose/plans/2026-07-13-linux-slice-1-shared-settings-contract.md` at
line 18, Update the Task 1 heading in the document to use the correct hierarchy
by changing it from an H3 to an H2, or place it under a new parent H2 Tasks
section while preserving the existing task content.
Source: Linters/SAST tools
| For tagged Linux releases, `release-linux.yml` should: | ||
|
|
||
| 1. Run Linux gates (`fmt`, `lint`, `typecheck`, tests, smoke) | ||
| 2. Build the `.deb` | ||
| 3. Write and validate `build-info.json`, `linux-capabilities.json`, | ||
| `THIRD-PARTY-NOTICES.txt` | ||
| 4. Attach checksums and release metadata | ||
| 5. Create an immutable release entry only if absent |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Generate all required metadata before artifact validation.
The supplied .github/workflows/release-linux.yml runs scripts/validate-linux-artifacts.mjs before generating linux-capabilities.json and THIRD-PARTY-NOTICES.txt, while the validator requires those files. The release job will therefore fail before upload. Reorder the workflow or change the validator contract, then keep this runbook aligned.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/releases/release-runbook.md` around lines 112 - 119, Update the tagged
Linux release flow in release-linux.yml so build-info.json,
linux-capabilities.json, and THIRD-PARTY-NOTICES.txt are generated before
scripts/validate-linux-artifacts.mjs runs. Preserve validation of all required
metadata before upload, and keep the release-runbook sequence aligned with the
corrected workflow.
| const shaPath = `${outDir}.sha256` | ||
| try { | ||
| const binary = readFileSync(artifactPath) | ||
| const { createHash } = await import('node:crypto') | ||
| const checksum = createHash('sha256') | ||
| checksum.update(binary) | ||
| await writeFile(shaPath, `${checksum.digest('hex')} ${artifactPath}\\n`, 'utf8') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Checksum sidecar file ends with a literal \n instead of a newline.
${checksum.digest('hex')} ${artifactPath}\\n produces a trailing literal backslash + n, not an actual newline. This breaks the standard sha256sum file format and will fail verification with sha256sum -c or similar tooling that expects a real line terminator.
🐛 Proposed fix
- await writeFile(shaPath, `${checksum.digest('hex')} ${artifactPath}\\n`, 'utf8')
+ await writeFile(shaPath, `${checksum.digest('hex')} ${artifactPath}\n`, 'utf8')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const shaPath = `${outDir}.sha256` | |
| try { | |
| const binary = readFileSync(artifactPath) | |
| const { createHash } = await import('node:crypto') | |
| const checksum = createHash('sha256') | |
| checksum.update(binary) | |
| await writeFile(shaPath, `${checksum.digest('hex')} ${artifactPath}\\n`, 'utf8') | |
| const shaPath = `${outDir}.sha256` | |
| try { | |
| const binary = readFileSync(artifactPath) | |
| const { createHash } = await import('node:crypto') | |
| const checksum = createHash('sha256') | |
| checksum.update(binary) | |
| await writeFile(shaPath, `${checksum.digest('hex')} ${artifactPath}\n`, 'utf8') |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build-linux-capabilities.mjs` around lines 61 - 67, Update the
checksum content written by the checksum generation flow around `shaPath` and
`checksum.digest` so it appends an actual newline character rather than the
literal backslash-n sequence. Preserve the existing SHA-256 format and artifact
path.
| const fallback = [ | ||
| 'THIRD-PARTY-NOTICES.txt', | ||
| '', | ||
| 'No packaged FFmpeg notice files were discoverable at build time.', | ||
| 'The project still includes source and license files in vendor/ffmpeg/current for audit.', | ||
| `Generated at: ${new Date().toISOString()}` | ||
| ] | ||
|
|
||
| const content = [ | ||
| 'Videorc Linux third-party notices', | ||
| 'Includes bundled FFmpeg notices and configured build artifacts when available.', | ||
| '', | ||
| ...ffmpegNotices, | ||
| '', | ||
| ...fallback | ||
| ].join('\n') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fallback notice is always emitted even when FFmpeg notices are found.
...fallback is unconditionally spread into content at line 40, so the message "No packaged FFmpeg notice files were discoverable at build time." appears even when ffmpegNotices is non-empty. The generated file contradicts itself by listing discovered notices alongside a statement that none were found.
🐛 Proposed fix: only emit fallback when no notices were found
const fallback = [
'THIRD-PARTY-NOTICES.txt',
'',
'No packaged FFmpeg notice files were discoverable at build time.',
'The project still includes source and license files in vendor/ffmpeg/current for audit.',
`Generated at: ${new Date().toISOString()}`
]
const content = [
'Videorc Linux third-party notices',
'Includes bundled FFmpeg notices and configured build artifacts when available.',
'',
- ...ffmpegNotices,
- '',
- ...fallback
+ ...(ffmpegNotices.length > 0
+ ? [...ffmpegNotices, '', `Generated at: ${new Date().toISOString()}`]
+ : fallback)
].join('\n')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fallback = [ | |
| 'THIRD-PARTY-NOTICES.txt', | |
| '', | |
| 'No packaged FFmpeg notice files were discoverable at build time.', | |
| 'The project still includes source and license files in vendor/ffmpeg/current for audit.', | |
| `Generated at: ${new Date().toISOString()}` | |
| ] | |
| const content = [ | |
| 'Videorc Linux third-party notices', | |
| 'Includes bundled FFmpeg notices and configured build artifacts when available.', | |
| '', | |
| ...ffmpegNotices, | |
| '', | |
| ...fallback | |
| ].join('\n') | |
| const fallback = [ | |
| 'THIRD-PARTY-NOTICES.txt', | |
| '', | |
| 'No packaged FFmpeg notice files were discoverable at build time.', | |
| 'The project still includes source and license files in vendor/ffmpeg/current for audit.', | |
| `Generated at: ${new Date().toISOString()}` | |
| ] | |
| const content = [ | |
| 'Videorc Linux third-party notices', | |
| 'Includes bundled FFmpeg notices and configured build artifacts when available.', | |
| '', | |
| ...(ffmpegNotices.length > 0 | |
| ? [...ffmpegNotices, '', `Generated at: ${new Date().toISOString()}`] | |
| : fallback) | |
| ].join('\n') |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build-linux-notices.mjs` around lines 26 - 41, Update the content
assembly in the build notice generation flow so the fallback messages are
included only when ffmpegNotices is empty. Preserve the discovered FFmpeg
notices and existing header when notices are present, while retaining the
fallback output for the no-notices case.
Summary
Why this direction
User requested CI-first Linux packaging flow to avoid heavy local builds. This PR wires validation + release paths in GitHub Actions and documents the modular option matrix.
Summary by CodeRabbit
New Features
.deb, including bundled backend and FFmpeg resources.Bug Fixes
Documentation