fix: parser correctness fixes (integer precision, nil safety, multi-variable lookup, type coercion, IfValue, XML duplicates)

[gOOvER]

Summary

This PR fixes six bugs in the configuration file parser (parser/parser.go, parser/helpers.go).

---

Changes

1. YAML: Preserve integer precision for large values (parser.go)

json.Unmarshal without UseNumber() converts integers larger than 2⁵³ to float64, causing precision loss. For example, a Snowflake ID like 150553673164927820 would be written to disk as 1.5055367316492782e+18.

The TOML parser already used UseNumber() correctly — this fix applies the same approach to the YAML parser: use json.NewDecoder with UseNumber() and a new normalizeYamlTypes() function that converts json.Number values to proper int64/float64 before marshalling back to YAML.

2. UnmarshalJSON: nil pointer dereference on incomplete egg config (parser.go)

If a configuration file entry from the panel was missing the file or parser keys, Wings would panic with a nil pointer dereference. Added explicit ok checks before dereferencing map values and return a descriptive error instead.

3. LookupConfigurationValue: only first placeholder resolved (helpers.go)

When a replace_with value contained multiple {{ config.x }} placeholders (e.g. {{ config.uuid }}-{{ config.port }}), only the first one was looked up and its value was substituted for all placeholders. Replaced the single ReplaceAllString call with a loop over FindAllString that resolves and substitutes each placeholder individually.

4. setValueWithSjson: string values silently coerced to integers (helpers.go)

The default case in setValueWithSjson called strconv.Atoi on string values and stored the integer result if successful. This caused string values like "25565" to be written as the integer 25565, changing the type of the value in config files unexpectedly. Removed the coercion — strings are now always stored as strings.

5. parseTextFile: IfValue condition never evaluated (parser.go)

The IfValue field on a replacement rule was never checked in the plain-text file parser. Lines were always replaced regardless of the current value. Added an explicit check: if IfValue is set, skip the replacement unless the current value of that line matches.

6. parseXmlFile: duplicate attributes on repeated runs (parser.go)

etree.CreateAttr does not overwrite an existing attribute — it appends a second one. On every Wings restart, XML config files would accumulate duplicate attributes. Added element.RemoveAttr(k) before element.CreateAttr(k, v) to ensure idempotent writes.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Multiple configuration placeholders within the same replacement string now process correctly
  • YAML numeric types now parse and normalize properly
  • Text file replacements now respect conditional value matching
  • Missing configuration keys no longer halt placeholder processing

• Refactor

  • Improved configuration file validation and error handling

[coderabbitai]

Warning

Rate limit exceeded

@gOOvER has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 54 minutes and 34 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ab0232ec-0845-4b9a-a2d9-1273372f7eb3

📥 Commits

Reviewing files that changed from the base of the PR and between cc436ae and 5344177.

📒 Files selected for processing (2)

• parser/helpers.go
• parser/parser.go

📝 Walkthrough

Walkthrough

Configuration file parsing enhancements add validation during unmarshalling, support multiple placeholder substitutions with graceful fallback, improve YAML numeric precision via UseNumber(), and add conditional filtering for text and XML replacements.

Changes

Configuration File Parsing and Replacement

Layer / File(s)	Summary
Configuration validation and unmarshalling parser/parser.go	ConfigurationFile.UnmarshalJSON validates required file and parser keys and conditionally unmarshals replace, logging a warning and defaulting to empty slice on unmarshal failure.
Configuration placeholder substitution parser/helpers.go	LookupConfigurationValue rewritten to iterate and replace multiple {{ config... }} placeholders one-by-one, leaving placeholders intact for missing keys while continuing to process others.
Value type coercion in replacements parser/helpers.go	setValueWithSjson default case stops attempting Atoi integer coercion and always assigns replacement strings as-is.
YAML numeric type normalization parser/parser.go	parseYamlFile uses json.Decoder with UseNumber() to preserve precision, then normalizeYamlTypes recursively converts json.Number to int64, float64, or string before re-marshalling.
Text file conditional replacement parser/parser.go	parseTextFile honors if_value: replacement only executes when line content after match prefix exactly equals IfValue.
XML attribute update handling parser/parser.go	XML attribute replacement now removes existing attributes before recreating them.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• pelican-dev/wings#161: Both PRs modify JSON replacement logic in parser/helpers.go around setValueWithSjson and placeholder handling; this PR builds on the sjson/gjson-based refactor.

Suggested reviewers

• parkervcp

Poem

🐰 Config placeholders multiply,
Type coercion cast away,
YAML numbers normalized with care,
Text and XML constraints now applied—
The parser hops to greater flair! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title concisely and specifically summarizes all six major bug fixes in the parser: integer precision, nil safety, multi-variable lookup, type coercion, IfValue, and XML duplicates.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR hardens the configuration file parser by adding defensive checks for required JSON keys, fixing YAML numeric handling for large integers, supporting multiple placeholder substitutions, and adding an if_value conditional for text-file replacements.

Changes:

• Safer JSON unmarshaling of ConfigurationFile with explicit errors for missing required keys and graceful handling of optional replace.
• YAML parsing now uses json.Decoder with UseNumber() plus a normalizeYamlTypes helper to preserve int64 precision (e.g., Snowflake IDs).
• LookupConfigurationValue now replaces all placeholders, and XML attribute updates remove the existing attribute before re-creating it.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
parser/parser.go	Defensive JSON key access, YAML numeric normalization, XML attribute replace fix, and if_value support for text replacements.
parser/helpers.go	Removed implicit string→int coercion in sjson setter and rewrote LookupConfigurationValue to handle multiple placeholders.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[parkervcp]

LGTM