Fix C-quoted filename handling in push_signed_commits.cjs#26277
Fix C-quoted filename handling in push_signed_commits.cjs#26277
Conversation
…mmits.cjs Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b0ede66f-9a71-4656-b886-afd3b0fa8cd1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| * @param {string} s - Raw path token from git output (may or may not be quoted) | ||
| * @returns {string} Unescaped path | ||
| */ | ||
| function unquoteCPath(s) { |
There was a problem hiding this comment.
Added a dedicated unquoteCPath test suite in commit f01052f. The 9 tests cover: plain strings returned unchanged, quote stripping, all standard C escape sequences (\\, \", \a/\b/\f/\n/\r/\t/\v), octal sequences decoding to UTF-8 bytes (e.g. \303\251 → é), filenames with spaces, unknown escapes (backslash preserved), edge cases (single char, empty string, 1/2/3-digit octals). The function is also now exported from the module to make it directly importable in tests.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/16fb5edb-1a9b-45e4-bac1-81546d0acd3f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes handling of C-quoted filenames emitted by git diff-tree --raw so push_signed_commits.cjs correctly reads file contents and emits proper GraphQL fileChanges paths for files with special characters (notably UTF-8/unicode names).
Changes:
- Added
unquoteCPath(s)to decode git C-quoted path tokens (C escapes + octal-encoded UTF-8 bytes). - Applied
unquoteCPathto raw diff-tree path extraction for normal, rename, and copy entries. - Added unit tests for
unquoteCPathplus new integration coverage for unicode/spaces, rename, and copy scenarios.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/push_signed_commits.cjs | Adds and uses unquoteCPath when parsing git diff-tree --raw paths; exports helper for testing. |
| actions/setup/js/push_signed_commits.test.cjs | Adds unit tests for unquoteCPath and new integration tests for special-character paths and rename/copy behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
actions/setup/js/push_signed_commits.cjs:190
- Same issue as rename handling:
paths[1]may be missing for an unexpected/partial copy entry, andunquoteCPath(paths[1])will throw before the subsequentif (!copiedPath)guard can run. Add an existence check before unquoting (or adjustunquoteCPathto handle non-strings) to preserve the intended skip-with-warning behavior.
} else if (status && status.startsWith("C")) {
// Copy: source path is kept (no deletion), only the destination path is added
const copiedPath = unquoteCPath(paths[1]);
if (!copiedPath) {
core.warning(`pushSignedCommits: copy entry missing destination path, skipping: ${line}`);
continue;
- Files reviewed: 2/2 changed files
- Comments generated: 2
| } else if (status && status.startsWith("R")) { | ||
| // Rename: source path is deleted, destination path is added | ||
| const renamedPath = paths[1]; | ||
| const renamedPath = unquoteCPath(paths[1]); | ||
| if (!renamedPath) { | ||
| core.warning(`pushSignedCommits: rename entry missing destination path, skipping: ${line}`); |
There was a problem hiding this comment.
paths[1] can be undefined for malformed/edge-case git diff-tree --raw output (the code already anticipates this via the if (!renamedPath) guard). Calling unquoteCPath(paths[1]) before checking existence will throw a TypeError and skip the warning/continue behavior. Guard paths[1] before passing it to unquoteCPath (or make unquoteCPath tolerate non-string inputs) so missing destination paths are handled gracefully.
This issue also appears on line 185 of the same file.
| } else { | ||
| const esc = inner[i++]; | ||
| switch (esc) { |
There was a problem hiding this comment.
If the quoted token is malformed (e.g. starts with " but is missing a closing quote) and the inner string ends with a single trailing backslash, const esc = inner[i++] evaluates to undefined and later esc.charCodeAt(0) will throw. Add an i >= inner.length check after consuming \\ to treat a trailing backslash as a literal instead of crashing.
🧪 Test Quality Sentinel ReportTest Quality Score: 83/100✅ Excellent test quality
Test Classification Details📋 All 13 test classifications
Flagged Tests — Requires ReviewNo tests flagged. All 13 tests verify observable behavioral contracts. Score Breakdown
Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.
|
There was a problem hiding this comment.
✅ Test Quality Sentinel: 83/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 13 new tests verify behavioral contracts with strong edge-case coverage (77%), including boundary values, C escape sequences, multi-byte UTF-8 octal decoding, unknown-escape fallback behavior, and end-to-end rename/copy contracts via real git subprocess execution.
git diff-tree --rawC-quotes filenames containing spaces, non-ASCII characters, or control characters (e.g."h\303\251llo.txt"). The code was passing these raw quoted strings directly tofs.readFileSyncand the GraphQL payload, causing failures for any commit touching such files.Changes
push_signed_commits.cjs: AddedunquoteCPath(s)helper that strips surrounding double-quotes and decodes C escape sequences (\\,\",\a–\v) and octal sequences (\NNN) as UTF-8 bytes — matching git's encoding of multi-byte Unicode filenames. Applied to all three path extraction sites:filePath,renamedPath, andcopiedPath. The helper is exported for direct testability.push_signed_commits.test.cjs: Added integration tests covering:héllo_wörld.txt)deletions, new path inadditionsdeletions, destination inadditionsAdded a dedicated unit test suite for
unquoteCPathcovering: plain strings (returned unchanged), quote stripping, all standard C escape sequences, octal sequences decoded as UTF-8 bytes, unknown escapes (backslash preserved), and edge cases (empty string, single character, 1/2/3-digit octals).