[bborbe]

Adds the Helm values for cluster-scoped task routing (agent-task-controller v0.14.0, agent-task-executor PR #82):

• cluster renders CLUSTER on every executor and controller.
• publishUntagged renders PUBLISH_UNTAGGED on controllers. The check is hasKey/kindIs bool, so false renders.
• executor.consumerGroup and executors[].consumerGroup render CONSUMER_GROUP.

With default values none of the three is rendered. The chart goes from 0.7.0 to 0.8.0, so the default render changes only the helm.sh/chart label. Checked against the nuke values-dev.yaml with the new values set: CLUSTER on all 6 workloads, PUBLISH_UNTAGGED on the 3 controllers, CONSUMER_GROUP on the 3 executors. Implemented by dark-factory prompt 239. A follow-up commit reverts three unrelated spec-status flips from the run.

[ben-s-pull-request-reviewer]

The ast-grep runner is not available in this environment. I'll proceed with my manual review of the changed files. Based on my analysis:

Changes verified:

1. helm/templates/controller-statefulset.yaml:102-109 — publishUntagged uses hasKey + kindIs "bool" correctly: when publishUntagged: false is set, hasKey returns true, kindIs "bool" confirms type, renders "false". When key is absent, nothing renders.

2. helm/templates/executor-deployment.yaml:107-113 (singular executor) — CLUSTER uses {{- with .Values.cluster }} and CONSUMER_GROUP uses {{- with .Values.executor.consumerGroup }}. The {{- variant trims whitespace, so empty values produce no env entry and no blank line.

3. helm/templates/executor-deployment.yaml:260-267 (executors[] range) — CONSUMER_GROUP uses ($e.consumerGroup | default $.Values.executor.consumerGroup) — per-entry wins, falls back to shared value, empty renders nothing via {{- with }}.

4. helm/Chart.yaml — version: 0.7.0 → 0.8.0 (minor, backwards-compatible), appVersion: "0.3.1" unchanged.

5. CHANGELOG.md — ## Unreleased section added with correct bullet, ## v0.106.0 preserved below.

6. No test files modified — this is a Helm chart with no existing test suite; the prompt executed manual helm template render checks.

Traceability: Mechanical funnel unavailable (runner script not in sandbox), selector guide not available, so this is a manual review only.

{
  "verdict": "approve",
  "summary": "Helm chart adds three optional cluster-routing environment variables (CLUSTER, PUBLISH_UNTAGGED, CONSUMER_GROUP) with correct conditional rendering. publishUntagged uses hasKey+kindIs bool to distinguish false from unset. All three are absent by default so default renders unchanged (only helm.sh/chart label differs via version bump 0.7.0→0.8.0). No test suite exists for the chart; manual render checks were run per the prompt. No issues found.",
  "comments": [],
  "concerns_addressed": [
    {
      "concern": "correctness: publishUntagged uses hasKey + kindIs bool to distinguish false from unset",
      "disposition": "addressed",
      "detail": "controller-statefulset.yaml:106-109 uses hasKey $.Values 'publishUntagged' AND kindIs 'bool' — false renders '\"false\"', absent key renders nothing. Verified the template logic."
    },
    {
      "concern": "correctness: CLUSTER and CONSUMER_GROUP use {{- with ...}} guards; empty values produce no env entry",
      "disposition": "addressed",
      "detail": "executor-deployment.yaml:107-113 (singular) and :260-267 (executors[] range) use {{- with }} which trims whitespace. Empty string produces no output. Verified template syntax."
    },
    {
      "concern": "correctness: executors[] range uses ($e.consumerGroup | default $.Values.executor.consumerGroup); verify fallback chain",
      "disposition": "addressed",
      "detail": "executor-deployment.yaml:264 uses correct default chain: per-entry value wins, falls back to executor.consumerGroup, empty renders nothing via {{- with }}."
    },
    {
      "concern": "tests: No test files modified; render checks run manually as part of prompt execution",
      "disposition": "addressed",
      "detail": "Helm chart has no existing test suite. Prompt executed manual helm template render checks (verified in completed prompt file). No automated test gap."
    },
    {
      "concern": "correctness: Chart version bumped 0.7.0 -> 0.8.0 (minor); appVersion unchanged",
      "disposition": "addressed",
      "detail": "Chart.yaml: version 0.7.0→0.8.0, appVersion '0.3.1' unchanged. Only helm.sh/chart label differs in default render."
    }
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

I've now verified all the changes. Let me compile the review findings.

Plan Concern Review:

1. controller-statefulset.yaml:106 — The {{- if ... }} idiom: the {{- prefix strips preceding whitespace including the newline after the previous - name: AUTO_INJECT_TASK_IDENTIFIER line. When the condition is false, the block renders nothing. When true, it renders the - name: PUBLISH_UNTAGGED line with proper indentation. No blank lines or malformed YAML. Verified correct.

2. executor-deployment.yaml:260-267 — The CLUSTER/CONSUMER_GROUP asymmetry is intentional and documented in values.yaml comments. CLUSTER uses root $.Values.cluster only (cluster-wide identity, no per-entry override needed). CONSUMER_GROUP uses per-entry fallback chain ($e.consumerGroup | default $.Values.executor.consumerGroup) which matches the established pattern for all other per-executor fields (e.g. logLevel, topicPrefix). Verified intentional.

3. values.yaml:60 — publishUntagged is commented out with no default. The template uses hasKey (checks key existence) not get with a default, so Helm produces no warning about a missing default — the key genuinely does not exist in values.yaml. This is the intended design (to distinguish "not set" from "set to false"). Verified correct.

4. No test files — These are Helm templates; the project has no Helm rendering tests. Manual render verification is documented in prompts/completed/239-helm-cluster-routing-env.md. Acceptable.

5. --set publishUntagged=false coverage — The prompt documents testing publishUntagged: true and publishUntagged: false explicitly. Verified.

No other issues found in the diff.

---

Verdict:

{
  "verdict": "approve",
  "summary": "Helm feature adds three optional cluster-routing environment variables (CLUSTER, PUBLISH_UNTAGGED, CONSUMER_GROUP) with proper conditional rendering, intentional design asymmetries confirmed correct, and backward-compatible defaults.",
  "comments": [],
  "concerns_addressed": [
    {
      "concern": "correctness: controller-statefulset.yaml PUBLISH_UNTAGGED whitespace-stripping idiom",
      "disposition": "not-an-issue",
      "detail": "Verified controller-statefulset.yaml:106 — {{- if }} prefix strips preceding whitespace; absent key produces no output, not blank/malformed YAML."
    },
    {
      "concern": "correctness: executor-deployment.yaml CLUSTER/CONSUMER_GROUP asymmetry",
      "disposition": "not-an-issue",
      "detail": "Verified executor-deployment.yaml:260-267 — asymmetry is intentional: CLUSTER is root-level only (cluster identity, no per-entry override), CONSUMER_GROUP uses the standard per-entry-with-fallback pattern matching all other executor fields."
    },
    {
      "concern": "correctness: values.yaml publishUntagged Helm lint warning for optional bool",
      "disposition": "not-an-issue",
      "detail": "Verified values.yaml:60 — publishUntagged is commented out with no default. Template uses hasKey (existence check), not get+default, so Helm produces no missing-default warning."
    },
    {
      "concern": "tests: no test files changed for helm templates",
      "disposition": "not-an-issue",
      "detail": "No Helm rendering tests exist in this repo. Manual render verification documented in prompts/completed/239-helm-cluster-routing-env.md."
    },
    {
      "concern": "tests: publishUntagged true/false both tested",
      "disposition": "not-an-issue",
      "detail": "Completed prompt 239 documents both publishUntagged: true and publishUntagged: false render checks."
    }
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

All concerns verified against the actual diff. Here's my analysis:

Correctness:

• publishUntagged (controller-statefulset.yaml:106-109) uses hasKey + kindIs "bool" — not truthiness. false renders "false", absent key renders nothing. ✅
• CLUSTER uses {{- with $.Values.cluster }} — empty string is falsy for with, renders nothing. ✅
• CONSUMER_GROUP uses {{- with ($e.consumerGroup | default $.Values.executor.consumerGroup) }} — per-entry wins, empty fallback renders nothing. ✅

Security:

• All three env values use | quote filter (controller lines 103/108, executor lines 108/111/261/264/265) — properly escapes for YAML string context. ✅

Other:

• Chart version: 0.7.0 → 0.8.0 (helm/Chart.yaml:6) ✅
• README documented correctly ✅
• CHANGELOG has ## Unreleased section with correct bullet ✅
• helm not available in this environment to re-run template render checks, but prompt verification was performed and documented in the completed prompt file.

{
  "verdict": "approve",
  "summary": "Helm chart adds three optional cluster-routing environment variables (CLUSTER, PUBLISH_UNTAGGED, CONSUMER_GROUP) with correct template logic — publishUntagged properly uses hasKey+kindIs bool check so false is honoured, empty values render nothing, and all values use the quote filter. Chart version bumped 0.7.0→0.8.0, README and CHANGELOG updated. Default render is unchanged apart from the chart label.",
  "comments": [],
  "concerns_addressed": [
    {
      "concern": "correctness: publishUntagged uses hasKey+kindIs bool check — false must render \"false\", not be skipped",
      "disposition": "addressed",
      "detail": "controller-statefulset.yaml:106-109 uses '{{- if and (hasKey $.Values \"publishUntagged\") (kindIs \"bool\" $.Values.publishUntagged) }}' — false renders \"false\", absent key renders nothing."
    },
    {
      "concern": "correctness: CLUSTER env uses {{- with $.Values.cluster }} — empty string renders nothing",
      "disposition": "addressed",
      "detail": "controller-statefulset.yaml:102-105 and executor-deployment.yaml:107-110/260-263 use '{{- with }}' which skips the block when the value is empty."
    },
    {
      "concern": "correctness: CONSUMER_GROUP uses per-entry override with default fallback",
      "disposition": "addressed",
      "detail": "executor-deployment.yaml:111-114 (singular) and :264-267 (list) use '{{- with ($e.consumerGroup | default $.Values.executor.consumerGroup) }}' — correct precedence."
    },
    {
      "concern": "security: CLUSTER and PUBLISH_UNTAGGED env values use | quote filter",
      "disposition": "addressed",
      "detail": "All new env values use '| quote' filter preventing Helm template injection."
    },
    {
      "concern": "security: CLUSTER and CONSUMER_GROUP env values use | quote filter",
      "disposition": "addressed",
      "detail": "All new env values use '| quote' filter preventing Helm template injection."
    },
    {
      "concern": "correctness: Chart version bumped 0.7.0→0.8.0 — only helm.sh/chart label changes on default render",
      "disposition": "addressed",
      "detail": "helm/Chart.yaml line 6 shows version: 0.8.0. This is the only permitted diff for default renders per the spec."
    },
    {
      "concern": "tests: No existing Helm tests found; render checks were performed manually",
      "disposition": "addressed",
      "detail": "Render checks documented in the completed prompt file (prompts/completed/239-helm-cluster-routing-env.md). helm not available in this review environment to re-run."
    }
  ]
}