fix: fall back to GitHub API for commit author when Firebase token la… by lbondaryk · Pull Request #2777 · concord-consortium/collaborative-learning

lbondaryk · 2026-02-18T22:06:09Z

…cks name/email (CLUE-427)

Extract resolveCommitAuthor helper that tries the Firebase decoded token first, then falls back to the GitHub API for missing name or email. This fixes the 'Could not determine author information from token' error for users whose GitHub profiles lack a public name or email. Add unit tests covering all fallback branches.

…cks name/email (CLUE-427) Extract resolveCommitAuthor helper that tries the Firebase decoded token first, then falls back to the GitHub API for missing name or email. This fixes the 'Could not determine author information from token' error for users whose GitHub profiles lack a public name or email. Add unit tests covering all fallback branches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR addresses missing commit author metadata when pushing units by introducing a helper that derives author name/email from the Firebase decoded token, falling back to the authenticated GitHub user profile when needed (CLUE-427).

Changes:

Add resolveCommitAuthor() helper to derive commit author from token, with GitHub API fallback.
Update pushUnit route to use the new helper and a clearer error message on failure.
Add Jest unit tests covering the fallback branches; add Jest-related dev dependencies.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

File	Description
authoring-api/src/routes/push-unit.ts	Switches commit author resolution to `resolveCommitAuthor()` with improved error messaging.
authoring-api/src/helpers/resolve-commit-author.ts	New helper implementing token-first, GitHub API fallback logic for commit author.
authoring-api/src/helpers/resolve-commit-author.test.ts	New Jest tests covering token-only, partial token, GitHub fallbacks, and error path.
authoring-api/package.json	Adds Jest/ts-jest and Jest types to support the new unit tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-18T22:09:51Z

authoring-api/package.json

    "@types/cors": "^2.8.19",
+    "@types/jest": "^30.0.0",
    "@typescript-eslint/eslint-plugin": "^8.39.1",
    "@typescript-eslint/parser": "^8.39.1",
    "dotenv": "16.4.5",
    "eslint": "^8.57.1",
    "eslint-config-google": "^0.14.0",
    "eslint-plugin-import": "^2.32.0",
    "firebase-tools": "^13.15.1",
+    "jest": "^30.2.0",
+    "ts-jest": "^29.4.6",
    "typescript": "^5.9.2"


jest/ts-jest are added but there is no test script in this package’s scripts. As a result, npm test (and any CI expecting it) won’t run the new unit tests; add a test (and optionally test:watch/test:coverage) script wired to jest.

Copilot · 2026-02-18T22:09:52Z

authoring-api/src/helpers/resolve-commit-author.test.ts

+import {resolveCommitAuthor} from "./resolve-commit-author";
+
+// Minimal mock matching the Octokit shape used by resolveCommitAuthor
+function mockOctokit(userData: {name?: string | null; email?: string | null; login: string}) {
+  return {


This test file lives under src/, and authoring-api/tsconfig.json includes src in the build, so it will be compiled into lib/ and shipped with the deployed function bundle. Consider moving tests under a dedicated test/ directory (as other packages in this repo do) and/or excluding **/*.test.ts from the production tsc build to avoid deploying test artifacts.

cypress · 2026-02-18T22:13:38Z

collaborative-learning Run #17889

Run Properties: Passed #17889 • 7d735b368d: chore: address PR review - fix test types, add CI integration, exclude from root...

Project	`collaborative-learning`
Branch Review	`CLUE-427-authors-cannot-commit-clue-changes`
Run status	`Passed #17889`
Run duration	`03m 04s`
Commit	`7d735b368d: chore: address PR review - fix test types, add CI integration, exclude from root...`
Committer	`lbondaryk`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`4`
View all changes introduced in this branch ↗︎

…LUE-427) Add npm test script wired to jest so tests run via npm test and CI. Exclude *.test.ts from tsconfig build to prevent test artifacts from being compiled into lib/ and deployed with the function bundle. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

codecov · 2026-02-18T23:14:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.45%. Comparing base (d56cbcd) to head (7d735b3).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2777      +/-   ##
==========================================
- Coverage   86.23%   85.45%   -0.78%     
==========================================
  Files         846      846              
  Lines       46028    46028              
  Branches    11919    11919              
==========================================
- Hits        39691    39332     -359     
- Misses       5939     6275     +336     
- Partials      398      421      +23

Flag	Coverage Δ
cypress	`?`
cypress-regression	`75.13% <ø> (-1.18%)`	⬇️
cypress-smoke	`43.42% <ø> (ø)`
jest	`50.33% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

emcelroy

Generally looking good, but some changes are needed.

I left a couple suggestions in resolve-commit-author.test.ts that should be applied.

Also, to get the new tests in /authoring-api to work correctly in CI (they're failing right now), you'll need to do the following...

in the root directory package.json file, add "/authoring-api/" to the testPathIgnorePatterns array.
in .github/workflows/ci.yml, after cms/package-lock.json on line 54, add authoring-api/package-lock.json. Then after this part a few lines down from there:

      - name: Install CMS Dependencies
        working-directory: ./cms
        run: npm ci

add this:

      - name: Install Authoring API Dependencies
        working-directory: ./authoring-api
        run: npm ci
      - name: Run Authoring API Tests
        working-directory: ./authoring-api
        run: npm test

emcelroy · 2026-02-19T04:02:07Z

authoring-api/src/helpers/resolve-commit-author.test.ts

+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  } as any;


Generally if you can avoid casting to any, it's best not to. It would be pretty simple and better to cast to Octokit like so:

Suggested change

// eslint-disable-next-line @typescript-eslint/no-explicit-any

} as any;

} as unknown as Octokit;

You'll also have to add import {Octokit} from "@octokit/rest"; at the top of the file for that to work.

Casting to any makes it so any type errors that might occur later in the test won't be caught.

emcelroy · 2026-02-19T04:03:08Z

authoring-api/src/helpers/resolve-commit-author.test.ts

+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  } as any;


Same suggestion as lines 11-12 above.

Suggested change

// eslint-disable-next-line @typescript-eslint/no-explicit-any

} as any;

// eslint-disable-next-line @typescript-eslint/no-explicit-any

} as unknown as Octokit;

emcelroy · 2026-02-19T04:10:33Z

authoring-api/package.json

+    "jest": "^30.2.0",
+    "ts-jest": "^29.4.6",


Just a note that I've only seen jest and ts-jest with matching version numbers in our projects (as in the package,json in the root directory where both are v28). I thought the versions always went together, but apparently ts-jest v29 will work with jest v30 according to https://github.com/kulshekhar/ts-jest/blob/main/package.json where it lists "@jest/types": "^29.0.0 || ^30.0.0" and "jest": "^29.0.0 || ^30.0.0" under peerDependencies. (And there is no v30 of ts-jest yet.)

…e from root jest (CLUE-427) Replace `as any` with `as unknown as Octokit` in test mocks for type safety. Add authoring-api install and test steps to CI workflow. Add /authoring-api/ to root testPathIgnorePatterns to prevent root jest from picking up authoring-api tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

lbondaryk · 2026-02-19T17:22:26Z

@emcelroy I think I've addressed your comments here? Can you approve?

emcelroy

Looks good 👍

lbondaryk requested a review from Copilot February 18, 2026 22:06

lbondaryk added the run regression label Feb 18, 2026

Merge branch 'master' into CLUE-427-authors-cannot-commit-clue-changes

acd4623

Copilot started reviewing on behalf of lbondaryk February 18, 2026 22:06 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

lbondaryk assigned emcelroy and unassigned emcelroy Feb 18, 2026

emcelroy requested changes Feb 19, 2026

View reviewed changes

github-actions bot temporarily deployed to development February 19, 2026 15:53 Inactive

emcelroy self-requested a review February 19, 2026 18:06

emcelroy approved these changes Feb 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: fall back to GitHub API for commit author when Firebase token la…#2777

fix: fall back to GitHub API for commit author when Firebase token la…#2777
lbondaryk wants to merge 4 commits intomasterfrom
CLUE-427-authors-cannot-commit-clue-changes

lbondaryk commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

cypress bot commented Feb 18, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 18, 2026 •

edited

Loading

Uh oh!

emcelroy left a comment

Uh oh!

emcelroy Feb 19, 2026

Uh oh!

emcelroy Feb 19, 2026

Uh oh!

emcelroy Feb 19, 2026 •

edited

Loading

Uh oh!

lbondaryk commented Feb 19, 2026

Uh oh!

emcelroy left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

		// eslint-disable-next-line @typescript-eslint/no-explicit-any
		} as any;

	// eslint-disable-next-line @typescript-eslint/no-explicit-any
	} as any;
	} as unknown as Octokit;

Conversation

lbondaryk commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

cypress bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

collaborative-learning Run #17889

Uh oh!

codecov bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

emcelroy left a comment

Choose a reason for hiding this comment

Uh oh!

emcelroy Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

emcelroy Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

emcelroy Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lbondaryk commented Feb 19, 2026

Uh oh!

emcelroy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

cypress bot commented Feb 18, 2026 •

edited

Loading

codecov bot commented Feb 18, 2026 •

edited

Loading

emcelroy Feb 19, 2026 •

edited

Loading