fix(regenerate): tolerate orphaned ACC baseline SHA in module detection - #1
Merged
Conversation
The scheduled "Regenerate learner branches" workflow crashed when the recorded course-build/.last-acc-sha pointed at an ACC commit that no longer exists (ACC history was rewritten). detect-affected-modules.mjs ran `git diff <bad-object>` and aborted with "fatal: bad object", failing the run. Add a commitExists() reachability check; when the supplied --from baseline is not a reachable commit in the ACC repo, fall back to first-run semantics (full regen) instead of crashing, and surface it via a WARN log and a baselineMissing output field. Add a temp-repo integration selftest covering both the valid-baseline and orphaned-baseline paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3750cefd-d322-4f02-9184-c8ac3eba2fff
There was a problem hiding this comment.
Pull request overview
Improves resilience of the course-build “detect affected modules” script used by the regenerate workflow by handling an invalid/orphaned baseline ACC SHA without crashing, and adds an integration selftest covering the fallback path.
Changes:
- Add
commitExists()andbaselineMissingoutput to treat an invalid--frombaseline as first-run/full-regen semantics. - Emit a warning when the baseline commit is missing/unresolvable instead of failing on
git diff. - Extend
selftest.mjswith a temp-repo integration test covering both valid and missing-baseline behavior.
Show a summary per file
| File | Description |
|---|---|
| course-build/scripts/detect-affected-modules.mjs | Adds baseline-existence guard and exposes baselineMissing to prevent git diff from crashing on bad baseline SHAs. |
| course-build/scripts/selftest.mjs | Adds an integration selftest that creates a temp git repo and verifies both normal diff and missing-baseline fallback behavior. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (1)
course-build/scripts/detect-affected-modules.mjs:88
baselineMissingis computed usingargs.from.trim(), butgit diffis run with the untrimmedargs.from/args.to. If either value contains whitespace/newlines (e.g. read from a file), the baseline check can pass whilegit diffstill fails with a bad revision. Consider normalizingfrom/toonce and using those values for both the existence check and the diff.
let files = [];
if (!firstRun) {
const out = git(args.acc, 'diff', '--name-only', `${args.from}`, `${args.to}`);
files = out.split('\n').map(s => s.trim()).filter(Boolean);
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
…ftest
Address review feedback:
- commitExists now uses `git rev-parse --verify --quiet <rev>^{commit}` and
only treats exit status 1 (well-formed but unknown/unreachable revision) as
"missing". Any other failure (bad --acc path, not a git repo, git missing)
is rethrown so real configuration/runtime errors fail fast instead of
silently masquerading as a missing baseline and triggering a full regen.
- selftest invokes the detector via process.execPath instead of "node" so it
works when node isn't on PATH or under a different runtime, and adds a case
asserting commitExists rethrows on a non-git directory.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3750cefd-d322-4f02-9184-c8ac3eba2fff
Address suppressed review note: baselineMissing was computed from a trimmed --from while `git diff` used the raw --from/--to, so a value with stray whitespace/newlines (e.g. read from a file) could pass the existence check yet still abort the diff on a bad revision. Trim both revisions once and use the normalized values for the existence check and the diff. Add a selftest asserting a whitespace-padded baseline is normalized. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3750cefd-d322-4f02-9184-c8ac3eba2fff
26 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The scheduled Regenerate learner branches (pull model) workflow has been failing on its daily cron. The
detectstep reads the last-processed ACC commit fromcourse-build/.last-acc-shaand diffs it against the current ACCmainHEAD. When the recorded baseline SHA no longer exists in the ACC repo (its history was rewritten / force-pushed, orphaning the commit),detect-affected-modules.mjsrangit diff <bad-object>and aborted withfatal: bad object ..., failing the run.This makes detection resilient: an unreachable baseline is now treated the same as "no baseline" (a full regen) instead of crashing, so the pipeline self-heals on the next run (the regen PR advances
.last-acc-shato a valid commit on merge).Related Issue
N/A
Type of Change
Affected Services
web(Astro SSR + React)assets-svc(.NET 10)workforce-svc(Java 21 / Spring Boot 3)reporting-svc(Python FastAPI)notifications-svc(Python FastAPI)audit-svc(legacy Java 11 / Spring Boot 2.7)auth-svc(legacy Java 11 / Spring Boot 2.7)Changes Made
commitExists(cwd, rev)reachability check (viagit cat-file -e <rev>^{commit}) todetect-affected-modules.mjs.--frombaseline is not a reachable commit in the ACC repo, fall back to first-run semantics (full regen) instead of runninggit diffon a bad object. This is surfaced via aWARNlog and a newbaselineMissingfield in the JSON output.Testing
npm run test:e2e(Playwright) passesdotnet testinservices/assets-svcpassespytestinservices/reporting-svcand/orservices/notifications-svcpassesmvn testinservices/workforce-svcpassesmvn testinservices/audit-svcand/orservices/auth-svcpassesnpm run devordocker compose up --build) and the affected flow works at http://localhost:4321Ran
node course-build/scripts/selftest.mjs(the same suite invoked byvalidate-branches.yml); all checks pass, including the new baseline-fallback cases.Checklist
main(I have not hand-edited generated learner branches or promoted refs — see CONTRIBUTING.md)exercises.md) if neededAdditional Notes
This resilience fix lands the code path, but the
.last-acc-shacurrently committed onmainstill points at the orphaned SHA. With this change, the next scheduled run no longer errors: it performs a full regen and opens a PR that advances.last-acc-shato a valid commit. No manual baseline edit is required, though committing a valid SHA would also work as an immediate unblock.