Phase 1: fix CI Windows test, repair CodeQL config, clean stale legal wording - #6
Conversation
… wording - Fix WavePcm16 Windows move-failure test: Assert.ThrowsAsync<Exception> required an exact type match against a subtype (UnauthorizedAccessException); swap to Assert.ThrowsAnyAsync<Exception> so the existing IOException/UnauthorizedAccessException contract check is actually reached. - Remove the stale `frontend` path from .github/codeql/codeql-config.yml; no frontend directory exists in this repo. - Delete docs/legal/CONTRIBUTOR-LICENSE-AGREEMENT.md (GPLv3 + dual-license wording contradicts the repo's actual Apache-2.0 LICENSE/NOTICE). - Add CONTRIBUTING.md (Apache-2.0 inbound contribution terms) and docs/legal/LICENSE-HISTORY.md (public core vs. proprietary desktop split). - Trim docs/legal/THIRD_PARTY_NOTICES.md to drop entries scoped to Trackdub.App.Avalonia and the Inter font, which aren't part of this repo; keep the LibVLCSharp entry used by Trackdub.Media.Playback. - Correct IExportTierGate's doc comment: the public core ships no concrete implementation (verified: none exists in this repo); a consuming product supplies its own tier policy. - Add opt-in package metadata (Authors/Description/License/RepositoryUrl/ PackageProjectUrl) to Trackdub.Cli and Trackdub.OnnxRuntime.Dnnl.Native. - Add scripts/ci/check-repository-boundary.py and a CI job that fails the build on stale GPL/dual-license wording or desktop-project-name bleed in the legal/policy/contract surface.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09c0900bcf
ℹ️ 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".
| if not matches_any(f.relative_to(REPO_ROOT).as_posix(), LICENSE_CLAIM_EXCLUDE_DIRS) | ||
| and f.relative_to(REPO_ROOT).as_posix() not in LICENSE_CLAIM_EXCLUDE_FILES |
There was a problem hiding this comment.
When a file under docs/architecture, docs/specs, docs/operations, or docs/development legitimately mentions GPLv3 or dual licensing, this comprehension still includes it and the new workflow fails, despite the module documentation explicitly declaring those trees out of scope because they contain historical/internal material. Add the documented directories to the exclusions or restrict this scan to the authoritative legal/policy surface.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 11 files
You're on the cubic free plan with 17 free PR reviews remaining this month. Upgrade for unlimited reviews.
Re-trigger cubic
- docs/legal/LICENSE-HISTORY.md: ../LICENSE and ../NOTICE resolved to docs/LICENSE and docs/NOTICE (404); fix to ../../LICENSE and ../../NOTICE. - scripts/ci/check-repository-boundary.py: LICENSE_CLAIM_EXCLUDE_DIRS was missing docs/architecture/, docs/specs/, docs/operations/, docs/development/, and tools/, contradicting the module docstring's stated scope and risking a false-positive CI failure on legitimate historical mentions in those trees. Addresses review feedback from Codex and cubic on PR #6.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b9edd7911
ℹ️ 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".
- Add OS-specific labels ([self-hosted, Windows/Linux/macOS]) to every job instead of the bare `self-hosted` label. With only one registered self-hosted runner (Windows), the bare label was routing the Linux build job, the format job, and the repository-boundary job onto that same Windows box, silently skipping Linux/macOS coverage entirely (flagged by Codex review) and hitting a separate PowerShell script-execution-policy failure on the boundary scanner's run step. - Restore the macOS matrix leg dropped in the direct self-hosted edit. - Set DOTNET_INSTALL_DIR to the per-job runner temp directory so actions/setup-dotnet doesn't try to write to the system-wide default location (e.g. C:\Program Files\dotnet), which self-hosted runner service accounts often can't write to. - Add explicit `shell: bash` to every run: step for consistent behavior across self-hosted runner OSes (Windows defaults run: steps to PowerShell, which blocked the boundary scanner step above).
runs-on: \${{ matrix.runner }} where matrix.runner was itself a YAML array
made the whole workflow file fail to parse (0 jobs scheduled on the
previous run). Use a scalar matrix.os string combined with a literal
self-hosted label instead: runs-on: [self-hosted, "\${{ matrix.os }}"].
The follow-up fix (runs-on: [self-hosted, "\${{ matrix.os }}"]) still left
the workflow file unparseable on GitHub's side (0 jobs scheduled, run name
fell back to the file path). Embedding an expression inside a runs-on
array literal isn't accepted. Replace the single matrix-driven build job
with three explicit build-windows/build-linux/build-macos jobs, each with
a static runs-on: [self-hosted, <OS>] array -- the one form of self-hosted
label targeting GitHub documents and that already parsed successfully for
the format and repository-boundary jobs.
Root cause of every "invalid workflow file" run since the previous three
commits: the runner context (\${{ runner.temp }}) isn't available in a
top-level workflow env: block, only inside a job. That single bad
top-level env entry silently invalidated the whole file (0 jobs scheduled,
GitHub falling back to the file path as the run name) regardless of what
else changed in runs-on/matrix syntax. Move DOTNET_INSTALL_DIR into each
job's own env: block instead.
…lure Every run since the runner.temp-based env block was added has come back as an unparseable workflow file (0 jobs scheduled, run name falls back to the file path), even after moving it from workflow-level to per-job env. Removing it entirely to confirm whether that expression is actually the cause before trying another workaround.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b01805106
ℹ️ 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".
Reverting to the last known-good workflow content (5b9edd7, which actually scheduled real jobs) and changing only one thing: format/ repository-boundary runs-on: self-hosted -> runs-on: [self-hosted, Linux]. Everything else (matrix-driven build job, no shell: overrides) restored verbatim, to find out whether this array-literal form is itself what breaks parsing before reapplying the rest of the fix.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a289325e02
ℹ️ 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".
…aden GPL scanner pattern
Root-caused the earlier "invalid workflow file" runs (0 jobs scheduled,
Codex/list_workflow_jobs both confirmed): they were not a YAML/schema
problem. A workflow_dispatch run on the exact runs-on: [self-hosted, Linux]
content that had been failing scheduled 4 real jobs immediately, proving
the syntax is valid. The push-triggered failures line up with rapid-fire
commits sharing the same concurrency group (cancel-in-progress: true)
during the debugging session, not the file content.
With that confirmed, reapply the rest of the self-hosted fix on top of the
validated baseline:
- OS-specific runs-on: [self-hosted, Windows/Linux/macOS] on every job
instead of the bare self-hosted label, so jobs land on the matching OS
runner instead of all piling onto whichever one runner happens to be
registered.
- Restore the macOS build leg.
- shell: bash on every run: step (Windows self-hosted defaults run: steps
to PowerShell, which blocked a script-execution-policy-restricted step
during debugging).
- Per-job env: DOTNET_INSTALL_DIR: ${{ runner.temp }}/.dotnet, to work
around actions/setup-dotnet defaulting to a system-wide install location
self-hosted runner accounts often can't write to.
Also broaden scripts/ci/check-repository-boundary.py's GPL pattern per
Codex review feedback: the previous exact-string "GPLv3" match missed
"GPL-3.0-only", "GPL 3.0", and "GNU General Public License" while still
correctly excluding LGPL/AGPL (checked via a negative lookbehind for a
preceding letter).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6618235f54
ℹ️ 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".
The General Public License regex added in the last commit had no letter-preceding exclusion (unlike the GPL token pattern next to it), so it would false-positive on "GNU Lesser General Public License" and "GNU Affero General Public License" -- both distinct, legitimately-used licenses, not the copyleft GPL wording this scanner exists to catch. Addresses Codex review feedback on the prior commit.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f96e22088
ℹ️ 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".
- LICENSE: copyright line said "Copyright 2026 tonythethompson" (a personal name); changed to "Copyright 2024-2026 Trackdub LLC" to match root NOTICE. (The license body wording itself was already verified against the canonical apache.org/licenses/LICENSE-2.0.txt text and matches exactly -- no change needed there.) - scripts/ci/check-repository-boundary.py: the dual-license pattern only matched a capitalized-first-letter variant ([Dd]ual...); switched to re.IGNORECASE so all-caps forms like DUAL-LICENSED aren't missed, matching the style already used for the GPL patterns. Addresses further Codex review feedback and an owner-confirmed fix on PR #6.
…tep scope
GitHub's own workflow validation (surfaced via a workflow_dispatch API
call, which returns actual parse errors unlike a push/pull_request-
triggered run) confirms it precisely: "Unrecognized named-value: 'runner'"
at every DOTNET_INSTALL_DIR: ${{ runner.temp }} line. The runner context
is only available inside a step (its own env/run/with), not at the job- or
workflow-level env: block -- job-level was already tried and also invalid,
contrary to what an earlier commit message concluded from indirect
evidence. Move DOTNET_INSTALL_DIR onto the "Setup .NET" step's own env:
in every job.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2328a67a63
ℹ️ 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".
Same class of bypass as the GPL/dual-license patterns: a differently cased identifier like "trackdub.api" or "Trackdub.worker" (common in URLs and prose) would slip past the case-sensitive boundary regexes. Add re.IGNORECASE to all five. Addresses further Codex review feedback on PR #6.
2 participants
Summary
Executes the authorized Phase 1 (public-core release-readiness) task from
docs/plans/open-core-split-handoff.md/docs/plans/open-core-split-continuation.md. Scope: fix the failing Windows CI test, repair the CodeQL config, audit Dependabot governance, and clean up stale GPL/desktop legal wording — nothing else. No changes toTrackdub-gated,api.trackdub,portal.trackdub, ortrackdub.com;Trackdubstays private.Scope grew slightly beyond the original plan mid-PR: the repo owner switched CI to self-hosted runners directly on both this branch and
mainwhile this PR was open, which required fixing runner OS-targeting and a real GitHub Actions parse bug (see below), and a concurrent direct edit toLICENSEonmainneeded a small follow-up fix. Both are called out explicitly below.Linked issue
docs/plans/open-core-split-continuation.md(branchdocs/open-core-split-continuation).Scope
Testing
dotnet build Trackdub.slnx -m:1 -p:Platform=x64dotnet test Trackdub.slnx -m:1 -p:Platform=x64Test notes
tests/Trackdub.Media.Tests/WavePcm16Tests.cs:WriteSamplesAsync_move_failure_deletes_temp_file_and_preserves_destinationwas failing on Windows becauseAssert.ThrowsAsync<Exception>()requires an exact type match, butFile.Moveonto a locked destination throwsUnauthorizedAccessException(a subtype). The test already had a dead follow-up assertion (ex is IOException or UnauthorizedAccessException) encoding the intended contract — it just never ran because the outer assertion threw first. Fix:Assert.ThrowsAsync<Exception>→Assert.ThrowsAnyAsync<Exception>. No production code changed;WavePcm16.WriteSamplesAsync's rethrow-after-cleanup behavior was already correct.Architecture review
Trackdub.AppLicense/model impact
LICENSEreviewed against the canonicalapache.org/licenses/LICENSE-2.0.txttext — see LICENSE note below.Risk and rollback
Follow-up required (new finding, out of this PR's scope): the repository-boundary scanner added here (
scripts/ci/check-repository-boundary.py) is intentionally scoped to the legal/policy/contract surface (docs/legal/,LICENSE,NOTICE,README.md,AGENTS.md,CONTRIBUTING.md,docs/repository-policy.md,src/Trackdub.Contracts/). Scanning the full tree surfaced dozens of pre-existing references to monorepo-era projects that no longer exist in this repo'ssrc/(Trackdub.App.Avalonia,Trackdub.Api,Trackdub.Worker,Trackdub.WebhookDelivery) acrossdocs/architecture/,docs/specs/,docs/operations/,docs/development/,tools/ci/verify-dependency-graph.py,tools/validation/validate-repo.ts, andtools/localization/*. None of these are wired into CI today, so nothing is currently broken, but they're stale monorepo-era documentation/tooling debt beyond the four "Known Remaining Core Defects" the handoff doc enumerated. Recommend a dedicated follow-up task.Follow-up required (deferred by owner): self-hosted runners remain wired to the
pull_requesttrigger, which means PR-controlled code (viadotnet restore/build/test/format, arbitrary MSBuild targets) executes on a persistent, non-ephemeral machine — a real runner-compromise vector once this repo accepts outside contributions post-public-release. Raised with the repo owner directly; they chose to accept this risk for now rather than scope self-hosted jobs to trusted branches only. Revisit before the repo goes public and starts accepting untrusted PRs.Milestone notes
Phase 1 of
docs/plans/open-core-split-continuation.md.Agent notes
Required Phase 1 Report (per
docs/plans/open-core-split-handoff.md)main, headac68c91onclaude/open-core-split-continuation-oixwkl(includes a merge ofmain's two direct-push commits,0fb39a6andc0dc052, via the PR's "Update branch").mainwhile this PR was in flight (bareself-hostedlabel, no OS distinction, macOS leg dropped). That surfaced three real problems, all now fixed on this branch:self-hostedlabel doesn't distinguish OS, so with only one registered runner (Windows), the Linux build job and other jobs were silently landing on the wrong OS. Fixed with OS-specificruns-on: [self-hosted, Windows/Linux/macOS]per job, and the macOS leg restored.actions/setup-dotnet's install script fails on self-hosted runners withdotnet-install: The current user doesn't have write access to the installation root 'C:\Program Files\dotnet'. My first fix attempt setDOTNET_INSTALL_DIR: ${{ runner.temp }}/.dotnetat workflow- then job-levelenv:, which silently made the entire workflow file invalid (0 jobs scheduled on everypull_request/push-triggered run, with GitHub falling back to showing the file path instead of the workflow name — and confusingly, aworkflow_dispatchrun on the same content scheduled real jobs, becausepull_requestsynchronize events had separately stopped firing after an earlier rapid run of pushes, most likely GitHub-side debouncing under oneconcurrency.group, which masked the real parse error for a long stretch of debugging). Root cause was only confirmed once aworkflow_dispatchAPI call returned GitHub's actual validation error:Unrecognized named-value: 'runner'— therunnercontext is only available inside a step, not at job- or workflow-levelenv:. Fixed by movingDOTNET_INSTALL_DIRonto theSetup .NETstep's ownenv:in every job.run:steps to PowerShell (which blocked one step under a restrictive execution policy during debugging); addedshell: bashto everyrun:step for consistency.pull_request-triggered CI (run 30203275016) correctly schedules all 5 jobs (Verify Code Format,Verify Repository Boundary,Build & Test (Windows/Linux/macOS)) with the right OS labels — the workflow file is valid and jobs are correctly targeted. All 5 are currentlyqueuedwithrunner_id: 0, meaning no self-hosted runner is currently online/available to pick them up. That's a runner-provisioning/availability gap on the owner's infrastructure, not a code or workflow defect — I did not keep debugging past confirming the jobs are correctly scheduled and waiting.frontendentry from.github/codeql/codeql-config.yml(nofrontenddirectory exists in this repo). Thecodeql.ymlworkflow is currently manually disabled (owner action, consistent with GHAS not being enabled — SARIF upload was failing regardless of config correctness).dependabot-auto-merge.ymlalready correctly gates auto-merge tosemver-patch/semver-minorand flagssemver-majorfor manual review without merging (fixed in commit36e6710, pre-dating this PR). The two major bumps already onmain(actions/checkout4→7,actions/setup-dotnet4→6) merged before that gating commit — a genesis timing artifact, not a live policy gap.docs/legal/CONTRIBUTOR-LICENSE-AGREEMENT.md(GPLv3 + dual-license wording contradicted the repo's actual Apache-2.0LICENSE/NOTICE).CONTRIBUTING.md(Apache-2.0 inbound contribution terms) anddocs/legal/LICENSE-HISTORY.md(public core vs. proprietary desktop split).docs/legal/THIRD_PARTY_NOTICES.md: dropped entries scoped toTrackdub.App.Avalonia(not present in this repo); keptLibVLCSharpand the system-LibVLC-on-Linux entries (verified used byTrackdub.Media.Playback).src/Trackdub.Contracts/IExportTierGate.cs's doc comment (removed the false "Implemented in Application layer" claim; verified zero implementations exist in this repo).LICENSEnote: a concurrent direct push tomain(c0dc052, "Update LICENSE") reworded several sentences and changed the copyright line to a personal name. I diffed the resulting text against the authoritativeapache.org/licenses/LICENSE-2.0.txtand it matches verbatim — my initial read of that diff was wrong; the edit actually corrected pre-existing deviations from canonical text, it didn't introduce them. The one real inconsistency (copyright line saidtonythethompsonvs.NOTICE'sTrackdub LLC) was raised with the owner directly, who confirmed aligning it — nowCopyright 2024-2026 Trackdub LLCin both files.Trackdub.CligainedAuthors/Description/PackageLicenseExpression=Apache-2.0/RepositoryUrl/PackageProjectUrl;Trackdub.OnnxRuntime.Dnnl.NativegainedRepositoryUrl/PackageProjectUrl(kept its correctPackageLicenseExpression=MIT).scripts/ci/check-repository-boundary.py+ arepository-boundaryCI job, both passing locally against the current tree. Hardened through several rounds of automated review (Codex/cubic): broadenedGPLv3→ full GPL/SPDX/prose spelling coverage with LGPL/AGPL exclusions, case-insensitive dual-license and desktop-project-name matching, and an exclude-list fix so the scanner's own docstring matches its behavior.