Skip to content

fix(playground): hoist enterQueryEdit out of its value-receiver returns - #89

Merged
sergiught merged 1 commit into
mainfrom
fix/query-edit-eval-order
Jul 31, 2026
Merged

fix(playground): hoist enterQueryEdit out of its value-receiver returns#89
sergiught merged 1 commit into
mainfrom
fix/query-edit-eval-order

Conversation

@sergiught

Copy link
Copy Markdown
Owner

📝 Summary

enterQueryEdit has a pointer receiver and mutates the model — it sets editing, and sets status on the no-store path. Four call sites returned it inline from value-receiver methods:

return m, m.enterQueryEdit()

Go only orders function calls left-to-right relative to each other; the plain read of m relative to that call is unspecified. So the model bubbletea receives back may be the pre-mutation copy — the query form would fail to open, or the "select a store first" status would be silently dropped.

It works under the current compiler, which is why it went unnoticed. This is the same hazard the confirm modal was already hoisted for (the codebase calls it TUI-F8, see the Hoisted: comments in section_keys.go), so this brings the last four sites in line and gives them the same refactor guards.

Sites fixed: section_keys.go i/enter, tab, shift+tab, and update.go's onEnterSection secQuery branch.

🏷 Type of change

  • 🐛 Bug fix (non-breaking)
  • ✨ Feature (non-breaking)
  • 💥 Breaking change (users must update)
  • ♻️ Refactor (no functional change)
  • 📚 Docs only
  • 🛠 Build / CI / tooling

🔗 Related issues

None. Found during review of #86 / #87; deliberately split out because it is pre-existing and unrelated to either.

✅ How to verify

make check   # fmt, vet, lint, test

The new guards in query_edit_eval_order_test.go assert the mutation is observable on the returned model. To confirm they actually bite, make one site return the pre-mutation copy:

case "i", "enter":
    pre := m
    cmd := m.enterQueryEdit()
    return pre, cmd

TestQueryEditKeyAppliesMutation/i, /enter, and TestQueryEditWithoutStoreAppliesStatus all fail; revert and they pass.

📋 Checklist

  • 🧪 Tests added or updated where it makes sense
  • 📖 Docs updated (README / command --help examples) if behavior changed — no user-visible behavior change
  • 🏷 PR title follows Conventional Commits
  • 🟢 make check passes locally

Notes for review

No behavior change under the current compiler — this closes an unspecified-behavior hole rather than fixing an observed misbehavior. The guards are the durable part: they make a future re-inlining fail loudly instead of silently.

Branches from main and touches neither #86's nor #87's areas.

enterQueryEdit has a pointer receiver and mutates the model (editing, and
status on the no-store path), but four call sites returned it inline as
`return m, m.enterQueryEdit()` from value-receiver methods. Go only
orders function calls left-to-right; the plain read of m relative to that
call is unspecified, so the model bubbletea receives back could be the
pre-mutation copy.

It worked under the current compiler, which is why it went unnoticed.
This is the same class the confirm modal was already hoisted for, and it
gains the same refactor guards.
@sergiught
sergiught force-pushed the fix/query-edit-eval-order branch from 7fae23a to 2eaedfe Compare July 31, 2026 12:37
@sergiught
sergiught merged commit 1026c95 into main Jul 31, 2026
7 checks passed
@sergiught
sergiught deleted the fix/query-edit-eval-order branch July 31, 2026 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant