Skip to content

fix(chart): use hasKey for boolean enabled to avoid Go template default trap#639

Merged
thepagent merged 2 commits intomainfrom
fix/chart-cron-enabled-haskey
May 1, 2026
Merged

fix(chart): use hasKey for boolean enabled to avoid Go template default trap#639
thepagent merged 2 commits intomainfrom
fix/chart-cron-enabled-haskey

Conversation

@chaodu-agent
Copy link
Copy Markdown
Collaborator

What problem does this solve?

PR #638 introduced enabled fields for cronjobs and the existing reactions.enabled already had the same pattern:

enabled = {{ .enabled | default true }}

In Go templates, default treats false as a zero value, so {{ false | default true }} returns true. This means users cannot disable cronjobs or reactions by setting enabled: false — the value is silently overridden to true.

Discovered during four-monk review of PR #638 by 覺渡法師 (Gemini).

Changes

File What
charts/openab/templates/configmap.yaml Replace {{ .enabled | default true }} with {{ if hasKey . "enabled" }}{{ .enabled }}{{ else }}true{{ end }} for both reactions.enabled and cron.jobs[].enabled
charts/openab/values.yaml Clarify enabled comment: false = skip scheduling (job config is still validated at startup)

Testing

# cronjob enabled=false → renders false ✅
helm template test charts/openab \
  --set agents.kiro.cronjobs[0].schedule="0 9 * * 1-5" \
  --set-string agents.kiro.cronjobs[0].channel=123456789 \
  --set agents.kiro.cronjobs[0].message=hello \
  --set agents.kiro.cronjobs[0].enabled=false

# cronjob enabled not set → renders true ✅
helm template test charts/openab \
  --set agents.kiro.cronjobs[0].schedule="0 9 * * 1-5" \
  --set-string agents.kiro.cronjobs[0].channel=123456789 \
  --set agents.kiro.cronjobs[0].message=hello

# reactions enabled=false → renders false ✅
helm template test charts/openab \
  --set agents.kiro.reactions.enabled=false

Related

@chaodu-agent chaodu-agent requested a review from thepagent as a code owner April 30, 2026 05:50
@github-actions github-actions Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label Apr 30, 2026
@github-actions
Copy link
Copy Markdown

⚠️ This PR is missing a Discord Discussion URL in the body.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

This PR will be automatically closed in 3 days if the link is not added.

@github-actions github-actions Bot added the pending-screening PR awaiting automated screening label Apr 30, 2026
@thepagent thepagent removed closing-soon PR missing Discord Discussion URL — will auto-close in 3 days pending-screening PR awaiting automated screening labels May 1, 2026
@github-actions github-actions Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label May 1, 2026
超渡法師 and others added 2 commits May 1, 2026 04:53
…e default trap

Go template's `default` treats `false` as a zero value, so
`{{ false | default true }}` returns `true`. This made it impossible
to disable cronjobs or reactions via `enabled: false`.

Fix both occurrences:
- reactions.enabled (L104)
- cron.jobs[].enabled (L142)

Also clarify the enabled field comment in values.yaml to reflect that
disabled jobs are still validated at startup.

Discovered during four-monk review of PR #638.
Co-authored-by: 覺渡法師 <gemini@openab.dev>
Extend the hasKey pattern to the remaining '| default false' boolean
fields for consistency. While the current default-false behavior is
correct today, using hasKey prevents a latent trap if defaults are
ever changed to true.
@chaodu-agent chaodu-agent force-pushed the fix/chart-cron-enabled-haskey branch from 36e0b4b to 1f391a5 Compare May 1, 2026 04:54
@thepagent thepagent merged commit 15d3f10 into main May 1, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 3 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants