feat: replace yarn with pnpm#471
Merged
Merged
Conversation
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
5 times, most recently
from
a456e8f to
3247e46
Compare
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
3247e46 to
dd6b2b2
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
2 times, most recently
from
145de61 to
15c2363
Compare
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
15c2363 to
5be2023
Compare
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
5be2023 to
2e3717f
Compare
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
2e3717f to
0c211d9
Compare
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
2 times, most recently
from
dc89771 to
07e6a30
Compare
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
07e6a30 to
8d7c7d0
Compare
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
8d7c7d0 to
46b8538
Compare
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
46b8538 to
1fde8eb
Compare
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/apollo-mocks/headerMenuMock.ts (1)
15-527:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate remaining padded base64 menu IDs in other mocks
The repo still contains legacy padded base64 IDs insrc/test/apollo-mocks/footerMenuMock.ts(e.g.,menu.id: 'dGVybTo1Nw=='andmenuItems.nodes[].id: 'cG9zdDo0MjY=',cG9zdDo0Mjc='). If the change to unpadded IDs was meant to be consistent, update this file as well; otherwise, document that padded IDs are intentionally kept there.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/apollo-mocks/headerMenuMock.ts` around lines 15 - 527, The footer mock still uses padded base64 IDs (e.g., menu.id 'dGVybTo1Nw==' and menuItems.nodes[].id 'cG9zdDo0MjY=', 'cG9zdDo0Mjc=') while headerMenuMock uses unpadded IDs; update src/test/apollo-mocks/footerMenuMock.ts to use the unpadded forms (remove trailing '=' padding) for the same fields so they match the MenuQueryResponse shape used elsewhere (update the values for menu.id and each menuItems.nodes[].id), or add a short comment in the file documenting that padded IDs are intentionally kept if that was the deliberate choice.
🧹 Nitpick comments (5)
scripts/update-runtime-env.ts (1)
28-31: ⚡ Quick winUse path.join for cross-platform compatibility.
Line 30 uses string concatenation to build the path, which is less robust than
path.join.♻️ Proposed fix
const configurationFile: string = path.join( __dirname, - '../public/' + configFile + '../public', + configFile );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/update-runtime-env.ts` around lines 28 - 31, The path for configurationFile is built with string concatenation; update the construction to use path.join for cross-platform correctness. In the configurationFile assignment (the variable named configurationFile that currently uses __dirname and configFile), call path.join with components (e.g., __dirname, '..', 'public', configFile) instead of concatenating '../public/' + configFile so separators are handled correctly across OSes..github/workflows/ci.yml (1)
11-16: ⚡ Quick winConsider adding explicit permissions to reduce attack surface.
The workflow inherits default permissions which may be overly broad. Explicitly defining minimal permissions follows the principle of least privilege.
🛡️ Proposed explicit permissions
jobs: common: + permissions: + contents: read + checks: write + pull-requests: write uses: City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@mainNote: Adjust the permissions based on what the reusable workflow actually requires.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 11 - 16, The workflow currently inherits default permissions for the reusable job "common" (uses: City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main); add an explicit permissions block under that job to restrict token scopes to the minimum required by the reusable workflow (e.g., explicitly set permissions: contents: read, packages: read, pull-requests: write or whatever the reusable workflow needs) instead of relying on defaults—review the reusable workflow's steps to determine exact scopes and add only those permission keys.src/utils/__tests__/envUtils.test.ts (1)
5-34: ⚡ Quick winConsider adding a test for the
process.envfallback path.The tests validate resolution from
window._env_andimport.meta.env, but the middle fallback—process.env—is untested. According to the implementation, this path is used by Vitest and server-side tooling when runtime injection is unavailable.🧪 Proposed test to cover process.env fallback
it('falls back to import.meta.env when runtime and process env are missing', () => { (globalThis.window as Window & { _env_?: Record<string, unknown> })._env_ = undefined; delete process.env.BASE_URL; expect(getEnvValue('BASE_URL')).toBe(import.meta.env.BASE_URL); }); + + it('falls back to process.env when runtime env is missing', () => { + (globalThis.window as Window & { _env_?: Record<string, unknown> })._env_ = + undefined; + process.env.TEST_ENV_UTILS_VALUE = 'process-env-value'; + + expect(getEnvValue('TEST_ENV_UTILS_VALUE')).toBe('process-env-value'); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/__tests__/envUtils.test.ts` around lines 5 - 34, Add a test that verifies getEnvValue falls back to process.env: set (globalThis.window as Window & { _env_?: Record<string, unknown> })._env_ = undefined, assign process.env.TEST_ENV_UTILS_VALUE = '456' (or another distinct value), then assert expect(getEnvValue('TEST_ENV_UTILS_VALUE')).toBe('456'); name the test to reflect the "process.env fallback" and rely on the existing afterEach cleanup that deletes TEST_ENV_UTILS_VALUE and restores process.env/runtime env..env.test (1)
5-6: ⚡ Quick winRemove placeholder syntax that doesn't expand.
The
$npm_package_*syntax won't be expanded bydotenvand is overridden by the generator script. These lines are effectively ignored since the generator explicitly reads frompackage.json.♻️ Suggested fix
VITE_APP_CMS_URI=https://kultus.app-staging.hkih.hion.dev/graphql -VITE_APP_APPLICATION_NAME=$npm_package_name -VITE_APP_VERSION=$npm_package_version +# VITE_APP_APPLICATION_NAME and VITE_APP_VERSION are auto-populated from package.json # Sentry🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.test around lines 5 - 6, Remove the non-expanding placeholder assignments for VITE_APP_APPLICATION_NAME and VITE_APP_VERSION in .env.test (the lines using $npm_package_name and $npm_package_version); either delete these two lines or replace them with concrete values (e.g., the actual app name and version) so dotenv isn't given unusable placeholders—note the generator already pulls these from package.json, so keep .env.test free of $npm_package_* placeholders and rely on the generator or explicit literal values instead..env.runtime.template (1)
23-24: ⚡ Quick winDocument that these variables are auto-populated.
VITE_APP_APPLICATION_NAMEandVITE_APP_VERSIONare automatically overridden by the generator script frompackage.json, so any values set here will be ignored. Consider adding a comment to clarify this behavior or removing them from the template entirely.📝 Suggested documentation fix
VITE_APP_IDLE_TIMEOUT_IN_MS= -VITE_APP_APPLICATION_NAME= -VITE_APP_VERSION= +# VITE_APP_APPLICATION_NAME= # Auto-populated from package.json +# VITE_APP_VERSION= # Auto-populated from package.json VITE_APP_COMMITHASH=🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.runtime.template around lines 23 - 24, Add a short comment above VITE_APP_APPLICATION_NAME and VITE_APP_VERSION explaining these values are auto-populated/overridden by the generator script from package.json (so values in this template are ignored) or remove these lines entirely; update the .env.runtime.template to either include that explanatory comment referencing VITE_APP_APPLICATION_NAME and VITE_APP_VERSION or delete those two entries to avoid confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.dockerignore:
- Around line 7-8: Remove the allowlist entries that expose local secret files
(the lines matching "!.env" and "!.env*") and instead explicitly allow only
non-secret template files (for example add patterns like "!.env.example" or
"!.env.template" or specific template filenames you ship) so real .env files
remain excluded from the Docker build context; update the .dockerignore by
deleting the "!.env" / "!.env*" entries and adding explicit template patterns
for any env templates you need to include.
In @.env.test:
- Line 29: The VITE_APP_OIDC_SCOPE environment variable is unquoted and contains
spaces; update the VITE_APP_OIDC_SCOPE entry to wrap the multi-word value in
quotes (e.g., "openid profile email") so dotenv parses it consistently (match
the quoted style used in .env.example) and ensure any tooling that reads
VITE_APP_OIDC_SCOPE receives the full space-separated scope string.
In @.github/workflows/ci.yml:
- Line 12: Replace the mutable branch ref used for the reusable workflow with a
pinned commit SHA: update the uses entry
"City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main" to point to
the exact commit hash (i.e.
"City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@<commit-sha>")
obtained from the repo (for example via the gh API or git) so the workflow is
immutable and supply-chain secure.
In `@package.json`:
- Line 116: The package.json npm script "test:debug" contains a typo in its
flags: replace the incorrect "----no-file-parallelism" with the correct
"--no-file-parallelism" in the "test:debug" script so the flag uses two hyphens;
update the value of the "test:debug" entry to use the corrected flag.
In `@pipelines/kultus-admin-ui-review.yml`:
- Around line 44-45: Remove the temporary pinned pipeline ref by deleting the
line containing "ref: refs/heads/feat/PT-2045-replace-yarn-with-pnpm" (and the
preceding TODO comment) in the pipelines/kultus-admin-ui-review.yml so the
pipeline no longer targets this PR branch; if a stable branch or tag is required
instead, replace that value with the intended branch/ref.
In `@pnpm-workspace.yaml`:
- Around line 1-3: The current minimumReleaseAge in pnpm-workspace.yaml is set
to 10 minutes which weakens supply-chain protections; update the
minimumReleaseAge setting to a more secure value (e.g., 1440 for 24 hours or
10080 for 1 week) in the pnpm-workspace.yaml file by changing the
minimumReleaseAge entry to the chosen higher integer to allow time for community
vetting and reduce risk from compromised or malicious new releases.
In `@src/clients/apiReportClient/apiReportClient.ts`:
- Around line 6-8: Ensure we fail fast if the environment var is missing: check
getEnvValue('VITE_APP_API_REPORT_URI') before creating axiosClient and throw a
clear error (or process.exit) when it's undefined/empty so axios.create({
baseURL: ... }) never receives an undefined baseURL; update the module that
defines axiosClient (the axiosClient constant in apiReportClient.ts) to read the
env into a local variable, validate it, and only then pass it into axios.create.
In `@src/index.tsx`:
- Around line 26-28: The tracePropagationTargets value produced by
getEnvValue('VITE_APP_SENTRY_TRACE_PROPAGATION_TARGETS') is split into [''] when
unset, which `@sentry/browser` treats as a wildcard; update the logic that builds
tracePropagationTargets so you split the env string, trim each entry, filter out
empty strings, and only include the tracePropagationTargets option when the
resulting array has at least one entry (otherwise omit the option so Sentry uses
its default allowlist). Reference the tracePropagationTargets assignment and the
getEnvValue call to locate and replace the current .split(',') handling.
In `@src/utils/getLinkedEventsInternalId.ts`:
- Around line 4-8: The helper getLinkedEventsInternalId builds a URL using
getEnvValue('VITE_APP_LINKEDEVENTS_API_URI') without guarding for a
missing/empty value, which can produce invalid IDs; update
getLinkedEventsInternalId to validate the env value returned by
getEnvValue('VITE_APP_LINKEDEVENTS_API_URI') (throw or return a clear
error/empty result) before interpolating, and construct the path using a
normalized base (ensure no duplicate slashes) so functions referencing
getLinkedEventsInternalId get only well-formed URLs.
---
Outside diff comments:
In `@src/test/apollo-mocks/headerMenuMock.ts`:
- Around line 15-527: The footer mock still uses padded base64 IDs (e.g.,
menu.id 'dGVybTo1Nw==' and menuItems.nodes[].id 'cG9zdDo0MjY=', 'cG9zdDo0Mjc=')
while headerMenuMock uses unpadded IDs; update
src/test/apollo-mocks/footerMenuMock.ts to use the unpadded forms (remove
trailing '=' padding) for the same fields so they match the MenuQueryResponse
shape used elsewhere (update the values for menu.id and each
menuItems.nodes[].id), or add a short comment in the file documenting that
padded IDs are intentionally kept if that was the deliberate choice.
---
Nitpick comments:
In @.env.runtime.template:
- Around line 23-24: Add a short comment above VITE_APP_APPLICATION_NAME and
VITE_APP_VERSION explaining these values are auto-populated/overridden by the
generator script from package.json (so values in this template are ignored) or
remove these lines entirely; update the .env.runtime.template to either include
that explanatory comment referencing VITE_APP_APPLICATION_NAME and
VITE_APP_VERSION or delete those two entries to avoid confusion.
In @.env.test:
- Around line 5-6: Remove the non-expanding placeholder assignments for
VITE_APP_APPLICATION_NAME and VITE_APP_VERSION in .env.test (the lines using
$npm_package_name and $npm_package_version); either delete these two lines or
replace them with concrete values (e.g., the actual app name and version) so
dotenv isn't given unusable placeholders—note the generator already pulls these
from package.json, so keep .env.test free of $npm_package_* placeholders and
rely on the generator or explicit literal values instead.
In @.github/workflows/ci.yml:
- Around line 11-16: The workflow currently inherits default permissions for the
reusable job "common" (uses:
City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main); add an
explicit permissions block under that job to restrict token scopes to the
minimum required by the reusable workflow (e.g., explicitly set permissions:
contents: read, packages: read, pull-requests: write or whatever the reusable
workflow needs) instead of relying on defaults—review the reusable workflow's
steps to determine exact scopes and add only those permission keys.
In `@scripts/update-runtime-env.ts`:
- Around line 28-31: The path for configurationFile is built with string
concatenation; update the construction to use path.join for cross-platform
correctness. In the configurationFile assignment (the variable named
configurationFile that currently uses __dirname and configFile), call path.join
with components (e.g., __dirname, '..', 'public', configFile) instead of
concatenating '../public/' + configFile so separators are handled correctly
across OSes.
In `@src/utils/__tests__/envUtils.test.ts`:
- Around line 5-34: Add a test that verifies getEnvValue falls back to
process.env: set (globalThis.window as Window & { _env_?: Record<string,
unknown> })._env_ = undefined, assign process.env.TEST_ENV_UTILS_VALUE = '456'
(or another distinct value), then assert
expect(getEnvValue('TEST_ENV_UTILS_VALUE')).toBe('456'); name the test to
reflect the "process.env fallback" and rely on the existing afterEach cleanup
that deletes TEST_ENV_UTILS_VALUE and restores process.env/runtime env.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c10b52b-95d3-4760-8eca-af9b0edac3cd
⛔ Files ignored due to path filters (5)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (32)
.dockerignore.env.development.local.example.env.example.env.runtime.template.env.test.github/workflows/ci.yml.gitignore.husky/pre-commit.nvmrcDockerfileREADME.mdcompose.ymlindex.htmlpackage.jsonpipelines/kultus-admin-ui-review.ymlplaywright.config.tspnpm-workspace.yamlscripts/tsconfig.jsonscripts/update-runtime-env.tssrc/clients/apiReportClient/apiReportClient.tssrc/domain/app/AppConfig.tssrc/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsxsrc/domain/venue/utils.tssrc/headless-cms/components/__tests__/CmsPage.test.tsxsrc/index.tsxsrc/playwright/README.mdsrc/test/apollo-mocks/headerMenuMock.tssrc/test/vitest-setup.tssrc/utils/__tests__/envUtils.test.tssrc/utils/envUtils.tssrc/utils/getLinkedEventsInternalId.tssrc/vite-env.d.ts
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
0a9c4ec to
d1fafde
Compare
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
d1fafde to
48a8796
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
10-16: ⚡ Quick winDeclare explicit least-privilege workflow permissions.
This workflow has no
permissions:block, so token scope depends on repo/org defaults. Set explicit minimal scopes to harden CI behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 10 - 16, Add an explicit least-privilege permissions block to the workflow job (the "common" job that uses City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main) so the workflow token isn't granted repo/org defaults; insert a top-level permissions: mapping that declares only the scopes required by your CI (for example at minimum permissions: contents: read and actions: read, and add checks: write or id-token: write only if the shared workflow requires them) to harden the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Line 19: The Dockerfile COPY currently uses a wildcard `.env*` which can
accidentally include local secret files; update the COPY command that includes
index.html, vite.config.ts, eslint.config.mjs, .prettierrc.json to explicitly
list only allowed env template files (e.g., .env.example or .env.production)
instead of `.env*`, ensuring you preserve the --chown=default:root flag and only
add the specific filenames you intend to bake into the image so secrets like
.env.local are not copied into image layers.
In `@README.md`:
- Line 310: The ordered list in README.md has a numbering gap: the step "Run the
Node application in development mode with `pnpm dev`. At this point you should
be also running the local APIs if you have selected to use them." is labeled
`5.` but should follow the previous top-level `3.` — change that list item to
`4.` (or renumber the sequence so top-level steps are sequential) and verify
nested items/indentation are correct so Markdown renders the steps in proper
order.
In `@scripts/update-runtime-env.ts`:
- Around line 17-23: The code is currently serializing the entire env map (env)
into the public browser config, exposing non-public keys; update the logic that
writes/serializes env so it filters env to only include public keys (e.g., keys
that start with "VITE_APP_" or your chosen public prefix) before creating the JS
output. Locate the dotenv.config usage and the code that writes the browser
config from env (references: env, USE_TEST_ENV) and replace the direct
serialization with a new object built by iterating env and copying only keys
matching /^VITE_APP_/ (or configured public prefix), then serialize that
filtered object into the generated file. Ensure override behavior is preserved
and no other env values are written to the client bundle.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 10-16: Add an explicit least-privilege permissions block to the
workflow job (the "common" job that uses
City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main) so the
workflow token isn't granted repo/org defaults; insert a top-level permissions:
mapping that declares only the scopes required by your CI (for example at
minimum permissions: contents: read and actions: read, and add checks: write or
id-token: write only if the shared workflow requires them) to harden the
workflow.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 51ddee04-8951-4056-90d7-87cc2866e11d
⛔ Files ignored due to path filters (5)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (33)
.dockerignore.env.development.local.example.env.example.env.runtime.template.env.test.github/workflows/ci.yml.gitignore.husky/pre-commit.nvmrcDockerfileREADME.mdcompose.ymlindex.htmlpackage.jsonpipelines/kultus-admin-ui-review.ymlplaywright.config.tspnpm-workspace.yamlscripts/tsconfig.jsonscripts/update-runtime-env.tssrc/clients/apiReportClient/apiReportClient.tssrc/domain/app/AppConfig.tssrc/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsxsrc/domain/venue/utils.tssrc/headless-cms/components/__tests__/CmsPage.test.tsxsrc/index.tsxsrc/playwright/README.mdsrc/test/apollo-mocks/footerMenuMock.tssrc/test/apollo-mocks/headerMenuMock.tssrc/test/vitest-setup.tssrc/utils/__tests__/envUtils.test.tssrc/utils/envUtils.tssrc/utils/getLinkedEventsInternalId.tssrc/vite-env.d.ts
✅ Files skipped from review due to trivial changes (10)
- .nvmrc
- .env.development.local.example
- scripts/tsconfig.json
- .env.example
- src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
- src/test/apollo-mocks/footerMenuMock.ts
- index.html
- src/playwright/README.md
- src/domain/venue/utils.ts
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (15)
- src/test/vitest-setup.ts
- src/headless-cms/components/tests/CmsPage.test.tsx
- pipelines/kultus-admin-ui-review.yml
- src/index.tsx
- .dockerignore
- src/utils/getLinkedEventsInternalId.ts
- .husky/pre-commit
- src/utils/tests/envUtils.test.ts
- playwright.config.ts
- src/clients/apiReportClient/apiReportClient.ts
- compose.yml
- src/utils/envUtils.ts
- src/test/apollo-mocks/headerMenuMock.ts
- src/domain/app/AppConfig.ts
- package.json
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
48a8796 to
0eb0883
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Dockerfile (1)
19-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace
.env*with an explicit allowlist.This still copies every
.env*file from the build context into the layer. Keep the template/example files explicit so local secrets cannot be baked into the image by accident.🧩 Suggested fix
-COPY --chown=default:root index.html vite.config.ts eslint.config.mjs .prettierrc.json .env* ./ +COPY --chown=default:root index.html vite.config.ts eslint.config.mjs .prettierrc.json ./ +COPY --chown=default:root .env.runtime.template .env.example .env.development.local.example .env.test ./🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` at line 19, The Dockerfile COPY line currently uses a glob `.env*` which risks including local secret env files; update the COPY invocation that contains "COPY --chown=default:root index.html vite.config.ts eslint.config.mjs .prettierrc.json .env* ./" to explicitly list only safe template/example env files (for example `.env.example` and/or `.env.template`) instead of the `.env*` glob so no accidental local `.env` or `.env.local` files are baked into the image.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Around line 16-19: The Dockerfile runs the command pnpm update-runtime-env
before the .env.runtime.template is added to the image, causing builds to fail
or produce wrong runtime config; fix by ensuring .env.runtime.template (or other
env templates) are copied into the image before invoking pnpm
update-runtime-env—either move the pnpm update-runtime-env step to after the
COPY that includes .env* or add an earlier COPY that places
.env.runtime.template into the image prior to running pnpm update-runtime-env so
the update-runtime-env script can read the template.
In `@src/headless-cms/components/__tests__/CmsPage.test.tsx`:
- Around line 124-131: Update the test comment to correctly describe why we pass
deterministic overrides to fakePost: the id/postId values are used only as React
keys in CmsSidebarContent (not as DOM id attributes for LayoutArticles); the
sidebar HTML id is generated from LayoutLinkList.anchor in
CmsSidebarContentLayoutLinkList.tsx, which is what the test queries—so change or
remove the current comment to state that deterministic values keep React keys
stable rather than claiming jsdom selector safety.
---
Duplicate comments:
In `@Dockerfile`:
- Line 19: The Dockerfile COPY line currently uses a glob `.env*` which risks
including local secret env files; update the COPY invocation that contains "COPY
--chown=default:root index.html vite.config.ts eslint.config.mjs
.prettierrc.json .env* ./" to explicitly list only safe template/example env
files (for example `.env.example` and/or `.env.template`) instead of the `.env*`
glob so no accidental local `.env` or `.env.local` files are baked into the
image.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: beaf6739-4cdb-4e63-af42-cfdf0e783607
⛔ Files ignored due to path filters (5)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (33)
.dockerignore.env.development.local.example.env.example.env.runtime.template.env.test.github/workflows/ci.yml.gitignore.husky/pre-commit.nvmrcDockerfileREADME.mdcompose.ymlindex.htmlpackage.jsonpipelines/kultus-admin-ui-review.ymlplaywright.config.tspnpm-workspace.yamlscripts/tsconfig.jsonscripts/update-runtime-env.tssrc/clients/apiReportClient/apiReportClient.tssrc/domain/app/AppConfig.tssrc/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsxsrc/domain/venue/utils.tssrc/headless-cms/components/__tests__/CmsPage.test.tsxsrc/index.tsxsrc/playwright/README.mdsrc/test/apollo-mocks/footerMenuMock.tssrc/test/apollo-mocks/headerMenuMock.tssrc/test/vitest-setup.tssrc/utils/__tests__/envUtils.test.tssrc/utils/envUtils.tssrc/utils/getLinkedEventsInternalId.tssrc/vite-env.d.ts
✅ Files skipped from review due to trivial changes (12)
- .nvmrc
- src/index.tsx
- src/domain/venue/utils.ts
- compose.yml
- .env.development.local.example
- src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
- src/vite-env.d.ts
- src/playwright/README.md
- .env.example
- index.html
- .gitignore
- .husky/pre-commit
🚧 Files skipped from review as they are similar to previous changes (13)
- src/test/vitest-setup.ts
- scripts/tsconfig.json
- playwright.config.ts
- src/utils/getLinkedEventsInternalId.ts
- .dockerignore
- src/utils/tests/envUtils.test.ts
- pnpm-workspace.yaml
- src/test/apollo-mocks/footerMenuMock.ts
- scripts/update-runtime-env.ts
- package.json
- src/test/apollo-mocks/headerMenuMock.ts
- src/domain/app/AppConfig.ts
- src/utils/envUtils.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 381-383: The README’s doctoc example command ("pnpm doctoc .")
does not match the pre-commit hook which runs "pnpm doctoc . -u"; update the
README entry to use the exact command used by the hook ("pnpm doctoc . -u") so
docs match behavior, or alternatively change the hook to remove the "-u"
flag—make the change where the README lists the doctoc command and ensure the
string "pnpm doctoc . -u" is used if choosing to align docs to the hook.
- Line 386: Update the README line that points to the lint-staged config:
replace the incorrect reference to "./lint-staged.config.js" with an instruction
that the lint-staged configuration lives in package.json under the "lint-staged"
field (e.g., mention "see package.json -> lint-staged"). Ensure the command
description remains the same and that the README clearly directs readers to the
package.json lint-staged entry.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12436551-56f7-48d6-a97a-0dd0280dee78
⛔ Files ignored due to path filters (5)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (33)
.dockerignore.env.development.local.example.env.example.env.runtime.template.env.test.github/workflows/ci.yml.gitignore.husky/pre-commit.nvmrcDockerfileREADME.mdcompose.ymlindex.htmlpackage.jsonpipelines/kultus-admin-ui-review.ymlplaywright.config.tspnpm-workspace.yamlscripts/tsconfig.jsonscripts/update-runtime-env.tssrc/clients/apiReportClient/apiReportClient.tssrc/domain/app/AppConfig.tssrc/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsxsrc/domain/venue/utils.tssrc/headless-cms/components/__tests__/CmsPage.test.tsxsrc/index.tsxsrc/playwright/README.mdsrc/test/apollo-mocks/footerMenuMock.tssrc/test/apollo-mocks/headerMenuMock.tssrc/test/vitest-setup.tssrc/utils/__tests__/envUtils.test.tssrc/utils/envUtils.tssrc/utils/getLinkedEventsInternalId.tssrc/vite-env.d.ts
✅ Files skipped from review due to trivial changes (9)
- .nvmrc
- src/headless-cms/components/tests/CmsPage.test.tsx
- src/domain/venue/utils.ts
- .env.example
- .env.development.local.example
- src/playwright/README.md
- src/utils/envUtils.ts
- src/test/apollo-mocks/footerMenuMock.ts
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (17)
- index.html
- playwright.config.ts
- pipelines/kultus-admin-ui-review.yml
- src/vite-env.d.ts
- scripts/tsconfig.json
- src/index.tsx
- src/clients/apiReportClient/apiReportClient.ts
- .dockerignore
- .husky/pre-commit
- src/utils/getLinkedEventsInternalId.ts
- pnpm-workspace.yaml
- scripts/update-runtime-env.ts
- src/utils/tests/envUtils.test.ts
- compose.yml
- package.json
- src/test/apollo-mocks/headerMenuMock.ts
- src/domain/app/AppConfig.ts
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
0eb0883 to
081095b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 11-16: Add an explicit GitHub Actions permissions block
immediately under the reusable workflow invocation (the common: uses:
City-of-Helsinki/... entry) to avoid default token scopes; update the job that
contains the uses: line (the "common" job) to include a minimal permissions set
that only grants what CI needs (for example, at minimum contents: read and
actions: read, and add checks: write or id-token: write only if the workflow
actually requires them)—ensure you place the new permissions: block at the same
level as uses:, secrets:, and with: and remove reliance on default scopes.
In `@index.html`:
- Around line 51-52: Update the development command in the HTML template
comment: replace the incorrect "pnpm start" string with the project's actual dev
command "pnpm dev" in the comment block that currently reads "To begin the
development, run `pnpm start`." so the docs match the real script used by the
project.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2134d53c-e87f-4140-9937-825496e446b3
⛔ Files ignored due to path filters (5)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snapis excluded by!**/*.snapsrc/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (33)
.dockerignore.env.development.local.example.env.example.env.runtime.template.env.test.github/workflows/ci.yml.gitignore.husky/pre-commit.nvmrcDockerfileREADME.mdcompose.ymlindex.htmlpackage.jsonpipelines/kultus-admin-ui-review.ymlplaywright.config.tspnpm-workspace.yamlscripts/tsconfig.jsonscripts/update-runtime-env.tssrc/clients/apiReportClient/apiReportClient.tssrc/domain/app/AppConfig.tssrc/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsxsrc/domain/venue/utils.tssrc/headless-cms/components/__tests__/CmsPage.test.tsxsrc/index.tsxsrc/playwright/README.mdsrc/test/apollo-mocks/footerMenuMock.tssrc/test/apollo-mocks/headerMenuMock.tssrc/test/vitest-setup.tssrc/utils/__tests__/envUtils.test.tssrc/utils/envUtils.tssrc/utils/getLinkedEventsInternalId.tssrc/vite-env.d.ts
✅ Files skipped from review due to trivial changes (9)
- .nvmrc
- .husky/pre-commit
- src/domain/venue/utils.ts
- src/playwright/README.md
- src/headless-cms/components/tests/CmsPage.test.tsx
- src/vite-env.d.ts
- .gitignore
- .env.development.local.example
- src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
- src/test/vitest-setup.ts
- .env.example
- src/utils/getLinkedEventsInternalId.ts
- compose.yml
- scripts/tsconfig.json
- src/test/apollo-mocks/footerMenuMock.ts
- pipelines/kultus-admin-ui-review.yml
- src/index.tsx
- src/utils/envUtils.ts
- playwright.config.ts
- src/utils/tests/envUtils.test.ts
- scripts/update-runtime-env.ts
- pnpm-workspace.yaml
- package.json
- src/test/apollo-mocks/headerMenuMock.ts
- src/domain/app/AppConfig.ts
- .dockerignore
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
081095b to
6ddcdf9
Compare
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
Riippi
approved these changes
Jun 8, 2026
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
6ddcdf9 to
15fc981
Compare
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
15fc981 to
ca4b3de
Compare
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
ca4b3de to
2c27577
Compare
Refs: PT-2045
tjgofore
force-pushed
the
feat/PT-2045-replace-yarn-with-pnpm
branch
from
2c27577 to
45411fb
Compare
|
|
KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀 |
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.
Description ✨
Issues 🐛
Closes 🙅♀️
PT-2045
Refs: PT-2045
Related 🤝
Testing ⚗️
Automated tests ⚙️️
Manual testing 👷♂️
Screenshots 📸
Additional notes 🗒️
Summary by CodeRabbit
New Features
Chores
Documentation
Tests