test: Identifier ID generation and Protected.isSensitiveWrite edge cases#387
test: Identifier ID generation and Protected.isSensitiveWrite edge cases#387anandgupta42 wants to merge 2 commits intomainfrom
Conversation
Add first-ever tests for Identifier (monotonic ID generation, prefix validation, sort ordering, timestamp extraction) and expand Protected.isSensitiveWrite coverage for private key extensions, remaining sensitive dirs/files, and Windows paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> https://claude.ai/code/session_011oA4CKCyHc7Bv7MizQ5bh8
The mock for TrainingStore.count was missing `context` and `rule` keys, and the fs.readFile mock had an overload mismatch. These errors blocked the pre-push typecheck hook. https://claude.ai/code/session_011oA4CKCyHc7Bv7MizQ5bh8
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review.
Tip: disable this comment in your organization's Code Review settings.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
Merges 9 test PRs (#390, #394, #395, #396, #397, #398, #399, #401, #387) into a single PR, discarding 4 duplicates (#391, #392, #393, #400). **New test coverage:** - `id.test.ts` — 14 tests: Identifier generation, monotonic ordering, timestamp extraction, Zod schema - `shell.test.ts` — 9 tests: shell blacklist enforcement for fish/nu, cache reset, fallback - `path-traversal.test.ts` — 7 tests (new): `Protected.isSensitiveWrite` for .pem/.key, .gnupg, .env variants - `ignore.test.ts` — 7 tests (new): `FileIgnore.match` directory patterns, globs, whitelist overrides - `paths-parsetext.test.ts` — 16 tests: `ConfigPaths.parseText` env/file substitution, JSONC parsing - `finops-formatting.test.ts` — 14 tests: `formatBytes`/`truncateQuery` edge cases - `finops-role-access.test.ts` — 10 tests: RBAC `formatGrants`/`formatHierarchy`/`formatUserRoles` - `tool-lookup.test.ts` — 4 tests: Zod schema introspection for tool parameters - `summary-git-path.test.ts` — 10 tests: `unquoteGitPath` decoding for non-ASCII filenames - `tracing.test.ts` — 2 tests (added): Recap loop detection boundaries - `patch.test.ts` — 9 tests (added): `maybeParseApplyPatchVerified` entry point coverage - `instruction.test.ts` — 13 tests (new): `InstructionPrompt.loaded()` dedup guards - `message-v2.test.ts` — 23 tests (new): `MessageV2.filterCompacted()` boundary detection **Bug fixes (discovered during testing):** - `finops-formatting.ts`: fix `formatBytes` crash on negative/NaN/fractional inputs - `finops-formatting.ts`: fix `truncateQuery` returning empty for whitespace-only, exceeding `maxLen` when < 4 - `training-import.test.ts`: fix pre-existing typecheck errors (missing `context`/`rule` mock keys) - `instruction.test.ts`, `message-v2.test.ts`: fix `MessageV2.Part` cast and branded ID type errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Consolidated into #403 |
* test: consolidated test coverage across 9 modules (133 new tests) Merges 9 test PRs (#390, #394, #395, #396, #397, #398, #399, #401, #387) into a single PR, discarding 4 duplicates (#391, #392, #393, #400). **New test coverage:** - `id.test.ts` — 14 tests: Identifier generation, monotonic ordering, timestamp extraction, Zod schema - `shell.test.ts` — 9 tests: shell blacklist enforcement for fish/nu, cache reset, fallback - `path-traversal.test.ts` — 7 tests (new): `Protected.isSensitiveWrite` for .pem/.key, .gnupg, .env variants - `ignore.test.ts` — 7 tests (new): `FileIgnore.match` directory patterns, globs, whitelist overrides - `paths-parsetext.test.ts` — 16 tests: `ConfigPaths.parseText` env/file substitution, JSONC parsing - `finops-formatting.test.ts` — 14 tests: `formatBytes`/`truncateQuery` edge cases - `finops-role-access.test.ts` — 10 tests: RBAC `formatGrants`/`formatHierarchy`/`formatUserRoles` - `tool-lookup.test.ts` — 4 tests: Zod schema introspection for tool parameters - `summary-git-path.test.ts` — 10 tests: `unquoteGitPath` decoding for non-ASCII filenames - `tracing.test.ts` — 2 tests (added): Recap loop detection boundaries - `patch.test.ts` — 9 tests (added): `maybeParseApplyPatchVerified` entry point coverage - `instruction.test.ts` — 13 tests (new): `InstructionPrompt.loaded()` dedup guards - `message-v2.test.ts` — 23 tests (new): `MessageV2.filterCompacted()` boundary detection **Bug fixes (discovered during testing):** - `finops-formatting.ts`: fix `formatBytes` crash on negative/NaN/fractional inputs - `finops-formatting.ts`: fix `truncateQuery` returning empty for whitespace-only, exceeding `maxLen` when < 4 - `training-import.test.ts`: fix pre-existing typecheck errors (missing `context`/`rule` mock keys) - `instruction.test.ts`, `message-v2.test.ts`: fix `MessageV2.Part` cast and branded ID type errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address code review findings — test artifact leakage and cleanup - summary-git-path.test.ts: use tmpdir() instead of projectRoot to prevent Storage.write() from polluting the developer's .opencode/storage/ directory - finops-role-access.test.ts: consolidate duplicate afterAll hooks to ensure ALTIMATE_TELEMETRY_DISABLED env var is cleaned up after tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
What does this PR do?
Adds first-ever unit tests for the
Identifiermodule and expands edge-case coverage forProtected.isSensitiveWrite, filling two significant gaps discovered during automated test discovery.1.
Identifier—src/id/id.ts(8 new tests)This module generates every entity ID in the system (sessions, messages, tools, permissions, etc.) and its
timestamp()function is used byTruncate.cleanup()to determine file age for retention. Zero tests existed prior to this PR. A bug in sort ordering would cause sessions/messages to display in wrong order; a bug in timestamp extraction would cause cleanup to delete recent output files or keep stale ones forever. New coverage includes:timestamp()preserves ordering (used byTruncate.cleanup()for age comparison)2.
Protected.isSensitiveWriteedge cases —src/file/protected.ts(7 new tests)This function gates whether the agent prompts the user before writing to sensitive files (credentials, private keys, config). The existing 6 tests covered
.git,.ssh,.aws,.env, andcredentials.json— but missed entire categories of sensitive patterns defined in the source. A user could write toserver.pem,.docker/config.json, or.npmrcwithout any permission prompt. New coverage includes:.pem,.key,.p12,.pfx).gnupg,.gcloud,.kube,.docker).npmrc,.pypirc,.netrc,.htpasswd,.pgpass,id_rsa,id_ed25519).env.*variant detection viastartsWithcheck with exact return valuesgit-helper.tsare not flagged)3. Fix: pre-existing typecheck errors in
training-import.test.tsThe
TrainingStore.countmock was missingcontextandrulekeys (added in a recentTrainingKindexpansion), and thefs.readFilemock had an overload type mismatch. These errors blocked the pre-push typecheck hook for all branches.Type of change
Issue for this PR
N/A — proactive test coverage via automated test discovery
How did you verify your code works?
Checklist
https://claude.ai/code/session_011oA4CKCyHc7Bv7MizQ5bh8