fix: correct sitemap.xml hostname and base path#19
Merged
nschimme merged 2 commits intoMay 12, 2026
Conversation
Updated scripts/ci-env.sh to correctly identify the mume.github.io repository and set the VITE_HOSTNAME to https://docs.mume.org. Also explicitly set VITE_BASE to / for the official production environment and improved CNAME reading by stripping whitespace. Updated .github/workflows/deploy.yml to prioritize VITE_BASE from the environment script and ensure no double-slashes are introduced when the base path is root.
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjust CI environment script and deploy workflow to correctly derive VITE_HOSTNAME and VITE_BASE for the official mume.github.io production deployment and avoid malformed hostnames or double-slash base paths. Flow diagram for CI env and deploy VITE_HOSTNAME/VITE_BASE logicflowchart TD
subgraph CI_env_sh
A[Start CI env] --> B[Set OWNER from GITHUB_REPOSITORY_OWNER or mume]
B --> C[Set REPO from GITHUB_REPOSITORY or OWNER/mume.github.io]
C --> D{CNAME exists and OWNER = mume}
D -->|yes| E[Read CNAME and strip whitespace to CNAME_VAL]
E --> F[Set VITE_HOSTNAME=https://CNAME_VAL]
D -->|no| G{PREVIEW_HOST is set}
G -->|yes| H[Set VITE_HOSTNAME=https://PREVIEW_HOST + PREVIEW_PREFIX]
G -->|no| I{OWNER = mume and REPO = mume.github.io}
I -->|yes| J[Set VITE_HOSTNAME=https://docs.mume.org]
I -->|yes| K[Set VITE_BASE=/]
I -->|no| L{REPO = OWNER.github.io}
L -->|yes| M[Set VITE_HOSTNAME=https://OWNER.github.io]
L -->|no| N[Set VITE_HOSTNAME=https://OWNER.github.io/REPO]
end
subgraph Deploy_workflow
O[Compute pages base_path] --> P{VITE_BASE in env}
P -->|yes| Q[Use env.VITE_BASE]
P -->|no| R[Set VITE_BASE=base_path/]
Q --> S[npm run build]
R --> S
end
F --> O
H --> O
J --> O
M --> O
N --> O
K --> O
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
ci-env.sh, you can avoid the subshell and unnecessarycatby using input redirection withtr, e.g.export VITE_HOSTNAME="https://$(tr -d '[:space:]' < CNAME)", which is a bit more idiomatic and efficient in sh scripts. - Since
OWNERandREPOare now computed at the top ofci-env.sh, you could simplify the later conditional branches by reusing them consistently and removing the redundant defaulting logic that was previously inside the fallback block.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ci-env.sh`, you can avoid the subshell and unnecessary `cat` by using input redirection with `tr`, e.g. `export VITE_HOSTNAME="https://$(tr -d '[:space:]' < CNAME)"`, which is a bit more idiomatic and efficient in sh scripts.
- Since `OWNER` and `REPO` are now computed at the top of `ci-env.sh`, you could simplify the later conditional branches by reusing them consistently and removing the redundant defaulting logic that was previously inside the fallback block.
## Individual Comments
### Comment 1
<location path="scripts/ci-env.sh" line_range="10-12" />
<code_context>
# Set VITE_HOSTNAME from CNAME file if it exists
-if [ -f CNAME ] && [ "$GITHUB_REPOSITORY_OWNER" = "mume" ]; then
- export VITE_HOSTNAME="https://$(cat CNAME)"
+if [ -f CNAME ] && [ "$OWNER" = "mume" ]; then
+ export VITE_HOSTNAME="https://$(cat CNAME | tr -d '[:space:]')"
else
# Fallback for PR previews if host/prefix are provided via env
</code_context>
<issue_to_address>
**suggestion:** Avoid subshell `cat` usage and tighten quoting when reading CNAME.
You can avoid the subshell and improve readability by assigning the cleaned value first:
```sh
if [ -f CNAME ] && [ "$OWNER" = "mume" ]; then
cname=$(tr -d '[:space:]' < CNAME)
export VITE_HOSTNAME="https://$cname"
fi
```
```suggestion
# Set VITE_HOSTNAME from CNAME file if it exists
if [ -f CNAME ] && [ "$OWNER" = "mume" ]; then
cname=$(tr -d '[:space:]' < CNAME)
export VITE_HOSTNAME="https://$cname"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Updated scripts/ci-env.sh to correctly identify the mume.github.io repository and set the VITE_HOSTNAME to https://docs.mume.org. Used idiomatic shell patterns for reading CNAME and ensured consistent variable usage. Updated .github/workflows/deploy.yml to prioritize VITE_BASE from the environment script and ensure no double-slashes are introduced when the base path is root.
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".github/workflows/deploy.yml" line_range="56" />
<code_context>
if [ -f CNAME ] && [ "${{ github.repository_owner }}" = "mume" ]; then cp CNAME dist/CNAME; fi
env:
- VITE_BASE: ${{ steps.pages.outputs.base_path }}/
+ VITE_BASE: ${{ env.VITE_BASE || format('{0}/', steps.pages.outputs.base_path) }}
- name: Collect PRs to deploy
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `env.VITE_BASE` here will not pick up values written via `GITHUB_ENV` in earlier steps.
`env.VITE_BASE` is evaluated at workflow compile time, so it can only see values defined statically in the YAML, not those written later via `$GITHUB_ENV` (e.g. by `ci-env.sh`). This means the expression will never use the runtime override and will always prefer the YAML-defined value.
To use the dynamic value, either define `VITE_BASE` at job-level in YAML for the production case, or rely solely on the shell environment in the build step (for example, `VITE_BASE=${VITE_BASE:-"${{ steps.pages.outputs.base_path }}/"} npm run build`). Otherwise, the intended production override `VITE_BASE="/"` may be ignored.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if [ -f CNAME ] && [ "${{ github.repository_owner }}" = "mume" ]; then cp CNAME dist/CNAME; fi | ||
| env: | ||
| VITE_BASE: ${{ steps.pages.outputs.base_path }}/ | ||
| VITE_BASE: ${{ env.VITE_BASE || format('{0}/', steps.pages.outputs.base_path) }} |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Using env.VITE_BASE here will not pick up values written via GITHUB_ENV in earlier steps.
env.VITE_BASE is evaluated at workflow compile time, so it can only see values defined statically in the YAML, not those written later via $GITHUB_ENV (e.g. by ci-env.sh). This means the expression will never use the runtime override and will always prefer the YAML-defined value.
To use the dynamic value, either define VITE_BASE at job-level in YAML for the production case, or rely solely on the shell environment in the build step (for example, VITE_BASE=${VITE_BASE:-"${{ steps.pages.outputs.base_path }}/"} npm run build). Otherwise, the intended production override VITE_BASE="/" may be ignored.
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
1 participant
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.
Updated scripts/ci-env.sh to correctly identify the mume.github.io repository and set the VITE_HOSTNAME to https://docs.mume.org. Also explicitly set VITE_BASE to / for the official production environment and improved CNAME reading by stripping whitespace.
Updated .github/workflows/deploy.yml to prioritize VITE_BASE from the environment script and ensure no double-slashes are introduced when the base path is root.
Summary by Sourcery
Align production hostname and base path configuration for the mume.github.io deployment.
Enhancements: