fix(console): explain filtered configurations - #302
Conversation
📝 WalkthroughWalkthroughThe Console now categorizes rejected configuration candidates, reports aggregate discovery counts, hides sensitive rejection details, and renders attention summaries in the dashboard. Documentation and tests cover the updated discovery, redaction, permissions, and UI behavior. ChangesConsole discovery attention reporting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Console
participant discoverConsoleConfigCatalog
participant readTrustedConfiguration
participant Dashboard
Console->>discoverConsoleConfigCatalog: discover local candidates
discoverConsoleConfigCatalog->>readTrustedConfiguration: validate each candidate
readTrustedConfiguration-->>discoverConsoleConfigCatalog: accepted config or safe reason
discoverConsoleConfigCatalog-->>Dashboard: catalog counts and attentionReasons
Dashboard->>Dashboard: render summary and refresh guidance
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/console/console-assets.ts`:
- Around line 833-849: The pluralization in the attention entry rendering
appends an extra “s” to the already-plural “private file permissions” label.
Update the labels handling in the configurationCatalogAttention block so the
file-permissions entry does not receive the suffix for counts above one, while
preserving existing singular/plural behavior for all other labels.
In `@src/console/console-config-catalog.ts`:
- Around line 584-587: Update the catch-all handling around
consoleInitializedConfigMetadata and createConfigMigrationSource so failures
after file readability is established are recorded as "invalid-configuration"
rather than "unreadable". Preserve the "unreadable" label only for genuine read
or candidate-access failures, and keep the existing candidate-stage observation
and attention recording behavior otherwise.
- Around line 423-437: Update catalogAttention so the reason ordering is backed
by an exhaustively checked structure for ConsoleConfigCatalogAttentionReason,
ensuring every union member is represented and newly added reasons cause a
compile-time failure until included. Preserve the existing output order and
filtering behavior for zero counts.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5a727a2a-e132-4205-9851-1330ceac6b86
📒 Files selected for processing (7)
docs/console-api.mdsrc/console/console-assets.tssrc/console/console-config-catalog.tssrc/console/console-config-metadata.tstests/console-dashboard-application-service.test.tstests/console-server.test.tstests/oauth-console-threat-model-docs-contract.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/console/console-config-catalog.ts (1)
375-380: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClassify post-read file changes as
unreadable.When identity and permissions remain valid but the size/content changes between
handle.stat()andreadFile(), this is a changing-file race—not an unsafe path. It currently incrementsunsafe-pathand gives incorrect repair guidance. Map those mutation cases tounreadable; reserveunsafe-pathfor identity/path replacement.🤖 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/console/console-config-catalog.ts` around lines 375 - 380, Update the post-read status classification in the identity and permission validation flow so size/content mutations detected between handle.stat() and readFile() return reason "unreadable". Keep "unsafe-path" only for identity or path replacement cases, while preserving the existing "file-permissions" classification for invalid permissions.
🤖 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.
Outside diff comments:
In `@src/console/console-config-catalog.ts`:
- Around line 375-380: Update the post-read status classification in the
identity and permission validation flow so size/content mutations detected
between handle.stat() and readFile() return reason "unreadable". Keep
"unsafe-path" only for identity or path replacement cases, while preserving the
existing "file-permissions" classification for invalid permissions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a2d40381-449b-48b4-b091-0e8bafb9d277
📒 Files selected for processing (4)
src/console/console-assets.tssrc/console/console-config-catalog.tstests/console-dashboard-application-service.test.tstests/console-server.test.ts
* fix(console): explain filtered configurations (#302) Show aggregate discovered/ready/attention counts without exposing rejected paths, explain safe repair categories, and preserve trusted discovery containment. Includes regression contracts and all review fixes. Closes #203. * fix(console): preserve valid sessions across reloads (#303) * fix(console): resume valid sessions after reload * fix(console): explain unlock rate limiting * fix(setup): explain environment secret readiness (#304) * fix(setup): explain environment secret readiness * fix: make secret readiness guidance truthful * fix: keep readiness guidance visible * fix: preserve environment readiness invariants * fix: prevent stale readiness guidance * release: prepare v0.5.2 (#306) * release: prepare v0.5.2 * test: bind release notes to current patch * test: scope release links to fixed section
Closes #203
Summary
TDD evidence
The existing unsafe-candidate contract was tightened first to require the missing aggregate fields. It failed on the prior implementation, then passed after the catalog/UI change. A separate Console shell contract and documentation contract were also observed failing before their matching UI/docs changes.
Validation
npm test— 1,784 passed, 33 skipped (platform/fixture-gated)npm run test:core— 425 passed, 26 skippednpm run test:coverage— clean rerun passed; 95.65% statements, 91.93% branches, 98.66% functionsnpm run lintnpm run typechecknpm run buildnpm run smoke:clinpm run check:pack— 48 files verifiednpm run test:package— 26 passedThe first aggregate coverage attempt hit two unchanged timing/contention failures in
upstream-manager-teardownanddoctor. Both exact tests passed alone, and both passed in the unchanged full coverage rerun. No timeout, worker, skip, containment, or coverage setting was changed.Security boundary
The browser receives only fixed reason enums and aggregate counts. Rejected candidate names, paths, configuration bytes, values, parser errors, file identities, and ACL details remain unavailable. No trust check or selected-file revalidation was weakened.
Summary by CodeRabbit
New Features
Documentation
--configlocal discovery documentation to document closed-fail behavior, platform-specific permission/ownership expectations, and safe first-time setup flow.