Improve UX with inline resources links#2174
Open
jpinsonneau wants to merge 4 commits into
Open
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughThis PR adds shared resource-model and status helpers, linked-resource extraction and inline rendering, local mock/startup tooling, and updated localization/configuration. It also expands tests across model resolution, parsing, status mapping, and navigation. ChangesLinked resources and local workflow
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 10
🧹 Nitpick comments (13)
src/linkedResources.ts (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrettier formatting.
Static analysis flags the multi-import statement formatting on this line.
🤖 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/linkedResources.ts` at line 1, The import in linkedResources.ts needs Prettier-compliant formatting because the current single-line multi-import is flagged by static analysis. Reformat the existing import from pageContext to match the project’s multiline import style, keeping the same symbols getModelKindName, K8sModelRef, resolveRefModelKey, and resolveRefModelKeyOrKind but spreading them across lines as needed.Source: Linters/SAST tools
112-120: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant recomputation in
getLinkedResourceOverflow.This calls
extractLinkableResourceRefsdirectly forlinkable, then callsextractLinkedResources, which internally recomputesextractLinkableResourceRefs(and re-runsprioritizeLinkedResources) a second time — duplicating the JSON/YAML parsing and regex-heavy extraction pipeline over the same tools/text for a single overflow calculation.♻️ Proposed fix avoiding duplicate extraction
export const getLinkedResourceOverflow = ( tools: Record<string, Tool> | undefined, responseText: string | undefined, models: Record<string, K8sModelRef>, ): { shown: number; total: number } => { const linkable = extractLinkableResourceRefs(tools, responseText, models); - const shown = extractLinkedResources(tools, responseText, models).length; - return { shown, total: linkable.length }; + const limit = resolveLinkedResourceLimit(tools, linkable.length); + return { shown: Math.min(linkable.length, limit), total: linkable.length }; };🤖 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/linkedResources.ts` around lines 112 - 120, getLinkedResourceOverflow is doing the same linkable-resource extraction twice by calling extractLinkableResourceRefs and then extractLinkedResources, which re-runs the same parsing/prioritization work. Update getLinkedResourceOverflow to compute the linked resources once and derive both shown and total from that shared result, or otherwise reuse the single extractLinkableResourceRefs output instead of invoking the pipeline again. Keep the fix localized to getLinkedResourceOverflow and the linked resource helpers it calls.src/resourceRefs.ts (1)
114-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate regex constant.
K8S_NODE_NAME_REhere is identical to the one already defined insrc/resourceListParsing.ts(Line 4). Consider exporting one and importing it to avoid drift between the two copies.🤖 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/resourceRefs.ts` at line 114, The K8S_NODE_NAME_RE regex is duplicated in resourceRefs and resourceListParsing, so update the duplicated constant to use a single shared definition instead of two copies. Export K8S_NODE_NAME_RE from resourceListParsing and import it into resourceRefs, then remove the local declaration so both ResourceRefs and ResourceListParsing rely on the same source of truth.unit-tests/linkedResourceWatch.test.ts (1)
33-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo test coverage for kind-mismatch in
matchesResourceRef.Given the missing kind check flagged in
src/linkedResourceWatch.ts(Lines 42-59), consider adding a test assertingmatchesResourceRefrejects a resource with matching name/namespace but a different kind, once that check is added.🤖 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 `@unit-tests/linkedResourceWatch.test.ts` around lines 33 - 53, Add a test in matchesResourceRef coverage to verify kind mismatches are rejected: extend the linkedResourceWatch.test.ts suite with a case where the resource name/namespace matches but the resource kind differs from the ref, and assert matchesResourceRef returns false. Use the matchesResourceRef helper and the existing Namespace-style cases as the reference point so the new test clearly exercises the kind check added in linkedResourceWatch.ts.src/linkedResourceWatch.ts (1)
42-59: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
matchesResourceRefdoesn't verify resource kind.The check only compares
metadata.nameandmetadata.namespace, not the kind. If aresourceReftransitions to a different kind but the same name/namespace (e.g. a component reused for a new resource beforeuseK8sWatchResourcere-fetches), a stale watched object of the previous kind would incorrectly be reported as matching, potentially showing status for the wrong resource kind.Consider also comparing
resource.kind(or the model's apiVersion/kind derived fromresourceRef) to close this gap.🤖 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/linkedResourceWatch.ts` around lines 42 - 59, `matchesResourceRef` only checks name and namespace, so it can treat a stale object from the wrong kind as a match. Update `matchesResourceRef` in `linkedResourceWatch` to also compare the resource kind against `resourceRef` (or the derived model kind/apiVersion from the ref) before returning true, while keeping the existing name/namespace checks intact.src/components/LinkedResourceStatus.tsx (1)
21-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
!hasMatchingResourcecheck on Line 37.After Line 33 already returns
nullforloaded && !hasMatchingResource, by the time Line 37 executes,!hasMatchingResourcecan only be true when!loadedis also true — so the check simplifies to just!loaded. Not a bug, but worth simplifying for clarity.♻️ Proposed simplification
- if (!loaded || !hasMatchingResource) { + if (!loaded) { return <Spinner aria-label={t('Loading resource status')} isInline size="sm" />; }🤖 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/components/LinkedResourceStatus.tsx` around lines 21 - 46, The loading condition in LinkedResourceStatus is redundant because the earlier null return already covers the case where loaded is true and the resource does not match. Simplify the conditional after the loadError check to only test loaded, keeping the existing behavior intact while making the logic in LinkedResourceStatus clearer.src/components/InlineLinkedResource.tsx (1)
49-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
keyprop is a no-op here.
key={resourceRefKey(resourceRef)}is set onLinkedResourceStatus, but it's rendered as a single conditional child, not inside a list/iteration, so React ignores it — it has no effect on remounting whenresourceRefchanges.🤖 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/components/InlineLinkedResource.tsx` around lines 49 - 55, The key on LinkedResourceStatus is ineffective because this conditional render is not part of a list, so React will ignore it. Remove the key from InlineLinkedResource and, if remounting on resourceRef changes is actually needed, move the identity change to the parent conditional or derive it through a different prop-driven render path using shouldShowLinkedResourceStatus, LinkedResourceStatus, and resourceRefKey as the relevant symbols.scripts/mock-ols-server.mjs (1)
973-973: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFormatting/lint cleanup: run the project formatter and drop the unused variable.
Static analysis reports numerous
prettier/prettierformatting diffs across this file (e.g. lines 69-70, 183-185, 190-191, 210, 243-245, 246-248, 367-368, 382-383, 446-448, 482, 593-594, 611-612, 668-669, 975, 999, 1002) plus an unusedconversationIddestructure here and a lowercase comment at line 970.🧹 Proposed fix
- const { body, scenarioKey, conversationId } = buildStreamBody(query); + const { body, scenarioKey } = buildStreamBody(query);Run
npx eslint --fix scripts/mock-ols-server.mjsto clear the remaining formatting diffs.🤖 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 `@scripts/mock-ols-server.mjs` at line 973, The file has broad formatter/lint drift and an unused destructured value in the stream setup. Run the project formatter/eslint fix on scripts/mock-ols-server.mjs to resolve the reported prettier/prettier issues, remove the unused conversationId from the buildStreamBody(query) destructure, and adjust the nearby lowercase comment so it matches the file’s style. Use the buildStreamBody and query-related code around the stream handler to locate the cleanup.Source: Linters/SAST tools
start-console.sh (1)
64-78: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPreflight checks validate both tools independently, even though only one is actually used.
resolve_api_server/resolve_bearer_tokenpreferocand only fall back tokubectl. But the preflight here requires bothoc(if installed) to be logged in ANDkubectl(if installed) to reach the API — even when only one of them will be used. A machine with both CLIs installed, wherekubectl's default context is unrelated to the target cluster, would fail to start even thoughocworks fine.♻️ Proposed fix: only validate the tool that will be used
-if command -v oc >/dev/null 2>&1 && ! oc whoami >/dev/null 2>&1; then - echo "error: not logged in to a cluster. Run 'oc login' (or point KUBECONFIG at a running cluster) and retry." >&2 - exit 1 -fi - -if command -v kubectl >/dev/null 2>&1 && ! kubectl cluster-info >/dev/null 2>&1; then - echo "error: cannot reach the Kubernetes API. Is your cluster running?" >&2 - echo " Try: kubectl cluster-info" >&2 - exit 1 -fi +if command -v oc >/dev/null 2>&1; then + if ! oc whoami >/dev/null 2>&1; then + echo "error: not logged in to a cluster. Run 'oc login' (or point KUBECONFIG at a running cluster) and retry." >&2 + exit 1 + fi +elif command -v kubectl >/dev/null 2>&1; then + if ! kubectl cluster-info >/dev/null 2>&1; then + echo "error: cannot reach the Kubernetes API. Is your cluster running?" >&2 + echo " Try: kubectl cluster-info" >&2 + exit 1 + fi +fi🤖 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 `@start-console.sh` around lines 64 - 78, The preflight checks in start-console.sh are validating both oc and kubectl independently, even though resolve_api_server and resolve_bearer_token only use one tool at runtime. Update the startup checks to mirror that selection logic: determine the preferred CLI once (oc first, otherwise kubectl), then run only the matching login/API reachability validation for that chosen command. Keep the existing error handling and messages aligned with the selected tool so machines with both CLIs installed do not fail due to the unused one.src/hooks/useMessageSources.tsx (1)
7-14: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRestrict
isURLto explicit safe protocols.Checking only
protocol && hostallows any scheme with a host component (e.g.intent://,file://, custom app schemes) to pass validation and render as anisExternalclickable link. Sincedoc_urloriginates from LLM/tool output, consider allowlistinghttp:/https:explicitly for defense-in-depth.🔒 Proposed hardening
const isURL = (value: string): boolean => { try { const url = new URL(value); - return !!(url.protocol && url.host); + return (url.protocol === 'http:' || url.protocol === 'https:') && !!url.host; } catch { return false; } };🤖 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/hooks/useMessageSources.tsx` around lines 7 - 14, The isURL helper currently treats any URL with a protocol and host as valid, which can let unsafe schemes through and mark them as external links. Update isURL in useMessageSources.tsx to explicitly allow only http: and https: when validating the parsed URL, and keep the existing URL parsing/error handling so doc_url values from message sources are only rendered as clickable links for safe web protocols.unit-tests/resourceStatus.test.ts (1)
1-211: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLGTM! Good coverage across kinds. Consider adding cases for a 0/0-replica workload and a ready
LoadBalancerService to cover the edge cases flagged insrc/resourceStatus.ts.🤖 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 `@unit-tests/resourceStatus.test.ts` around lines 1 - 211, The resource status tests are missing coverage for two edge cases mentioned in src/resourceStatus.ts: a workload with zero desired replicas and a LoadBalancer Service that is already ready. Add focused cases in getResourceStatusSummary tests to assert the correct label and variant for a 0/0-replica state and for a ready LoadBalancer Service, using the existing getResourceStatusSummary and isInformativeStatusLabel helpers as anchors.src/crStatus.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCircular module dependency between
crStatus.tsandresourceStatus.ts.
crStatus.tsimports theStatusSummarytype from./resourceStatus, whileresourceStatus.tsimportsgetGenericResourceStatus/variantForPhase(values) from./crStatus(line 3 there). This is a real circular import between the two modules. If the build toolchain uses a single-file transpiler (e.g., Babel/isolatedModules) that can't elide type-only imports withoutimport type, this becomes a genuine circularrequire, which is fragile even though today's top-level code doesn't invoke functions during module init.♻️ Suggested fix
-import { StatusSummary } from './resourceStatus'; +import type { StatusSummary } from './resourceStatus';🤖 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/crStatus.ts` around lines 1 - 3, The import in crStatus.ts creates a circular dependency with resourceStatus.ts because StatusSummary is only used as a type while resourceStatus.ts already depends on values from crStatus.ts. Update the crStatus.ts import to be type-only and keep the crStatus/getGenericResourceStatus/variantForPhase split intact so the modules no longer form a runtime require cycle. Use the existing StatusSummary reference in crStatus.ts and the value imports in resourceStatus.ts to verify the dependency is broken without changing behavior.unit-tests/crStatus.test.ts (1)
1-64: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLGTM! Tests correctly exercise top-level phase precedence, standard
Readyconditions, and OLM-style phase conditions. Consider adding a case with bothAvailable: TrueandDegraded: Trueconditions to lock in the priority-order fix suggested insrc/crStatus.ts.🤖 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 `@unit-tests/crStatus.test.ts` around lines 1 - 64, Add a test in getGenericResourceStatus coverage for the mixed-condition case where both Available: True and Degraded: True are present, so the priority order change in getGenericResourceStatus is locked in. Extend crStatus.test.ts by asserting the expected label/variant for that scenario, using the existing getGenericResourceStatus helper and matching the same style as the current Ready and phase-condition tests.
🤖 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 `@scripts/mock-ols-server.mjs`:
- Around line 1-52: The mock server script uses Node globals like process and
Buffer, but the lint config for scripts/**/*.mjs does not declare a Node
environment, causing no-undef failures. Update the ESLint configuration to apply
a Node globals/env override for this script group so the mock server entrypoint
(mock-ols-server.mjs) is linted with process and Buffer recognized, without
changing the runtime code.
In `@src/components/GeneralPage.tsx`:
- Around line 174-183: The `messageSources` hook is unnecessarily tied to
`modelsLoaded` even though it only uses `entry.references` and does not depend
on `k8sModels`. In `GeneralPage`, split the shared `enabled` logic so
`useMessageSources` only waits on the AI entry/streaming/error checks, while
`useLinkedResourceMarkdown` keeps the `modelsLoaded` requirement. Use the
existing `messageSources` and `linkedResourceMarkdown` calls to decouple the
conditions cleanly.
In `@src/components/InlineLinkedResource.tsx`:
- Around line 33-48: The inline resource link in InlineLinkedResource should not
render as an active-looking link when buildResourceConsolePath produces an empty
value. Update the Button in InlineLinkedResource to reflect the non-navigable
state when path is falsy, for example by using an accessibility-disabled state
such as isAriaDisabled and/or preventing link styling, while keeping
navigate(path) only for valid paths. Ensure the behavior is handled in the same
component that computes path and renders ResourceIcon and the resource name.
In `@src/crStatus.ts`:
- Around line 133-167: The standard condition selection in
getStandardConditionStatus is masking Degraded=True by returning on
Ready/Available/Progressing first. Update the priority handling in
getStandardConditionStatus so Degraded and Failure are checked before the other
standard condition types, keeping the existing special-case variant logic
intact; this ensures a bad rollout isn’t reported as success when Available or
Progressing is also present.
In `@src/hooks/useConsoleNavigation.ts`:
- Around line 15-19: The fallback navigation in useConsoleNavigation’s navigate
handler is not fully guarded because navigateToConsolePath(path) can still throw
inside the outer catch. Update the try/catch around navigate(path) so the
fallback call has its own try/catch, or otherwise swallow errors from
navigateToConsolePath, and keep the behavior fail-closed without leaking
exceptions.
In `@src/linkedResources.ts`:
- Around line 17-49: The prioritization logic in prioritizeLinkedResources is
using only ref.name.toLowerCase() for both byName and used, which causes
distinct resources with the same name to collapse or be skipped. Update the
lookup and deduping to use each ResourceRef’s full identity (for example, name
plus kind/namespace or whatever unique fields exist on ResourceRef) when
building the Map and tracking used entries, so same-named but different
resources can all be preserved and only exact duplicates are reordered away.
In `@src/resourceRefs.ts`:
- Around line 249-261: The fallback in extractRefFromObject is too strict: it
only returns a ResourceRef when the item already has kind, so
extractRefsFromListDocument never gets to use the list document’s defaultKind.
Update extractRefFromObject (and its callers in extractRefsFromListDocument) so
items with metadata.name but no kind can still be converted using the parsed
list kind as the fallback, while preserving the existing namespace handling.
In `@src/resourceStatus.ts`:
- Around line 115-130: The `workloadReplicaStatus` helper is treating
intentionally scaled-to-0 workloads as an error state. Update the status logic
so that when `total === 0` it returns a neutral/non-error variant instead of
falling through to `danger`, while keeping the existing success and warning
behavior for nonzero replica counts. Use the `workloadReplicaStatus` symbol to
locate the conditional branches and adjust the `label`/`variant` mapping
accordingly.
- Around line 246-255: The Service status mapping is being shadowed by the
generic “ends with ready” rule, causing LoadBalancer services to display the
wrong state. Update the matching order in linkedResourceStatusDisplay so the
exact label check for “LoadBalancer ready” runs before the label.endsWith('
ready') branch, while keeping the label generated in resourceStatus.ts
unchanged.
In `@start-console.sh`:
- Around line 50-59: The token-minting path in start-console.sh is auto-creating
a ServiceAccount and cluster-admin ClusterRoleBinding without an explicit safety
check. Update the logic around the kubectl serviceaccount, clusterrolebinding,
and create token commands to require an opt-in guard (for example an env var
like MOCK_ALLOW_DEV_TOKEN_MINT=1) before provisioning anything, and otherwise
skip or fail with a clear message. Keep the existing flow for the dev-token
case, but make it only run when the explicit opt-in is present.
---
Nitpick comments:
In `@scripts/mock-ols-server.mjs`:
- Line 973: The file has broad formatter/lint drift and an unused destructured
value in the stream setup. Run the project formatter/eslint fix on
scripts/mock-ols-server.mjs to resolve the reported prettier/prettier issues,
remove the unused conversationId from the buildStreamBody(query) destructure,
and adjust the nearby lowercase comment so it matches the file’s style. Use the
buildStreamBody and query-related code around the stream handler to locate the
cleanup.
In `@src/components/InlineLinkedResource.tsx`:
- Around line 49-55: The key on LinkedResourceStatus is ineffective because this
conditional render is not part of a list, so React will ignore it. Remove the
key from InlineLinkedResource and, if remounting on resourceRef changes is
actually needed, move the identity change to the parent conditional or derive it
through a different prop-driven render path using
shouldShowLinkedResourceStatus, LinkedResourceStatus, and resourceRefKey as the
relevant symbols.
In `@src/components/LinkedResourceStatus.tsx`:
- Around line 21-46: The loading condition in LinkedResourceStatus is redundant
because the earlier null return already covers the case where loaded is true and
the resource does not match. Simplify the conditional after the loadError check
to only test loaded, keeping the existing behavior intact while making the logic
in LinkedResourceStatus clearer.
In `@src/crStatus.ts`:
- Around line 1-3: The import in crStatus.ts creates a circular dependency with
resourceStatus.ts because StatusSummary is only used as a type while
resourceStatus.ts already depends on values from crStatus.ts. Update the
crStatus.ts import to be type-only and keep the
crStatus/getGenericResourceStatus/variantForPhase split intact so the modules no
longer form a runtime require cycle. Use the existing StatusSummary reference in
crStatus.ts and the value imports in resourceStatus.ts to verify the dependency
is broken without changing behavior.
In `@src/hooks/useMessageSources.tsx`:
- Around line 7-14: The isURL helper currently treats any URL with a protocol
and host as valid, which can let unsafe schemes through and mark them as
external links. Update isURL in useMessageSources.tsx to explicitly allow only
http: and https: when validating the parsed URL, and keep the existing URL
parsing/error handling so doc_url values from message sources are only rendered
as clickable links for safe web protocols.
In `@src/linkedResources.ts`:
- Line 1: The import in linkedResources.ts needs Prettier-compliant formatting
because the current single-line multi-import is flagged by static analysis.
Reformat the existing import from pageContext to match the project’s multiline
import style, keeping the same symbols getModelKindName, K8sModelRef,
resolveRefModelKey, and resolveRefModelKeyOrKind but spreading them across lines
as needed.
- Around line 112-120: getLinkedResourceOverflow is doing the same
linkable-resource extraction twice by calling extractLinkableResourceRefs and
then extractLinkedResources, which re-runs the same parsing/prioritization work.
Update getLinkedResourceOverflow to compute the linked resources once and derive
both shown and total from that shared result, or otherwise reuse the single
extractLinkableResourceRefs output instead of invoking the pipeline again. Keep
the fix localized to getLinkedResourceOverflow and the linked resource helpers
it calls.
In `@src/linkedResourceWatch.ts`:
- Around line 42-59: `matchesResourceRef` only checks name and namespace, so it
can treat a stale object from the wrong kind as a match. Update
`matchesResourceRef` in `linkedResourceWatch` to also compare the resource kind
against `resourceRef` (or the derived model kind/apiVersion from the ref) before
returning true, while keeping the existing name/namespace checks intact.
In `@src/resourceRefs.ts`:
- Line 114: The K8S_NODE_NAME_RE regex is duplicated in resourceRefs and
resourceListParsing, so update the duplicated constant to use a single shared
definition instead of two copies. Export K8S_NODE_NAME_RE from
resourceListParsing and import it into resourceRefs, then remove the local
declaration so both ResourceRefs and ResourceListParsing rely on the same source
of truth.
In `@start-console.sh`:
- Around line 64-78: The preflight checks in start-console.sh are validating
both oc and kubectl independently, even though resolve_api_server and
resolve_bearer_token only use one tool at runtime. Update the startup checks to
mirror that selection logic: determine the preferred CLI once (oc first,
otherwise kubectl), then run only the matching login/API reachability validation
for that chosen command. Keep the existing error handling and messages aligned
with the selected tool so machines with both CLIs installed do not fail due to
the unused one.
In `@unit-tests/crStatus.test.ts`:
- Around line 1-64: Add a test in getGenericResourceStatus coverage for the
mixed-condition case where both Available: True and Degraded: True are present,
so the priority order change in getGenericResourceStatus is locked in. Extend
crStatus.test.ts by asserting the expected label/variant for that scenario,
using the existing getGenericResourceStatus helper and matching the same style
as the current Ready and phase-condition tests.
In `@unit-tests/linkedResourceWatch.test.ts`:
- Around line 33-53: Add a test in matchesResourceRef coverage to verify kind
mismatches are rejected: extend the linkedResourceWatch.test.ts suite with a
case where the resource name/namespace matches but the resource kind differs
from the ref, and assert matchesResourceRef returns false. Use the
matchesResourceRef helper and the existing Namespace-style cases as the
reference point so the new test clearly exercises the kind check added in
linkedResourceWatch.ts.
In `@unit-tests/resourceStatus.test.ts`:
- Around line 1-211: The resource status tests are missing coverage for two edge
cases mentioned in src/resourceStatus.ts: a workload with zero desired replicas
and a LoadBalancer Service that is already ready. Add focused cases in
getResourceStatusSummary tests to assert the correct label and variant for a
0/0-replica state and for a ready LoadBalancer Service, using the existing
getResourceStatusSummary and isInformativeStatusLabel helpers as anchors.
🪄 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: CHILL
Plan: Enterprise
Run ID: 798c07ca-e8ac-45a9-8285-de5d4e633ecf
📒 Files selected for processing (35)
README.mdlocales/en/plugin__lightspeed-console-plugin.jsonpackage.jsonscripts/mock-ols-server.mjssrc/components/GeneralPage.tsxsrc/components/InlineLinkedResource.tsxsrc/components/LinkedResourceStatus.tsxsrc/components/ResourceIcon.tsxsrc/components/linked-resources.csssrc/consoleNavigation.tssrc/crStatus.tssrc/hooks/useConsoleNavigation.tssrc/hooks/useLinkedResourceMarkdown.tsxsrc/hooks/useMessageSources.tsxsrc/linkedResourceStatusDisplay.tssrc/linkedResourceText.tssrc/linkedResourceWatch.tssrc/linkedResources.tssrc/pageContext.tssrc/resourceListParsing.tssrc/resourceRefs.tssrc/resourceStatus.tsstart-console.shunit-tests/consoleNavigation.test.tsunit-tests/crStatus.test.tsunit-tests/fixtures/k8sModels.tsunit-tests/linkedResourceStatusDisplay.test.tsunit-tests/linkedResourceText.test.tsunit-tests/linkedResourceWatch.test.tsunit-tests/linkedResources.test.tsunit-tests/pageContext.test.tsunit-tests/resourceListParsing.test.tsunit-tests/resourceRefs.test.tsunit-tests/resourceStatus.test.tswebpack.config.ts
Author
|
/retest-failed |
Author
Seems like konflux is failing to lint with |
Dev-only mock server, start-console-kind, and related README/start-console changes live on the mock-ols-server branch for a separate PR. Co-authored-by: Cursor <cursoragent@cursor.com>
jpinsonneau
force-pushed
the
linked-resources
branch
from
527212e to
a803f34
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds inline linked Kubernetes resources to Lightspeed AI responses. When the assistant mentions resources returned by MCP tools (pod lists,
resources_get, etc.), matching names in the response prose become clickable console links with kind icons and live status (SDKResourceIcon+StatusComponent).ResourceRefs; filters noise and non-linkable kinds (ConfigMaps, Secrets, namespaces).InlineLinkedResource(icon + name + status badge) inside chat messages; navigation works from the Lightspeed modal viauseConsoleNavigation.useK8sWatchResource; pod status uses container waiting reasons (e.g.CrashLoopBackOff) to match the console, not justphase.Mock OLS server —=> moved to Add mock OLS server #2182npm run start-mock-olsserves a minimal lightspeed-service-compatible API for local testing without a real OLS backend. Supports common list/get scenarios, reads live pod data fromocwhen available, and aligns SSE/MCP output with kubernetes-mcp-server. Configurable via env vars (MOCK_NAMESPACE,MOCK_PODS,MOCK_DIVERSE_POD_STATUSES, etc.).Also refactors
GeneralPagemessage rendering (useLinkedResourceMarkdown,useMessageSources) and documents console image pinning in the README.Test plan
npm run test:unitnpm run lint-fix && npm run buildnpm run start-mock-ols— Terminal 2:npm run start— Terminal 3:oc login && npm run start-consoleMOCK_DIVERSE_POD_STATUSES=1 npm run start-mock-olsto exercise varied statuses in tool output when no cluster is availableAssisted-by:
cursor + claude opus 4.6Summary by CodeRabbit