fix: bound Inventory failure feedback - #195
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough인벤토리 집계와 비동기 작업에 상태 초기화와 세대·루트 검사를 추가했습니다. 오류 메시지와 오류 표시를 분리하고 접근성 속성을 적용했습니다. 릴리스 아티팩트 검사를 플랫폼별 디렉터리로 제한하고 릴리스 작업에 검증 단계를 추가했습니다. Changes인벤토리 오류 처리
릴리스 아티팩트 검증
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔴 Critical · up to The change narrows customer-visible Inventory error feedback and prevents stale inventory results, but the commit remains blocked by a failed required release verification gate, missing required approval and coverage requirements, plus a bounded concurrency issue that can leave an outdated warning visible after a newer success. Sequence Diagram(s)sequenceDiagram
participant Inventory as Inventory.svelte
participant Policy as inventoryInsightPolicy
participant API as Inventory API
Inventory->>Policy: 요청 루트와 세대 검사
Policy->>API: 유효한 집계 또는 인사이트 요청
API-->>Inventory: 비동기 결과 또는 오류 반환
Inventory->>Inventory: 최신 결과만 상태에 반영
Inventory-->>Inventory: 오류 유형별 메시지와 경고 표시
sequenceDiagram
participant ReleaseWorkflow as attest-release
participant ArtifactSet as release-artifacts
participant Verifier as verify-release-artifacts.sh
ReleaseWorkflow->>ArtifactSet: 릴리스 아티팩트 다운로드
ReleaseWorkflow->>Verifier: 디렉터리와 run_attempt 전달
Verifier->>ArtifactSet: 플랫폼별 파일과 체크섬 검사
ArtifactSet-->>Verifier: 검증 결과 반환
Verifier-->>ReleaseWorkflow: 성공 또는 실패 반환
ReleaseWorkflow->>ReleaseWorkflow: 검증 후 SBOM 생성
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
summarizeUnknown() had no generation guard, unlike the sibling reasonUnknownExtensions() fire-and-forget call. Triggering a new inventory load() while a prior summarize call was still in flight let the stale response overwrite summary/summaryError with data computed from the previous report. Capture the load generation at call start and gate every state write (success, failure, and the busy/loaded finally block) on it still being current; also reset summaryBusy in load() so a superseded summarize can't leave the button permanently disabled. Addresses a reviewer finding on PR #195. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
opencode-agent
Bot
added
priority: medium
Pull request was converted to draft
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| async function summarizeUnknown() { | ||
| const root = scannedRoot; | ||
| if (!root) return; | ||
| const generation = loadGeneration; |
There was a problem hiding this comment.
📝 Info: Summary staleness guard reuses the current generation
summarizeUnknown captures const generation = loadGeneration without incrementing, tying the summary to the current load. A later load() or root change bumps loadGeneration and clears summary state, so any in-flight summary is discarded rather than shown against a newer report.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } finally { | ||
| busy = false; | ||
| if (generation === loadGeneration) { | ||
| busy = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
📝 Info: busy cannot remain stuck
The finally clears busy only when generation === loadGeneration. The generation diverges only through the root-change $effect, which itself sets busy = false, or a fresh load(), which the disabled button blocks while busy. So busy always resolves to false.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } catch { | ||
| } catch (error) { | ||
| model = null; | ||
| modelStatusError = inventoryFailureMessage("model-status", error); |
There was a problem hiding this comment.
🟡 Stale model failure survives download
When startup status finishes after a post-download check, loadModel overwrites the newer result. A successful download can appear missing or failed.
Prompt for agents
Add request freshness tracking for model-status calls in src/lib/Inventory.svelte. The mount effect and doDownload can run loadModel concurrently; an older success or failure can overwrite the status returned after download. Bind model and modelStatusError writes to the latest model-status request, while preserving modelBusy and modelError behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
Preserve the four-file Inventory privacy/async-state delta while adopting canonical #264 release and hosted-runner prerequisite changes through ancestry only. No release/workflow source is copied into the Inventory owner. Signed-off-by: Seongho Bae <me@seonghobae.me>
1 participant
Purpose
Prevent arbitrary backend exception text, stale prior Inventory evidence, and silent advisory failures from crossing the desktop boundary while preserving existing inventory, rules, model, ontology, summary, persistence, network, filesystem, and release authority.
Current authority — 2026-09-05 KST
main:0e90f9cebadbd7f59606baaec4ca1d2f178c899a;3e33229b4157a084a1985a1c7e952f98a64f25e3;d04f97c351042d185a475b519537f4da6cf153f6;src/lib/Inventory.svelte,src/lib/inventoryErrorPrivacyContract.test.ts,src/lib/inventoryInsightPolicy.ts, andsrc/lib/inventoryInsightPolicy.test.ts. Release/Test source is inherited rather than copied;Behavior and privacy contract
Async writes are bound to both request generation and scanned-root identity, preventing an older inventory/advisory/summary response from overwriting a newer root. Replacement loads clear stale inventory, summary, advisory, ontology, and error evidence before contacting the backend. Unknown-extension reasoning remains bounded and advisory only. Inventory failure mapping is operation-specific and path-free; arbitrary backend text is not rendered into the customer surface.
Settings, ontology, model, filesystem, and release authority remain with their existing owners; this PR does not absorb those domains.
Required before integration
Keep Draft until #264 is integrated or equivalently present on protected lineage and one unchanged exact #195 head satisfies every applicable native/central Test/Release/Security/SAST/CodeQL/OSV/Scorecard/review/ruleset gate plus repository-wide exact owned-production coverage through #337. No self-approval, force-push, destructive rebase, gate weakening, administrative bypass, or predecessor-evidence transfer.