feat : auto-generate manifest.json by CI - #54
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a manifest generation system: new GitHub Actions workflow, CLI subcommand, ManifestGenerator service, manifest data models, and tests to produce and update Changes
Sequence DiagramsequenceDiagram
participant Workflow as GitHub Actions
participant CLI as Program CLI
participant Gen as ManifestGenerator
participant FS as File System
participant LangMap as SupportedLanguages
Workflow->>CLI: run `generate-manifest` (manual or on push)
CLI->>Gen: instantiate via DI
CLI->>Gen: GenerateManifest(translationPath, manifestPath)
Gen->>FS: Enumerate translation files (translations/*.json)
FS-->>Gen: file list
loop per file
Gen->>LangMap: GetLanguageInfoByName(filenameWithoutExt)
LangMap-->>Gen: (code, LanguageInfo) or null
Gen->>FS: Read file content
FS-->>Gen: JSON content
Gen->>Gen: compute SHA-256
Gen->>Gen: parse `_maintainer` field (if any)
Gen->>Gen: build ManifestEntry (File = translations/<name>.json)
end
Gen->>FS: Read existing manifest.json (if exists)
FS-->>Gen: existing manifest
alt existing entry SHA matches
Gen->>Gen: preserve existing Updated
else
Gen->>Gen: set Updated = run UTC timestamp
end
Gen->>FS: Write manifest.json (indented, relaxed escaping)
FS-->>Gen: write result
Gen-->>CLI: return success/failure
CLI->>Workflow: exit code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Translator/Services/ManifestGenerator.cs (1)
132-147: Make manifest entry ordering deterministic.Current iteration order depends on filesystem enumeration. Sorting before building entries avoids nondeterministic
manifest.jsonchurn.♻️ Proposed refactor
- var files = GetTranslationFiles(translationDirectoryPath)?.ToArray(); + var files = GetTranslationFiles(translationDirectoryPath)? + .OrderBy(Path.GetFileName, StringComparer.OrdinalIgnoreCase) + .ToArray();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Translator/Services/ManifestGenerator.cs` around lines 132 - 147, The manifest generation is nondeterministic because filesystem enumeration order is used; before iterating call GetTranslationFiles(translationDirectoryPath) and sort the resulting file list (e.g., OrderBy(Path.GetFileName, StringComparer.OrdinalIgnoreCase) or another stable key) so the loop that calls BuildEntry(file, existingEntry) always processes files in a deterministic order; update the loop that populates entries (and any variable like files) to use the sorted sequence so manifest.json churn is eliminated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Translator.Tests/CliTestHost.cs`:
- Line 65: The timeout Task.Delay is using cts.Token which may already be
canceled so the delay completes immediately; change the call in the awaiting
expression that combines stdOutTask and stdErrTask (the Task.WhenAny awaiting
Task.WhenAll(stdOutTask, stdErrTask), Task.Delay(...)) to use a non-cancelable
token (e.g., CancellationToken.None) or a fresh CancellationToken that isn’t
already cancelled so the 2000ms grace period actually waits for stream drain
instead of being bypassed.
---
Nitpick comments:
In `@Translator/Services/ManifestGenerator.cs`:
- Around line 132-147: The manifest generation is nondeterministic because
filesystem enumeration order is used; before iterating call
GetTranslationFiles(translationDirectoryPath) and sort the resulting file list
(e.g., OrderBy(Path.GetFileName, StringComparer.OrdinalIgnoreCase) or another
stable key) so the loop that calls BuildEntry(file, existingEntry) always
processes files in a deterministic order; update the loop that populates entries
(and any variable like files) to use the sorted sequence so manifest.json churn
is eliminated.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6d6b33e-ae00-4145-a776-81d566a76fe2
📒 Files selected for processing (8)
.github/workflows/manifest.ymlTranslator.Tests/CliTestHost.csTranslator.Tests/Services/ManifestGeneratorTests.csTranslator/Models/LanguageInfo.csTranslator/Models/ManifestEntry.csTranslator/Program.csTranslator/Services/ManifestGenerator.csTranslator/translations/french.json
Per @j3nnystar_bot review (btcpayserver#54), @teamssUTXO triaged 8 findings; this commit ships btcpayserver#1 + btcpayserver#2 + btcpayserver#3 + btcpayserver#5 + btcpayserver#6 + btcpayserver#7 + btcpayserver#8. btcpayserver#1 Per-file timestamp drift inside one run. ManifestGenerator now captures `runUpdatedAt` once at the top of GenerateManifest and threads it into BuildEntry. All files changed in a single run share one Updated stamp instead of drifting per file as the run progresses. btcpayserver#2 File-name to language coupling - solution B (explicit mapping). Models/LanguageInfo.cs adds an explicit `FileNameToCode` dictionary mapping translation-file basenames (`french`, `german`, etc.) to Languages keys. New `GetLanguageInfoByFileName` replaces the old `GetLanguageInfoByName` Name-equality lookup. The contract is now visible: a new translation file requires a row here, which is intentional - implicit Name lookup could silently mis-route or drop entries (e.g. `portuguese.json` -> "Portuguese (Brazil)" by parenthetical accident). btcpayserver#3 Fail-fast on per-file failure: kept, plus manual recovery trigger. .github/workflows/manifest.yml gains `workflow_dispatch:` so a broken translation file can be fixed and the manifest re-generated from the Actions tab without waiting for another translations push. ManifestGenerator's foreach has a comment explicitly naming the fail-fast posture and pointing at the dispatch trigger as the recovery path. btcpayserver#5 GetMaintainer async hygiene. Swapped sync `File.ReadAllText` for `await File.ReadAllTextAsync`, made the helper return `Task<string?>`, awaited at the call site. btcpayserver#6 Whitespace nit in HashFiles. Single space between `await` and `sha256.ComputeHashAsync`. btcpayserver#7 Default paths anchored to project directory (per @teamssUTXO go-ahead). New `ResolveProjectDirectory()` helper walks up from `AppContext.BaseDirectory` looking for `BTCPayTranslator.csproj`. `--translation-path` defaults to `<project-dir>/translations`, `--manifest-path` defaults to `<repo-root>/manifest.json`. The command produces the same manifest whether invoked from the repo root, from inside Translator/, or from the workflow with its explicit `working-directory: Translator`. btcpayserver#8 Rename `CreateManifest` -> `CreateGenerateManifestCommand`. Matches the symmetry of `CreateUpdateCommand`, `CreateValidatePacksCommand`, etc. btcpayserver#4 (existing translation files lacking `_maintainer`) was triaged out of scope for this PR per @teamssUTXO. Local verification: dotnet build clean (0/0 errors, 0 warnings), dotnet test 40/40 passing on .NET 10 RC.2 (`10.0.100-rc.2.25502.107`). Co-Authored-By: Timothé <183613235+teamssUTXO@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Translator/Services/ManifestGenerator.cs (1)
31-35: Avoid wrapping with genericException; preserve original exception context.On Line 31, Line 47, Line 69, and Line 117, catching and rethrowing
new Exception(...)adds noise and loses precise exception typing for callers/log pipelines. Preferthrow;after logging or let exceptions bubble.Also applies to: 47-51, 69-73, 117-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Translator/Services/ManifestGenerator.cs` around lines 31 - 35, The catch blocks in ManifestGenerator.cs that currently do "_logger.LogError(ex, ...)" followed by "throw new Exception(..., ex)" (seen in the catch(Exception ex) handlers around the translation file handling and the other three locations) are wrapping and losing the original exception type; replace each "throw new Exception(..., ex)" with a plain "throw;" (or remove the catch entirely if logging is redundant) so the original exception and stack trace are preserved while keeping the existing _logger.LogError(...) calls (search for the catch blocks that reference "_logger.LogError(ex, \"Couldn't find translation files in {Directory}\", translationDirectoryPath)" and the other similar catch handlers and update them to rethrow with "throw;").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/manifest.yml:
- Line 7: The workflow path filter currently uses the non-recursive glob
'Translator/translations/**.json' which can miss nested JSONs; update the path
pattern to the explicit recursive glob 'Translator/translations/**/*.json' so
files in subdirectories are matched reliably (replace the existing
'Translator/translations/**.json' entry in the manifest with
'Translator/translations/**/*.json').
- Around line 41-47: Replace the mutable tag "uses: EndBug/add-and-commit@v10"
with an immutable full-length commit SHA for the EndBug/add-and-commit action;
locate the uses line (uses: EndBug/add-and-commit@v10) and update it to the
corresponding full commit SHA (e.g., uses:
EndBug/add-and-commit@<full-commit-sha>) so the workflow is pinned to an exact
commit per GitHub security guidelines, and verify the SHA matches the intended
release before committing.
In `@Translator/Services/ManifestGenerator.cs`:
- Around line 141-161: The list of translation files returned by
GetTranslationFiles can be non-deterministic (Directory.GetFiles ordering), so
sort the files before building entries to ensure deterministic manifest output;
after obtaining files (the files variable from GetTranslationFiles) sort them
(e.g., by full path or filename) before the foreach that calls BuildEntry(file,
existingEntry, runUpdatedAt) so ManifestEntry order is stable across runs.
---
Nitpick comments:
In `@Translator/Services/ManifestGenerator.cs`:
- Around line 31-35: The catch blocks in ManifestGenerator.cs that currently do
"_logger.LogError(ex, ...)" followed by "throw new Exception(..., ex)" (seen in
the catch(Exception ex) handlers around the translation file handling and the
other three locations) are wrapping and losing the original exception type;
replace each "throw new Exception(..., ex)" with a plain "throw;" (or remove the
catch entirely if logging is redundant) so the original exception and stack
trace are preserved while keeping the existing _logger.LogError(...) calls
(search for the catch blocks that reference "_logger.LogError(ex, \"Couldn't
find translation files in {Directory}\", translationDirectoryPath)" and the
other similar catch handlers and update them to rethrow with "throw;").
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9ed2866-fee6-44a0-9fcf-f335dac4700d
📒 Files selected for processing (4)
.github/workflows/manifest.ymlTranslator/Models/LanguageInfo.csTranslator/Program.csTranslator/Services/ManifestGenerator.cs
Comment on lines
+41
to
+47
| uses: EndBug/add-and-commit@v10 | ||
| with: | ||
| default_author: github_actor | ||
| add: ./manifest.json # if the working directory is the repo root dir | ||
| message: "Update manifest.json" | ||
| commit: "" | ||
| push: true No newline at end of file |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What do GitHub’s official security hardening guidelines recommend for pinning third-party actions (tag vs full-length commit SHA)?
💡 Result:
GitHub's official security hardening guidelines recommend pinning third-party actions to a full-length commit SHA as the most secure option. This is the only way to use an action as an immutable release, mitigating risks from malicious updates since it requires a SHA-1 collision to compromise. They warn against short SHAs due to ambiguity risks from forks. Pinning to a tag is acceptable only if you fully trust the creator (e.g., verified badge on Marketplace), but tags are mutable and can be moved or deleted by repository access compromise. GitHub provides repository/organization policies to enforce full-length SHA pinning.
Citations:
- 1: https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions?learn=getting_started&learnProduct=actions
- 2: https://docs.github.com/enterprise-server@3.0/actions/security-guides/security-hardening-for-github-actions
- 3: https://docs.github.com/en/actions/reference/security/secure-use
- 4: https://docs.github.com/en/actions/how-tos/security-for-github-actions/security-guides/security-hardening-for-github-actions
🏁 Script executed:
# Check if the file exists and verify the content at lines 41-47
if [ -f .github/workflows/manifest.yml ]; then
echo "=== File exists. Showing lines 35-50 for context ==="
sed -n '35,50p' .github/workflows/manifest.yml
else
echo "File not found at .github/workflows/manifest.yml"
# Try to find it
find . -name "manifest.yml" -type f 2>/dev/null | head -20
fiRepository: btcpayserver/btcpayserver-translator
Length of output: 547
🌐 Web query:
What is the current commit SHA for EndBug/add-and-commit v10 release on GitHub
💡 Result:
The current commit SHA for the EndBug/add-and-commit v10 release (v10.0.0) on GitHub is 290ea2c423ad77ca9c62ae0f5b224379612c0321. This is confirmed as the commit tagged with v10.0.0 on March 22, 2026, which bumps the version from 9.1.4 to 10.0.0 in package.json and package-lock.json. There is also a tag "v10" pointing to the same commit. No newer v10 releases exist as of 2026-04-29.
Citations:
- 1: https://github.com/EndBug/add-and-commit/commits?author=EndBug
- 2: https://github.com/EndBug/add-and-commit/releases/tag/v10.0.0
- 3: https://github.com/EndBug/add-and-commit/tags
- 4: EndBug/add-and-commit@290ea2c
🏁 Script executed:
# Search for all 'uses:' directives in the manifest.yml file
grep -n "uses:" .github/workflows/manifest.ymlRepository: btcpayserver/btcpayserver-translator
Length of output: 201
Pin third-party GitHub Action by commit SHA to reduce supply-chain risk.
On Line 41, EndBug/add-and-commit@v10 uses a mutable tag. Pin to a full-length commit SHA per GitHub's official security hardening guidelines, which recommend this as the only way to use an action as an immutable release.
Proposed fix
- uses: EndBug/add-and-commit@v10
+ uses: EndBug/add-and-commit@290ea2c423ad77ca9c62ae0f5b224379612c0321🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/manifest.yml around lines 41 - 47, Replace the mutable tag
"uses: EndBug/add-and-commit@v10" with an immutable full-length commit SHA for
the EndBug/add-and-commit action; locate the uses line (uses:
EndBug/add-and-commit@v10) and update it to the corresponding full commit SHA
(e.g., uses: EndBug/add-and-commit@<full-commit-sha>) so the workflow is pinned
to an exact commit per GitHub security guidelines, and verify the SHA matches
the intended release before committing.
r1ckstardev
approved these changes
Apr 29, 2026
r1ckstardev
left a comment
r1ckstardev
left a comment
Collaborator
There was a problem hiding this comment.
Approved. Build clean, tests 40/40 on .NET 10 RC.2. Issue #52 acceptance criteria met (15 languages, BCP47 / native / file / sha / maintainer / updated fields, manifest.yml workflow with workflow_dispatch trigger). Stack tells a clean story: tests added, manifest generator built with FileNameToCode dictionary + project-anchored defaults via ResolveProjectDirectory, follow-up addressed timestamp drift + async maintainer fetch + workflow_dispatch. Thanks Tim.
This was referenced Jun 12, 2026
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
2 participants
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.
This PR tracks Phase 1 of the BTCPay Server translations revamp roadmap.
Changes
ManifestGeneratorservice : build and write the manifest filegenerate-manifest: (dotnet run -- generate-manifest)mainwhen a translation file changes + runsgenerate-manifestcommand, commits and pushesmanifest.jsonManifestGeneratorservice