fix: address 6 critical/minor code review findings (FAR-15)

1. Fix resources.* dotted-key config — UI fields now correctly read
2. Fix operator precedence bug in container status key (add parens)
3. Add missing RBAC checks to testEnvironment (jobs/list, secrets/*, pvc)
4. Add bail timer log message for debuggability
5. Make result-event detection robust to JSON whitespace variations
6. Remove namespace short-circuit so all checks run on first attempt

Co-Authored-By: Paperclip <noreply@paperclip.ing>

src/server/execute.ts

@@ -176,7 +176,7 @@ async function waitForPod(
     const containerStatuses = pod.status?.containerStatuses ?? [];
 
     // Log phase transitions
-    const statusKey = `${phase}:${initStatuses.map((s) => s.state?.waiting?.reason ?? s.state?.terminated?.reason ?? "ok").join(",")}:${containerStatuses.map((s) => s.state?.waiting?.reason ?? s.state?.running ? "running" : "waiting").join(",")}`;
+    const statusKey = `${phase}:${initStatuses.map((s) => s.state?.waiting?.reason ?? s.state?.terminated?.reason ?? "ok").join(",")}:${containerStatuses.map((s) => s.state?.waiting?.reason ?? (s.state?.running ? "running" : "waiting")).join(",")}`;
     if (statusKey !== lastStatus) {
       const details: string[] = [`phase=${phase}`];
       for (const init of initStatuses) {
@@ -301,7 +301,10 @@ export async function streamPodLogsOnce(
       if (stopSignal.stopped) {
         if (!writable.destroyed) writable.destroy();
         if (!bailTimer && bailResolve) {
-          bailTimer = setTimeout(bailResolve, LOG_STREAM_BAIL_TIMEOUT_MS);
+          bailTimer = setTimeout(() => {
+            onLog("stderr", "[paperclip] Log stream bail timer fired — forcing return\n").catch(() => {});
+            bailResolve!();
+          }, LOG_STREAM_BAIL_TIMEOUT_MS);
         }
       }
     }, 200);
@@ -958,7 +961,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
     // from the beginning of the log, giving us the full output.
     // We use a cheap string scan for the result-event guard (avoids a full JSON parse here;
     // the authoritative parse happens once below after all fallbacks complete).
-    const hasResultEvent = stdout.includes('"type":"result"');
+    const hasResultEvent = stdout.split("\n").some((l) => { try { return JSON.parse(l).type === "result"; } catch { return false; } });
     const needsOneShot = !stdout.trim() || (stdout.trim() && !hasResultEvent);
     if (needsOneShot) {
       if (!stdout.trim()) {

src/server/job-manifest.test.ts

@@ -438,10 +438,10 @@ describe("buildJobManifest", () => {
 
     it("uses configured resource overrides", () => {
       ctx.config = {
-        resources: {
-          requests: { cpu: "500m", memory: "1Gi" },
-          limits: { cpu: "2000m", memory: "4Gi" },
-        },
+        "resources.requests.cpu": "500m",
+        "resources.requests.memory": "1Gi",
+        "resources.limits.cpu": "2000m",
+        "resources.limits.memory": "4Gi",
       };
       const { job } = buildJobManifest({ ctx, selfPod });
       const resources = job.spec?.template?.spec?.containers[0]?.resources;

src/server/job-manifest.ts

@@ -345,7 +345,6 @@ export function buildJobManifest(input: JobBuildInput): JobBuildResult {
   const extraArgs = asStringArray(config.extraArgs);
   const timeoutSec = asNumber(config.timeoutSec, 0);
   const ttlSeconds = asNumber(config.ttlSecondsAfterFinished, 300);
-  const resources = parseObject(config.resources);
   const nodeSelector = parseKeyValueConfig(config.nodeSelector);
   const tolerations = Array.isArray(config.tolerations) ? config.tolerations : [];
   const extraLabels = parseKeyValueConfig(config.labels);
@@ -427,17 +426,16 @@ export function buildJobManifest(input: JobBuildInput): JobBuildResult {
   // Build env vars
   const envVars = buildEnvVars(ctx, selfPod, config);
 
-  // Resource defaults
-  const resourceRequests = parseObject(resources.requests);
-  const resourceLimits = parseObject(resources.limits);
+  // Resource defaults — UI stores dotted keys (e.g. "resources.requests.cpu")
+  // as flat config entries, so read them directly from config with the dotted key.
   const containerResources: k8s.V1ResourceRequirements = {
     requests: {
-      cpu: asString(resourceRequests.cpu, "1000m"),
-      memory: asString(resourceRequests.memory, "2Gi"),
+      cpu: asString(config["resources.requests.cpu"], "1000m"),
+      memory: asString(config["resources.requests.memory"], "2Gi"),
     },
     limits: {
-      cpu: asString(resourceLimits.cpu, "4000m"),
-      memory: asString(resourceLimits.memory, "8Gi"),
+      cpu: asString(config["resources.limits.cpu"], "4000m"),
+      memory: asString(config["resources.limits.memory"], "8Gi"),
     },
   };
 

src/server/test.ts

@@ -85,8 +85,13 @@ async function checkRbac(
     { resource: "jobs", group: "batch", verb: "create", code: "k8s_rbac_job_create", label: "create Jobs" },
     { resource: "jobs", group: "batch", verb: "delete", code: "k8s_rbac_job_delete", label: "delete Jobs" },
     { resource: "jobs", group: "batch", verb: "get", code: "k8s_rbac_job_get", label: "get Jobs" },
+    { resource: "jobs", group: "batch", verb: "list", code: "k8s_rbac_job_list", label: "list Jobs" },
     { resource: "pods", group: "", verb: "list", code: "k8s_rbac_pod_list", label: "list Pods" },
     { resource: "pods/log", group: "", verb: "get", code: "k8s_rbac_pod_log", label: "get Pod logs" },
+    { resource: "secrets", group: "", verb: "create", code: "k8s_rbac_secret_create", label: "create Secrets" },
+    { resource: "secrets", group: "", verb: "delete", code: "k8s_rbac_secret_delete", label: "delete Secrets" },
+    { resource: "secrets", group: "", verb: "get", code: "k8s_rbac_secret_get", label: "get Secrets" },
+    { resource: "persistentvolumeclaims", group: "", verb: "get", code: "k8s_rbac_pvc_get", label: "get PersistentVolumeClaims" },
   ];
 
   for (const check of rbacChecks) {
@@ -221,16 +226,18 @@ export async function testEnvironment(
 
   // 2. Target namespace exists
   const nsOk = await checkNamespace(namespace, selfPod.namespace, checks, kubeconfigPath);
-  if (!nsOk) {
-    return { adapterType: ctx.adapterType, status: summarizeStatus(checks), checks, testedAt: new Date().toISOString() };
-  }
 
-  // 3-5. Run remaining checks in parallel
-  await Promise.all([
-    checkRbac(namespace, checks, kubeconfigPath),
-    checkSecret(namespace, secretRef, checks, kubeconfigPath),
-    checkPvc(selfPod, checks, kubeconfigPath),
-  ]);
+  // 3-5. Run remaining checks even if namespace check failed so operators see
+  // all issues at once instead of fixing them one at a time.
+  if (nsOk) {
+    await Promise.all([
+      checkRbac(namespace, checks, kubeconfigPath),
+      checkSecret(namespace, secretRef, checks, kubeconfigPath),
+      checkPvc(selfPod, checks, kubeconfigPath),
+    ]);
+  } else {
+    await checkRbac(namespace, checks, kubeconfigPath);
+  }
 
   return {
     adapterType: ctx.adapterType,