Skip to content

Fix stale certificate issued state #575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
heisbrot merged 2 commits into from
Mar 9, 2026
Merged

Fix stale certificate issued state #575

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a27bb6
Fix stale certificate issued state
heisbrot Mar 4, 2026
3f85bce
fix coderabbit
heisbrot Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 15 additions & 20 deletions src/modules/reverse-proxy/table/ReverseProxyStatusCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
import useFetchApi from "@utils/api";
import Badge from "@components/Badge";
import { Loader2 } from "lucide-react";
import { useMemo } from "react";
import { useRef } from "react";

type Props = {
serviceId: string;
Expand All @@ -21,38 +21,33 @@ export default function ReverseProxyStatusCell({
meta,
enabled,
}: Readonly<Props>) {
const status = meta?.status;
const certificateIssued = !!meta?.certificate_issued_at;
const dataRef = useRef<ReverseProxy | undefined>(undefined);

const isSettingUp =
enabled &&
status !== undefined &&
status !== ReverseProxyStatus.ACTIVE &&
!certificateIssued;
const isActive =
meta?.status === ReverseProxyStatus.ACTIVE ||
dataRef.current?.meta?.status === ReverseProxyStatus.ACTIVE;

const certificateIssued =
!!meta?.certificate_issued_at ||
!!dataRef.current?.meta?.certificate_issued_at;

const shouldPoll = !!enabled && !(isActive && certificateIssued);
Comment on lines +26 to +34
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use one coherent metadata source for readiness checks

isActive and certificateIssued are currently computed with separate ORs across meta and dataRef.current. That can produce a synthetic (ACTIVE && issued) state from different snapshots, which may stop polling and hide the cell too early.

🔧 Proposed fix 
-  const isActive =
-    meta?.status === ReverseProxyStatus.ACTIVE ||
-    dataRef.current?.meta?.status === ReverseProxyStatus.ACTIVE;
-
-  const certificateIssued =
-    !!meta?.certificate_issued_at ||
-    !!dataRef.current?.meta?.certificate_issued_at;
+  const effectiveMeta = dataRef.current?.meta ?? meta;
+  const isActive = effectiveMeta?.status === ReverseProxyStatus.ACTIVE;
+  const certificateIssued = !!effectiveMeta?.certificate_issued_at;

Also applies to: 46-46

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/modules/reverse-proxy/table/ReverseProxyStatusCell.tsx` around lines 26 -
34, The readiness checks use mixed snapshots (meta and dataRef.current?.meta)
which can combine to a false-positive ready state; fix by selecting one coherent
metadata object (e.g., const currentMeta = meta ?? dataRef.current?.meta) and
then compute isActive and certificateIssued from that single currentMeta (use
ReverseProxyStatus.ACTIVE for isActive and check certificate_issued_at for
certificateIssued), and update shouldPoll accordingly; apply the same change to
the analogous check around the other occurrence referenced (the second
computation at line ~46).


const { data } = useFetchApi<ReverseProxy>(
`/reverse-proxies/services/${serviceId}`,
true,
false,
isSettingUp,
shouldPoll,
{ refreshInterval: POLL_INTERVAL_MS },
);

const currentStatus = data?.meta?.status ?? status;

const currentCertificateIssued = useMemo(() => {
if (data && data?.meta) return !!data?.meta?.certificate_issued_at;
return certificateIssued;
}, [data]);
dataRef.current = data;
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Preserve last known data instead of overwriting ref with undefined

Unconditionally assigning dataRef.current = data can clear cached status when fetching is disabled, which can reintroduce stale UI state on later renders.

🔧 Proposed fix 
-  dataRef.current = data;
+  if (data !== undefined) {
+    dataRef.current = data;
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dataRef.current = data;
if (data !== undefined) {
dataRef.current = data;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/modules/reverse-proxy/table/ReverseProxyStatusCell.tsx` at line 44, The
code unconditionally assigns dataRef.current = data which can overwrite the
last-known status with undefined when fetching is disabled; change the update to
only overwrite the ref when a real value is present (e.g., check data !==
undefined/null) so dataRef.current retains the previous cached status when data
is undefined; update the assignment used in ReverseProxyStatusCell (the dataRef
and data variables in that component) to guard the write accordingly.


if (
!enabled ||
(currentStatus === ReverseProxyStatus.ACTIVE && currentCertificateIssued)
) {
if (!enabled || (isActive && certificateIssued)) {
return null;
}

if (!currentCertificateIssued) {
if (!certificateIssued) {
return (
<div className={"flex"}>
<Badge variant={"yellow"}>
Expand Down