Skip to content

fix(chart): apply hasKey to remaining boolean defaults for consistency#672

Closed
chaodu-agent wants to merge 1 commit intomainfrom
fix/chart-boolean-default-haskey-consistency
Closed

fix(chart): apply hasKey to remaining boolean defaults for consistency#672
chaodu-agent wants to merge 1 commit intomainfrom
fix/chart-boolean-default-haskey-consistency

Conversation

@chaodu-agent
Copy link
Copy Markdown
Collaborator

What problem does this solve?

PR #639 fixed the | default true trap for enabled fields, but two | default false fields were left unchanged:

  • remove_after_reply = {{ ($cfg.reactions).removeAfterReply | default false }}
  • usercron_enabled = {{ (($cfg.cron).usercronEnabled) | default false }}

While {{ false | default false }} happens to produce the correct result today, it creates a latent trap — if anyone changes the default to true in the future, the same false-is-zero-value bug would resurface silently.

Flagged during triage review of PR #639.

Changes

File What
charts/openab/templates/configmap.yaml Replace `

Testing

# removeAfterReply=true → renders true ✅
helm template test charts/openab --set agents.kiro.reactions.removeAfterReply=true | grep remove_after_reply

# removeAfterReply=false → renders false ✅
helm template test charts/openab --set agents.kiro.reactions.removeAfterReply=false | grep remove_after_reply

# removeAfterReply not set → renders false (default) ✅
helm template test charts/openab | grep remove_after_reply

# usercronEnabled=true → renders true ✅
helm template test charts/openab --set agents.kiro.cron.usercronEnabled=true | grep usercron_enabled

# usercronEnabled=false → renders false ✅
helm template test charts/openab --set agents.kiro.cron.usercronEnabled=false --set agents.kiro.cron.usercronPath=/tmp/cron | grep usercron_enabled

# usercronEnabled not set → renders false (default) ✅
helm template test charts/openab --set agents.kiro.cron.usercronPath=/tmp/cron | grep usercron_enabled

Related

removeAfterReply and usercronEnabled used '| default false' which
happens to work today but creates a latent trap — if anyone changes
the default to true in the future, the same false-is-zero-value bug
from PR #639 would resurface.

Apply the hasKey pattern to both fields for consistency with the
enabled fields already fixed in #639.
@chaodu-agent chaodu-agent requested a review from thepagent as a code owner May 1, 2026 04:51
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

⚠️ 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 closing-soon PR missing Discord Discussion URL — will auto-close in 3 days pending-screening PR awaiting automated screening labels May 1, 2026
@chaodu-agent
Copy link
Copy Markdown
Collaborator Author

Closing — will fold this fix into PR #639 instead.

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 pending-screening PR awaiting automated screening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant