chore: Phase 0 INV resolutions + MS-001/002/004 quick-wins#2020
chore: Phase 0 INV resolutions + MS-001/002/004 quick-wins#2020qvidal01 wants to merge 2 commits into
Conversation
Records four audit-pipeline Phase-0 investigation decisions in the operator-facing docs. Full rationale and supporting evidence for each INV is captured in .claude/pipeline/inv-resolutions-2026-05-12.md (gitignored), generated from .claude/pipeline/roadmap-2026-05-12.md. INV-004 / UNK-004 — Coverage thresholds in vitest.config.ts are regression floors (set by d958fa6), not targets. Documented as such in apps/desktop/CONTRIBUTING.md; ratchet plan tracked by IDEA-011 / MS-008. INV-005 / UNK-005 — services/profile/profile-service.ts imports @anthropic-ai/sdk solely for typed-exception API-key validation when users add a profile. Decision: document the exception in CLAUDE.md rather than refactor; new code outside this file still forbidden from using @anthropic-ai/sdk. INV-006 / UNK-007 — SERPER_API_KEY is embedded at build time by electron.vite.config.ts but is not supplied by any CI workflow, so the Serper search provider ships effectively disabled in CI release builds. Corrected the misleading "search works out of the box" comment; documented all four build-time embedded constants and their CI-secret sources in RELEASE.md. Guardrail tracked by IDEA-007 / MS-005. INV-007 / UNK-008 — main/platform/ and shared/platform.{ts,cjs} are complementary (main-process path/executable utilities vs. typed cross-process platform detection), not duplicates. CLAUDE.md's "Platform abstraction" rule amended to acknowledge both layers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bundles three audit-pipeline Phase-1 milestones into one clean commit. Each change is independently safe; together they close the unblocked low-risk items so the larger Phase-1 work (MS-003 logger, MS-005 build-key guardrail) has a tidier baseline. MS-001 — Tidy the repo surface (alias + cruft cleanup) IDEA-004 / RISK-015 — apps/desktop/vitest.config.ts aliases now mirror apps/desktop/tsconfig.json exactly (@ → src/renderer, plus the @preload/@features/@components/@hooks/@lib aliases that tsconfig defines). 0 tests used the old broader @/ mapping, so no test refactors needed. IDEA-014 / RISK-010 — Removed the 264-line apps/desktop/src/main/ipc-handlers/changelog-handlers.ts.bk backup file. Added *.bk, *.bak, *.orig, *~ patterns to .gitignore so the pattern doesn't reappear. IDEA-016 / RISK-007 — Removed the 114-byte empty pnpm-lock.yaml stub at the repo root. The project actively maintains package-lock.json; the empty pnpm stub was swinging package-manager auto-detection in tools like Renovate. MS-002 — Tighten pinning on native + security-critical dependencies IDEA-010 / RISK-008 — Converted 17 caret ranges to tilde ranges in apps/desktop/package.json: the Vercel AI SDK group (ai + @ai-sdk/{amazon-bedrock,anthropic,azure,google,groq,mcp,mistral, openai,openai-compatible,xai}), the @anthropic-ai/sdk holdout (kept per INV-005), and the native + Electron-runtime deps (@libsql/client, @lydell/node-pty, @sentry/electron, electron-log, electron-updater). electron itself was already exact-pinned at 40.0.0. package-lock.json regenerated via `npm install --package-lock-only`; no version drift. MS-004 deletions only (cleanup portion) IDEA-008 / RISK-009 + INV-001 — Deleted card_data.txt (zero references in the codebase; filename invited future accidental paste of real card data). Filename-pattern addition to secret-scanner.ts + pre-commit hook deferred to the rest of MS-004. IDEA-013 / RISK-011 + INV-002 — Deleted /run.py/agent.py and its parent directory (orphan from the Python→TS migration; 0-byte file, no references). Separate dead-instruction problem in apps/desktop/prompts/{planner,followup_planner}.md and source comments — referencing auto-claude/run.py — is filed for the next /audit cycle (not in scope here). IDEA-015 / RISK-012 + INV-003 — Removed "libs/*" from the root package.json workspaces array (no libs/ directory exists and no planned split was documented). package-lock.json reflects the change. Validation: - apps/desktop $ npm run lint → 0 errors (831 pre-existing warnings unchanged) - apps/desktop $ npm run typecheck → 0 errors - apps/desktop $ npm test → 4602 passing, 1 pre-existing failure in src/renderer/components/github-issues/utils/__tests__/ github-error-parser.test.ts (verified against pre-change state; same failure exists; unrelated to this PR) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates project documentation, refines dependency versioning, and adjusts test configurations. Key changes include adding documentation for build-time embedded keys and coverage strategies, pinning several AI-related dependencies to tilde versions in apps/desktop/package.json, and updating Vitest path aliases to match the renderer's configuration. Feedback suggests extending the tilde pinning to other AI stack dependencies for consistency and re-adding a @main alias in the Vitest configuration to avoid brittle relative imports in main-process tests.
| "@modelcontextprotocol/sdk": "^1.27.1", | ||
| "@openrouter/ai-sdk-provider": "^2.3.1", |
There was a problem hiding this comment.
For consistency with the other AI-related and volatile dependencies pinned in this PR (MS-002), consider also pinning @modelcontextprotocol/sdk and @openrouter/ai-sdk-provider to tilde (~) versions. This helps prevent unexpected breaking changes in the AI stack during minor version updates, which is the stated goal of this batch of changes.
| "@modelcontextprotocol/sdk": "^1.27.1", | |
| "@openrouter/ai-sdk-provider": "^2.3.1", | |
| "@modelcontextprotocol/sdk": "~1.27.1", | |
| "@openrouter/ai-sdk-provider": "~2.3.1", |
| '@': resolve(__dirname, 'src/renderer'), | ||
| '@shared': resolve(__dirname, 'src/shared'), | ||
| '@preload': resolve(__dirname, 'src/preload'), | ||
| '@features': resolve(__dirname, 'src/renderer/features'), | ||
| '@components': resolve(__dirname, 'src/renderer/shared/components'), | ||
| '@hooks': resolve(__dirname, 'src/renderer/shared/hooks'), | ||
| '@lib': resolve(__dirname, 'src/renderer/shared/lib') |
There was a problem hiding this comment.
The removal of the @main alias (previously pointing to src/main) and the redirection of @ to src/renderer significantly changes the import resolution strategy for tests. While this aligns with the renderer's tsconfig.json, it leaves the main process without a convenient alias, forcing the use of relative paths in main-process tests. To maintain a consistent developer experience and avoid brittle relative imports, consider adding a @main alias that points to src/main in both tsconfig.json and this Vitest configuration.
'@': resolve(__dirname, 'src/renderer'),
'@main': resolve(__dirname, 'src/main'),
'@shared': resolve(__dirname, 'src/shared'),
'@preload': resolve(__dirname, 'src/preload'),
'@features': resolve(__dirname, 'src/renderer/features'),
'@components': resolve(__dirname, 'src/renderer/shared/components'),
'@hooks': resolve(__dirname, 'src/renderer/shared/hooks'),
'@lib': resolve(__dirname, 'src/renderer/shared/lib')
📝 WalkthroughWalkthroughThis PR consolidates documentation clarifications about build-time API key embedding, updates desktop app test and dependency configurations, and includes general repository cleanup by removing workspace patterns and orphaned files. ChangesAPI Key Embedding & SDK Documentation
Desktop Application Configuration
Repository Cleanup & Workspace Reorganization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.gitignore:
- Around line 35-38: Remove the duplicate ignore pattern '*.bak' from the
repository's .gitignore by keeping it only in the logical "IDE & Editors"
section and removing the second occurrence in the "Misc" section; locate the two
entries by searching for the literal pattern '*.bak' and delete the redundant
one so the pattern appears just once.
In `@apps/desktop/CONTRIBUTING.md`:
- Around line 110-112: Update the "### Coverage Strategy (INV-004 / UNK-004)"
header to make the internal codes explicit to readers: either append " (internal
tracking)" after the codes or remove the codes from the visible header and move
them into an inline doc comment directly below the header mentioning they are
internal references; also add a short parenthetical note after the
`.claude/pipeline/roadmap-2026-05-12.md` reference stating that this roadmap is
an internal, gitignored file and may be inaccessible to external contributors so
they should file issues/PRs against the public docs instead.
In `@apps/desktop/vitest.config.ts`:
- Around line 32-33: Remove the internal audit tracking codes "(IDEA-004 /
RISK-015)" from the inline comment that begins "Mirrors
apps/desktop/tsconfig.json paths..." in vitest.config.ts; either delete the
parenthetical codes or move them to an external audit log/commit message per
policy so the committed source contains only the descriptive comment about test
import resolution.
- Around line 32-42: The alias map in vitest.config.ts declares four aliases
that point to non-existent directories (keys '@features', '@components',
'@hooks', '@lib') which will break imports during tests; fix by either (A)
creating the missing directories under src/renderer/shared or
src/renderer/features to match those alias targets so the paths resolve at
runtime, or (B) remove those four alias entries from the alias object in
vitest.config.ts and mirror the same removals in tsconfig.json paths so both
test resolver and TypeScript config stay consistent; locate the alias object in
vitest.config.ts and update the keys '@features', '@components', '@hooks', and
'@lib' (and mirror changes in tsconfig.json) accordingly.
In `@RELEASE.md`:
- Line 203: The RELEASE.md line points to a gitignored roadmap
`.claude/pipeline/roadmap-2026-05-12.md`; replace that reference with a durable,
committed tracker (e.g., a committed docs file or a GitHub issue/PR link) so
maintainers can find the guardrail tracking; update the phrase that currently
says "tracked by `IDEA-007` / `MS-005` in
`.claude/pipeline/roadmap-2026-05-12.md`" to instead reference the chosen
durable identifier (for example "tracked in ISSUE-123" or "tracked in
docs/roadmap.md#IDEA-007 / MS-005") and ensure the rest of the sentence about
wiring `SERPER_API_KEY` remains unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 01874fb8-2192-4e7a-9bc2-723b6dee6133
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.gitignoreCLAUDE.mdRELEASE.mdapps/desktop/CONTRIBUTING.mdapps/desktop/electron.vite.config.tsapps/desktop/package.jsonapps/desktop/src/main/ipc-handlers/changelog-handlers.ts.bkapps/desktop/vitest.config.tscard_data.txtpackage.jsonrun.py/agent.py
💤 Files with no reviewable changes (2)
- card_data.txt
- apps/desktop/src/main/ipc-handlers/changelog-handlers.ts.bk
| *.bk | ||
| *.bak | ||
| *.orig | ||
| *~ |
There was a problem hiding this comment.
Duplicate pattern: *.bak appears twice in the file.
The pattern *.bak is already present on line 128 in the "Misc" section. Having duplicate ignore patterns creates unnecessary maintenance burden and can confuse future contributors.
🧹 Proposed fix to remove the duplicate
Remove the duplicate *.bak from line 128 since it's already covered here in the IDE & Editors section where backup patterns logically belong:
# ===========================
# Misc
# ===========================
*.local
-*.bak
*.tmp
*.temp🤖 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 @.gitignore around lines 35 - 38, Remove the duplicate ignore pattern '*.bak'
from the repository's .gitignore by keeping it only in the logical "IDE &
Editors" section and removing the second occurrence in the "Misc" section;
locate the two entries by searching for the literal pattern '*.bak' and delete
the redundant one so the pattern appears just once.
| ### Coverage Strategy (INV-004 / UNK-004) | ||
|
|
||
| The Vitest coverage thresholds in `vitest.config.ts` (`lines: 22, branches: 17, functions: 19, statements: 22`) are **regression floors**, not targets. They were set by commit `d958fa65` (*"harden CI/CD with coverage enforcement"*) as the post-Python-elimination baseline. PRs must not drop coverage below these floors. Raising the floors and adding per-module thresholds for high-risk paths (`ai/security`, `ai/tools`, `ai/agent`, `ipc-handlers`) is tracked by `IDEA-011` / `MS-008` in `.claude/pipeline/roadmap-2026-05-12.md`. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that commit d958fa65 exists in the repository
git cat-file -t d958fa65 2>/dev/null || echo "Commit not found"
git log --oneline -1 d958fa65 2>/dev/nullRepository: AndyMik90/Aperant
Length of output: 165
Consider clarifying internal tracking codes in the section header.
The section header includes internal audit codes (INV-004 / UNK-004) in public-facing contributor documentation. While harmless, external contributors won't understand these references. Consider adding a brief note like "(internal tracking)" or relocating them to a doc comment if they're only for team reference.
The reference to .claude/pipeline/roadmap-2026-05-12.md is also inaccessible to external contributors since this directory is gitignored. This is acceptable for now but worth noting when the roadmap is updated.
🤖 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 `@apps/desktop/CONTRIBUTING.md` around lines 110 - 112, Update the "###
Coverage Strategy (INV-004 / UNK-004)" header to make the internal codes
explicit to readers: either append " (internal tracking)" after the codes or
remove the codes from the visible header and move them into an inline doc
comment directly below the header mentioning they are internal references; also
add a short parenthetical note after the
`.claude/pipeline/roadmap-2026-05-12.md` reference stating that this roadmap is
an internal, gitignored file and may be inaccessible to external contributors so
they should file issues/PRs against the public docs instead.
| // Mirrors apps/desktop/tsconfig.json paths exactly so tests resolve | ||
| // imports the same way production builds do (audit: IDEA-004 / RISK-015). |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Internal audit codes in code comments.
Similar to the CONTRIBUTING.md file, this comment includes internal audit tracking codes (IDEA-004 / RISK-015) that reference gitignored pipeline artifacts. While useful for team traceability, consider whether these belong in the committed codebase or if they should be documented elsewhere (e.g., commit message, separate audit log).
🤖 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 `@apps/desktop/vitest.config.ts` around lines 32 - 33, Remove the internal
audit tracking codes "(IDEA-004 / RISK-015)" from the inline comment that begins
"Mirrors apps/desktop/tsconfig.json paths..." in vitest.config.ts; either delete
the parenthetical codes or move them to an external audit log/commit message per
policy so the committed source contains only the descriptive comment about test
import resolution.
| // Mirrors apps/desktop/tsconfig.json paths exactly so tests resolve | ||
| // imports the same way production builds do (audit: IDEA-004 / RISK-015). | ||
| alias: { | ||
| '@': resolve(__dirname, 'src'), | ||
| '@main': resolve(__dirname, 'src/main'), | ||
| '@renderer': resolve(__dirname, 'src/renderer'), | ||
| '@shared': resolve(__dirname, 'src/shared') | ||
| '@': resolve(__dirname, 'src/renderer'), | ||
| '@shared': resolve(__dirname, 'src/shared'), | ||
| '@preload': resolve(__dirname, 'src/preload'), | ||
| '@features': resolve(__dirname, 'src/renderer/features'), | ||
| '@components': resolve(__dirname, 'src/renderer/shared/components'), | ||
| '@hooks': resolve(__dirname, 'src/renderer/shared/hooks'), | ||
| '@lib': resolve(__dirname, 'src/renderer/shared/lib') | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify alias target directories exist
echo "=== Checking alias paths exist ==="
for dir in "src/renderer" "src/shared" "src/preload" "src/renderer/features" "src/renderer/shared/components" "src/renderer/shared/hooks" "src/renderer/shared/lib"; do
if [ -d "apps/desktop/$dir" ]; then
echo "✓ $dir exists"
else
echo "✗ $dir NOT FOUND"
fi
done
echo -e "\n=== Comparing with tsconfig.json paths ==="
cat apps/desktop/tsconfig.json | jq '.compilerOptions.paths' 2>/dev/null || echo "Could not parse tsconfig.json paths"
echo -e "\n=== Checking for removed `@main/`@renderer imports in tests ==="
rg -n --type ts --glob '**/*.test.ts' --glob '**/*.spec.ts' -e 'from ["\x27]@main' -e 'from ["\x27]@renderer' apps/desktop/src/ || echo "No references to removed aliases found"Repository: AndyMik90/Aperant
Length of output: 805
Fix alias configuration: four alias paths point to non-existent directories.
The aliases do mirror apps/desktop/tsconfig.json exactly, but verification reveals a critical issue: four of the seven alias paths don't exist in the codebase:
@features→src/renderer/features(missing)@components→src/renderer/shared/components(missing)@hooks→src/renderer/shared/hooks(missing)@lib→src/renderer/shared/lib(missing)
The existing directories (src/renderer, src/shared, src/preload) are present and correctly configured. No test files currently reference the removed @main or @renderer aliases. However, tests attempting to use the missing alias paths will fail at import time. Either create these directories or remove the aliases from both vitest.config.ts and tsconfig.json to match actual structure.
🤖 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 `@apps/desktop/vitest.config.ts` around lines 32 - 42, The alias map in
vitest.config.ts declares four aliases that point to non-existent directories
(keys '@features', '@components', '@hooks', '@lib') which will break imports
during tests; fix by either (A) creating the missing directories under
src/renderer/shared or src/renderer/features to match those alias targets so the
paths resolve at runtime, or (B) remove those four alias entries from the alias
object in vitest.config.ts and mirror the same removals in tsconfig.json paths
so both test resolver and TypeScript config stay consistent; locate the alias
object in vitest.config.ts and update the keys '@features', '@components',
'@hooks', and '@lib' (and mirror changes in tsconfig.json) accordingly.
|
|
||
| **Current state of Serper search in shipped builds.** Because no CI workflow supplies `SERPER_API_KEY`, the `WebSearch` tool's Serper provider effectively ships **disabled** in CI release builds: the build embeds an empty string and `serper-search.ts` returns the "missing key" error path. Local builds with a populated `apps/desktop/.env` will bundle the developer's key. | ||
|
|
||
| A guardrail that refuses to embed any `__*_API_KEY__` constant in non-CI builds (unless `ALLOW_LOCAL_KEY_EMBED=1` is explicitly set) is tracked by `IDEA-007` / `MS-005` in `.claude/pipeline/roadmap-2026-05-12.md`. Wiring `SERPER_API_KEY` into the release-workflow secrets is **not** currently planned; if you want Serper search to work in shipped builds, add `SERPER_API_KEY: ${{ secrets.SERPER_API_KEY }}` to the build steps in `release.yml`, `beta-release.yml`, and `build-prebuilds.yml`. |
There was a problem hiding this comment.
Replace gitignored roadmap reference with a durable tracker link.
Pointing release docs to .claude/pipeline/roadmap-2026-05-12.md is not portable for most maintainers/readers. Please reference a committed doc or GitHub issue instead.
🤖 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 `@RELEASE.md` at line 203, The RELEASE.md line points to a gitignored roadmap
`.claude/pipeline/roadmap-2026-05-12.md`; replace that reference with a durable,
committed tracker (e.g., a committed docs file or a GitHub issue/PR link) so
maintainers can find the guardrail tracking; update the phrase that currently
says "tracked by `IDEA-007` / `MS-005` in
`.claude/pipeline/roadmap-2026-05-12.md`" to instead reference the chosen
durable identifier (for example "tracked in ISSUE-123" or "tracked in
docs/roadmap.md#IDEA-007 / MS-005") and ensure the rest of the sentence about
wiring `SERPER_API_KEY` remains unchanged.
1 participant
Summary
Two commits from the audit→ideate→roadmap pipeline run on
cba7a027. Documentation of Phase-0 investigation outcomes plus the unblocked Phase-1 quick-win cleanup batch.docs:— Records four Phase-0 investigation decisions (INV-004 through INV-007) inCLAUDE.md,RELEASE.md,apps/desktop/CONTRIBUTING.md, and the misleading comment inelectron.vite.config.ts. Surfaces a real finding:SERPER_API_KEYis embedded at build time but never supplied by any CI workflow — Serper search ships effectively disabled in CI release builds today (corrected in the comment + documented in RELEASE.md).chore:— Ships three Phase-1 milestones in one batch: MS-001 (vitest alias align +.bkremoval + emptypnpm-lock.yamlremoval), MS-002 (17 caret-to-tilde pins on native + AI-SDK + Sentry-Electron + electron-updater deps), and the deletion portion of MS-004 (card_data.txt,run.py/,libs/*workspace declaration).Validation
npm run lintnpm run typechecknpm testsrc/renderer/components/github-issues/utils/__tests__/github-error-parser.test.ts(verified against the pre-change state — same failure exists, unrelated to this PR)Pipeline traceability
Each change traces back to an audit-pipeline artifact (all in
.claude/pipeline/, gitignored):audit-2026-05-12.mdideas-2026-05-12.mdroadmap-2026-05-12.mdWhat's not in this PR
.claude/pipeline/inv-resolutions-2026-05-12.mdlocally.(card|payment|ssn|tax_id)_data\.*toapps/desktop/src/main/ai/security/secret-scanner.tsplus a pre-commit hook (the secret-scanner hardening half of IDEA-008) — deferred to a follow-up.electron.vite.config.tsbuild-time-key guardrail (ALLOW_LOCAL_KEY_EMBEDswitch + non-CI refusal) is documented inRELEASE.mdbut not yet implemented.apps/desktop/prompts/{planner,followup_planner}.mdand source comments still instruct generated agents to runpython auto-claude/run.py --spec ...— that path doesn't exist post-Python→TS migration. Will become an IDEA on the next/auditcycle.Test plan
github-error-parserfailure ondevelop)@/,@shared/,@preload/,@features/,@components/,@hooks/, or@lib/(or trust the typecheck + lint passes)apps/desktop/.gitignorestill suppresses.bk/.bak/.orig/~files on subsequent commits🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
.gitignoreto ignore common backup and swap file patterns.Documentation
Refactor