Skip to content

Clarify private investigations and simplify sharing confirmation - #1650

Merged
nadaverell merged 1 commit into
mainfrom
fix/investigation-sharing-copy-cleanup
Sep 6, 2026
Merged

nadaverell merged 1 commit into
mainfrom
fix/investigation-sharing-copy-cleanup

Conversation

@nadaverell

@nadaverell nadaverell commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Make investigation privacy visible even when sharing controls are unavailable, and keep the sharing confirmation focused on its actual audience and capabilities rather than a redundant generic warning.

Changes

  • Private hosted investigations without sharing controls retain a Private badge. Its tooltip says: “Only you can view this investigation. Other organization members don’t have access.” It does not guess why sharing is unavailable.
  • The sharing confirmation retains the disclosure that organization members can read the entire investigation and continue or stop it, along with its confirm/cancel actions.
  • Add an optional showWarning prop to the shared ConfirmDialog, defaulting to true. Only the investigation-sharing dialog opts out; existing warning and destructive-dialog defaults remain unchanged.

OSS vs hosted

These investigation affordances apply to hosted runs with visibility metadata, including SaaS and self-hosted Hub. Standalone OSS gains no sharing controls or permissions. Existing callers of the shared dialog retain their current behavior.

Testing

  • Frontend typecheck and full make build (frontend, embed, Go binary).
  • 24 targeted tests: DiagnoseSurface plus three dialog-rendering tests covering disclosure/action preservation, opt-out, and unchanged warning/destructive defaults.
  • Visual-test: skipped for this small copy/affordance change; no new screenshots or live E2E claims.

Release notes

Companion settings-copy PR: https://github.com/skyhook-dev/radar-hub-web/pull/283 (independent; no runtime dependency).

Before publishing radar-app, publish the k8s-ui version containing showWarning, then raise radar-app's @skyhook-io/k8s-ui peer minimum (and matching lock metadata) to that actual release. Merely updating Cloud's installed package is not sufficient: the package contract must exclude incompatible older peers. Adopt the matching packages in Cloud. The version is intentionally not guessed before the release is selected.

No backend API, authorization, retention, or investigation-execution changes. Package publication and deployment are not part of this PR.

@nadaverell
nadaverell requested a review from hisco as a code owner September 6, 2026 19:55
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Clarify private investigation visibility and sharing confirmation

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Preserve Private badges when users cannot manage hosted investigation visibility.
• Remove redundant sharing warnings while retaining audience disclosure and confirmation actions.
• Add regression tests for warning opt-out and backward-compatible dialog defaults.
Diagram

graph TD
  Run["Run metadata"] --> Control["Visibility control"] --> Manage{"Can manage?"} -->|No| Private{"Private visibility?"} -->|Yes| Badge["Private badge"]
  Manage -->|Yes| Share["Share action"] --> Props["Dialog props"] --> Dialog["Confirm dialog"]
Loading
High-Level Assessment

The optional, default-enabled showWarning prop is the appropriate backward-compatible seam: it lets this sharing flow remove redundant boilerplate without changing warning or destructive defaults elsewhere. Adding another dialog variant or injecting custom children was considered but would conflate visual severity with warning visibility or unnecessarily replace standard dialog content.

Files changed (3) +48 / -1

Enhancement (1) +3 / -1
ConfirmDialog.tsxMake confirmation warning boilerplate optional +3/-1

Make confirmation warning boilerplate optional

• Adds a 'showWarning' prop that controls the standard warning block. It defaults to 'true', preserving behavior for all existing callers.

packages/k8s-ui/src/components/ui/ConfirmDialog.tsx

Bug fix (1) +8 / -0
DiagnoseSurface.tsxClarify private visibility and simplify sharing confirmation +8/-0

Clarify private visibility and simplify sharing confirmation

• Shows a Private badge and explicit access tooltip for private investigations when visibility controls are unavailable. The organization-sharing confirmation opts out of generic warning boilerplate while retaining its detailed disclosure and actions.

web/src/components/diagnose/DiagnoseSurface.tsx

Tests (1) +37 / -0
ConfirmDialog.test.tsxCover optional and default confirmation warnings +37/-0

Cover optional and default confirmation warnings

• Adds static-rendering tests proving warning dialogs can omit generic boilerplate without losing disclosure or actions. Also verifies existing warning and destructive defaults remain unchanged.

packages/k8s-ui/src/components/ui/ConfirmDialog.test.tsx

@qodo-code-review

qodo-code-review Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Package consumers keep the extra warning ✗ Dismissed 🐞 Bug ≡ Correctness
Description
@skyhook-io/radar-app now passes showWarning to ConfirmDialog, but its peer dependency still
permits @skyhook-io/k8s-ui >=1.13.4, including releases whose component contract predates that
prop. When an external consumer resolves one of those allowed versions, source type-checking rejects
the dialog call or transpiled code silently ignores the prop, so the sharing confirmation retains
the warning this change is meant to remove.
Code

web/src/components/diagnose/DiagnoseSurface.tsx[287]

+        showWarning={false}
Relevance

●●● Strong

Peer floor must include the newly required prop’s release for source-consuming package consumers.

PR-#1585

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The app exposes source files as its package entry point and declares @skyhook-io/k8s-ui as a peer
with the unchanged >=1.13.4 floor, while its local dialog wrapper imports the component directly
from that peer. The sharing surface now relies on the newly introduced prop, so the package contract
must require a UI release containing it.

web/package.json[12-16]
web/package.json[43-45]
web/src/components/ui/ConfirmDialog.tsx[1-1]
packages/k8s-ui/src/components/ui/ConfirmDialog.tsx[18-21]
web/src/components/diagnose/DiagnoseSurface.tsx[284-288]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The app package now requires the `showWarning` capability, but its UI peer dependency range still allows older releases without that prop.

## Issue Context
External consumers may type-check against an incompatible component contract or silently retain the generic warning. Raise the minimum to the published `k8s-ui` version that introduces `showWarning`, then regenerate dependency metadata.

## Fix Focus Areas
- web/package.json[43-45]
- web/src/components/diagnose/DiagnoseSurface.tsx[284-288]
- package-lock.json[6609-6611]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 41 rules
Review mode: ⚖️ Balanced: This is a localized UI behavior and shared-dialog API change affecting investigation privacy and sharing disclosures, so it carries meaningful user-facing and contract risk despite the small diff.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread web/src/components/diagnose/DiagnoseSurface.tsx
@nadaverell
nadaverell merged commit db08317 into main Sep 6, 2026
9 checks passed
@nadaverell
nadaverell deleted the fix/investigation-sharing-copy-cleanup branch September 6, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant