chore: backfill selected main fixes into v0.2#273
Conversation
# Conflicts: # .github/scripts/deepseek-pr-review.mjs
There was a problem hiding this comment.
Open-CoDesign Bot
Findings
-
[Major] DCO requirement removed from CI and documentation — The PR deletes the entire
dcojob from.github/workflows/ci.yml:67-78and removes all references to DCO sign-off fromCONTRIBUTING.md,README.md, andAGENTS.md. While the project is free to change its contribution policy, this diff makes that change implicitly as part of a larger backfill PR without any discussion in the diff body or changeset. Dropping the DCO check removes a governance signal that prevented unsigned contributions; if intentional, the PR should explain the rationale and updateAGENTS.md's "Hard Constraints" section accordingly. If unintentional, the DCO job should be restored.Suggested fix / next action:
If the project deliberately abandons DCO, updateAGENTS.mdto remove the sentence "Commits: Conventional Commits with DCO sign-off" and add a rationale in the PR body or a dedicated commit. Otherwise revert the removal of thedcojob and restoreSigned-off-byreferences in docs. -
[Minor] Locale IPC duplicates supported-locale list — In
apps/desktop/src/main/locale-ipc.ts:55-62, theregisterLocaleIpchandler duplicates theavailableLocalescheck from@open-codesign/i18nwith an inline list ('pt-br','es','es-','es_'). If a new locale is added topackages/i18n/src/index.ts, the IPC handler must be manually updated — otherwise the new locale is silently rejected. This already happened with Spanish and Portuguese:availableLocalespresumably includes them, yet the IPC still needs the hardcoded addition.Suggested fix / next action:
Replace the inline suffix checks with a dynamic check:availableLocales.some(locale => lower.startsWith(locale) || lower === locale)to avoid future drift. -
[Minor] Workspace protocol is not disabled when snapshots DB is unavailable — In
apps/desktop/src/main/index.ts:175-182,registerWorkspaceProtocolHandleris only called inside theif (dbResult.ok)branch. That is correct — the workspace protocol requires a valid DB to resolve design IDs. However, theregisterWorkspaceScheme()call at line 141 happens unconditionally beforeapp.whenReady(). This registers a privilegedworkspace://scheme even when the DB cannot be opened, meaning the protocol is registered but its handler never provides a response. If a renderer or iframe loads aworkspace://URL while the DB is broken, Electron will either show an error or hang. This is a minor risk becauseworkspace://URLs are only generated by the app itself, but it creates an unexpected failure mode.Suggested fix / next action:
MoveregisterWorkspaceScheme()inside thedbResult.okbranch, or register it conditionally after the DB is confirmed healthy. -
[Nit] Dependabot broadened to ignore all major updates —
.github/dependabot.yml:43-44changesignorefrom justelectronto"*"withversion-update:semver-major. This prevents Dependabot from ever proposing major version updates for any npm dependency. While this reduces noise, it also silently skips security-critical major upgrades (e.g., Electron major versions that patch vulnerabilities). This is a policy choice, but it should be documented in the PR or a changeset.Suggested fix / next action:
Add a comment explaining the rationale for ignoring all major updates, or revert to package-specific ignores.
Questions
- None. The diff is self-explanatory.
Summary
Review mode: initial
This large backfill PR merges main-only commits into dev/v0.2 (the product trunk). It adds Spanish locale support, a workspace:// protocol for preview asset serving, ChatGPT OAuth region-localization, dependency bumps, and CI/bot rework.
Key risks addressed:
- DCO removal (see Major finding)
- Duplicate locale list in IPC (Minor)
- Early privileged scheme registration (Minor)
- Dependabot major-update silence (Nit)
No hard constraint violations found: no new prod dependencies, no direct SDK imports, no hardcoded UI values (translations use i18n), no silent fallbacks detected in the changed paths.
Testing
Good coverage added for:
workspace-protocol.test.ts— path resolution, traversal, symlink rejection (96 lines)index.workspace.test.ts— seeded initial files works (expanded)codex-oauth-ipc.test.ts— Chinese error message expectation updated
Missing coverage:
preparePromptContext.test.tscomment-only addition (low risk)- No test that
createRuntimeTextEditorFsrejects unknown workspace directories - No test for the locale IPC duplicate-locale drift scenario
Open-CoDesign Bot
Summary
Backfills selected
origin/main-only changes onto the v0.2 product trunk without merging the old main branch wholesale. The branch keeps v0.2 architecture as source of truth, cherry-picks low-risk changes where they apply cleanly, and records covered/rewritten commits in.github/v0.2-main-backfill.md.What changed
Validation
git diff --checkpnpm lintpnpm typecheckpnpm testpnpm --filter @open-codesign/desktop buildpnpm --filter @open-codesign/desktop build:dirattempted twice;electron-vite buildpassed, but electron-builder could not downloadelectron-v39.8.9-darwin-arm64.zipbecause GitHub returned HTTP 502.Notes
Open PRs still targeting
mainshould be retargeted or rebased after this backfill lands. The final trunk switch remains a separatedev/v0.2 -> mainPR.