feat(settings): seed settings from OPENC3_SETTING_* env vars - #3699
Open
mcosgriff wants to merge 35 commits into
Open
feat(settings): seed settings from OPENC3_SETTING_* env vars#3699mcosgriff wants to merge 35 commits into
mcosgriff wants to merge 35 commits into
Conversation
- Add `openc3cli initsettings`, run by init.sh, which seeds settings from OPENC3_SETTING_<NAME> variables so time zone, time format and other Admin Console values can be configured at deploy time - Only write a setting that doesn't already exist so Admin Console edits survive a restart; OPENC3_SETTINGS_OVERWRITE makes the environment authoritative - Reject an unrecognized setting name (suggesting the near match) rather than writing a dead Redis key a typo would leave behind; OPENC3_SETTINGS_ALLOW_UNKNOWN opts out for settings added by a newer tool - Coerce values as JSON so boolean settings like ai_chat aren't stored as the string "false", which is truthy in the frontend - Add ConfigParser.handle_true_false_strict, which accepts 1/TRUE/0/FALSE and raises otherwise, so OPENC3_SETTINGS_OVERWRITE=0 means off rather than the on implied by the OPENC3_NO_* presence flags - Apply the same seed guard and JSON coercion to LocalMode.sync_settings, which previously overwrote Redis on every localinit and stored files verbatim - Document the variables in compose.override.yaml; compose.yaml is unchanged because an override's environment block adds variables it doesn't list
…o-overwrite-settings-in-init
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3699 +/- ##
==========================================
+ Coverage 79.18% 79.24% +0.06%
==========================================
Files 679 895 +216
Lines 43482 67231 +23749
Branches 2600 2556 -44
==========================================
+ Hits 34431 53279 +18848
- Misses 8387 13287 +4900
- Partials 664 665 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Coerce each value by its setting's declared type rather than by attempting JSON.parse: booleans reach Redis as real booleans (the string "false" is truthy in the frontend) while every other setting keeps the text given - Fix astro, classification_banner and context_tag being stored as parsed objects. Those components JSON.parse the stored value, so a Hash makes them throw. Type-driven coercion also stops a subtitle of "2024" becoming the number 2024 - Enumerate all 14 Admin Console settings in KNOWN_SETTINGS with their types and allowed values. The table previously held 4, so OPENC3_SETTING_THEME failed as an unknown name - Add SettingModel.describe_settings, used by `cli initsettings --help`, so the documented list is generated from the table and can't drift - Document how to add a setting above KNOWN_SETTINGS, including how to tell a JSON-text setting from an object one - Point compose.override.yaml at `cli initsettings --help` instead of repeating the list, and show the boolean, free text and JSON text forms
- Add .claude/commands/commit-message.md following Conventional Commits v1.0.0, citing the numbered rules and separating them from git convention like the 72 character first line, so the same file works in any repo - Scope the command to git diff --cached only, so committed-but-unpushed work, unstaged changes and untracked files stay out of the generated message - Point CLAUDE.md at the command instead of restating the format, dropping the duplicated Angular rules and the 2-4 line body cap that contradicted it - Remove the 🤖 Generated with Claude Code footer, which is not a valid token: value footer under rule 9 - Delete generate-commit-message.md, fully superseded by the new command
- Compare KNOWN_SETTINGS against the setting names and types extracted from the Vue components so the table cannot silently fall behind. The previous "seeds every setting" test looped over the table itself, so dropping a row kept it green - Assert the declared type matches what each component passes to saveSetting, catching a JSON text setting declared as a boolean. Both directions verified by mutation - Require the heuristics to resolve every setting rather than only agreeing on what they resolved, so a component shape infer_type does not handle fails instead of quietly shrinking the check - Fix tool_config_model_spec "deletes", which depended on ambient container state: delete_tool_config returns nil early unless OPENC3_LOCAL_MODE is set and the local mode path exists, so the test only passed in Docker. Set both up against a temp dir and restore them after - Assert the config file is actually removed, not just that rm_f echoed the path it was asked to remove, and cover the early-return branch with local mode off
- Wrap body lines at 72 characters, cap the body at 5 bullets, and allow one sentence each so git log stays readable - Drop the "note a non-obvious consequence" guidance, which invited a second sentence on nearly every bullet
- Read OPENC3_LOCAL_MODE via ENV.fetch with an explicit nil default, since nil records that the variable was unset - Add else clauses to the two case statements in setting_model_spec, making the existing implicit nil fall-through explicit
- Add initsettings to the cli command list and to the branch offering only --help, since it takes no positional arguments - Add reingest, which was missing entirely, with its own branch prompting JOB_ID then SCOPE as both are required
- Indent every commented service block so uncommenting yields valid YAML, since a service at column 0 fails with "additional properties 'openc3-cosmos-init' not allowed" - Document that services: must be uncommented too, quoting that error so the message maps to the cause - Stop the log to stderr section redeclaring openc3-operator, which collided with the block above and discarded its ports and volumes
- Record each value initsettings writes to a companion Redis hash, so a later init can tell an untouched setting from one an operator changed - Apply a changed env value while the setting still matches what was seeded, which makes editing the override and restarting work - Leave any setting whose value differs from the seeded one, including settings with no record from before this tracking existed - Name the destroyed value when OPENC3_SETTINGS_OVERWRITE clobbers an edit, which previously logged the same line as a first-time seed
- Add --dry-run, which reports the action planned for each setting, writes nothing, and exits non-zero if any would fail - Skip an invalid setting instead of aborting init, which crash looped COSMOS under restart: on-failure over a cosmetic value - Run cli initsettings in the init container, the only one that receives OPENC3_SETTING_* variables - Extract plan_setting so a dry run cannot report one thing and the real run do another - Fall back to name and value checks when Redis is unreachable, so the check is usable before starting COSMOS
…o-overwrite-settings-in-init
- Add OpenC3::RubygemsUrl.validate to reject non-http(s) or malformed rubygems_url values, mirroring the existing PypiUrl guard - Reuse RubygemsUrl::DEFAULT in scope_model seed_database so the rubygems.org default is defined in one place - Cover the untested PluginModel.resolve_pypi_url and build_pypi_args, including the --trusted-host branch - Add PythonPackageModel.install specs for a nil setting, an injection payload, and a non-http scheme - Add an explicit default case to the openc3.sh cli CLI_SERVICE switch Co-Authored-By: Claude [email protected]
Contributor
Author
services:
openc3-cosmos-init:
environment:
- OPENC3_SETTING_TIME_ZONEs=local
- OPENC3_SETTING_TIME_FORMAT=ampm
- OPENC3_SETTING_PYPI_URL=https://mirrors.sustech.edu.cn/pypi/web/
- OPENC3_SETTING_THEME=cosmosDarkSlate
|
- Re-comment the services: and openc3-cosmos-init: keys that were left active, so the file is inert example config again - Replace the scratch OPENC3_SETTING_TIME_ZONEs=Mars line with commented TIME_ZONE, TIME_FORMAT, PYPI_URL, and THEME examples Co-Authored-By: Claude [email protected]
- Add STRICT_ENV_VAR so a rejected setting only fails init when asked; by default every problem is reported and that setting is skipped - Route control variables through read_control_flag so a malformed OVERWRITE/ALLOW_UNKNOWN/STRICT is reported and treated as off - Collect coerce failures in parse_defaults_env, so a value like OPENC3_SETTING_AI_CHAT=nope no longer escapes as an exception - Add a :record action to plan_setting so OVERWRITE takes provenance without rewriting a matching value and bumping updated_at - Guard the news_feed seed in ScopeModel#seed_database, which was unconditional and discarded OPENC3_SETTING_NEWS_FEED Co-Authored-By: Claude [email protected]
Without a near match the sentence ran straight into "Set OPENC3_SETTINGS_ALLOW_UNKNOWN" with no period between them. Co-Authored-By: Claude [email protected]
…t_config - Split the JSON settings out of :string into :json_text and :json, so coerce parses and validates the blob instead of writing it through - Add require_keys and enforce it for system_health, whose reader raises on a partial blob rather than falling back - Add system_health and ai_chat_config to KNOWN_SETTINGS, with NO_ADMIN_TAB so the Admin Console drift specs still pass - Report a JSON setting's allowed value as its object shape in describe_settings rather than "any text" Refs #3471 Co-Authored-By: Claude [email protected]
Contributor
Author
❯ ./openc3.sh cli initsettings --export
Container cosmos-openc3-cosmos-init-run-5f4bab5da18a Creating
Container cosmos-openc3-cosmos-init-run-5f4bab5da18a Created
# Paste under the openc3-cosmos-init service's "environment:" key in
# compose.override.yaml, keeping this indentation.
- OPENC3_SETTING_NEWS_FEED=true
- OPENC3_SETTING_TIME_FORMAT=ampm
- OPENC3_SETTING_THEME=cosmosDarkSlate
- "OPENC3_SETTING_SOURCE_URL=https://github.com/OpenC3/cosmos"
- "OPENC3_SETTING_RUBYGEMS_URL=https://rubygems.org"
- "OPENC3_SETTING_PYPI_URL=https://mirrors.sustech.edu.cn/pypi/web/"
- "OPENC3_SETTING_SYSTEM_HEALTH={\"cpu\":{\"redThreshold\":90.0,\"yellowThreshold\":80.0,\"snoozeMinutes\":15,\"lastTriggerTimeRed\":null,\"lastTriggerTimeYellow\":null,\"sustainedSeconds\":15},\"memory\":{\"redThreshold\":90.0,\"yellowThreshold\":80.0,\"snoozeMinutes\":15,\"lastTriggerTimeRed\":null,\"lastTriggerTimeYellow\":null,\"sustainedSeconds\":15},\"disk\":{\"redThreshold\":90.0,\"yellowThreshold\":80.0,\"snoozeMinutes\":720,\"lastTriggerTimeRed\":null,\"lastTriggerTimeYellow\":null,\"sustainedSeconds\":60},\"global\":{\"enableAlerts\":true}}" |
- Add SettingModel.export_lines and yaml_env_item, quoting values so a JSON blob pasted into compose.override.yaml isn't read as a mapping - Gate --export on redis_available? so a stopped COSMOS gets an actionable message instead of "Bad file descriptor (redis://...)" - Add an example to every JSON setting and print them from describe_json_settings in --help, since the shape was undiscoverable - Stop apply_defaults claiming no variables were set when every one of them failed to coerce Co-Authored-By: Claude [email protected]
- Add the missing space to #services: so every structural line uses the same # prefix and uncommenting yields valid YAML - Use https://pypi.org for the OPENC3_SETTING_PYPI_URL example instead of a third-party university mirror
- Drop the existence skip in LocalMode.sync_settings: localinit runs after the plugin load that seeds source_url, rubygems_url, pypi_url, news_feed and system_health, so those five files never applied - Stop reading OPENC3_SETTINGS_OVERWRITE there, since it governs env var seeding and a malformed value only added a spurious error - Reword the plan_setting skip message, which claimed a setting "was changed from the seeded value" when an unrecorded value is the norm on a deployment upgrading into this feature - Correct the OVERWRITE_ENV_VAR comment: handle_true_false_strict takes 1/true, not yes/on Refs #3471 Co-Authored-By: Claude [email protected]
- Assert export_lines survives YAML for text carrying #, a quote, a backslash or a trailing space, which the example-driven round trip never exercised - Pair save_setting with sync_settings per type, the gap that let a boolean come back as the string "false" - Cover sync_settings across scopes, and that a folder for a scope ScopeModel doesn't know is left alone - Pin both plan_setting skip branches, whitespace handling in coerce and validate_setting!, the MAX_VALUE_BYTES boundary, the case-sensitive prefix scan and near_match? on a transposition Refs #3471 Co-Authored-By: Claude [email protected]
- Add a wrapper step covering --help, --export and the abort on an unknown option, so openc3.sh cli routing to openc3-cosmos-init stays exercised - Add a seeding step driving the init container through docker compose directly, since openc3.sh cli cannot inject OPENC3_SETTING_* vars - Cover the behaviors the feature promises: idempotent re-seed, a value initsettings didn't write left alone, OPENC3_SETTINGS_OVERWRITE, OPENC3_SETTINGS_STRICT exiting non-zero, and --dry-run writing nothing Refs #3471 Co-Authored-By: Claude [email protected]
mcosgriff
marked this pull request as ready for review
August 20, 2026 21:58
mcosgriff
requested review from
clayandgen and
jmthomas
and removed request for
jmthomas
August 20, 2026 21:58
jmthomas
reviewed
Aug 23, 2026
jmthomas
left a comment
jmthomas
left a comment
Member
There was a problem hiding this comment.
While the concept is simply the change got huge. Seemingly because of all the interactions with local mode, Admin Console, settings with different types (bool, string, json), etc. I like the documentation throughout but feels like we need some docs on this somewhere. At the very least the CLI was updated so https://docs.openc3.com/docs/getting-started/cli should be touched. Probably mention this capability here: https://docs.openc3.com/docs/tools/admin#settings.
I think we need another page or 2 under https://docs.openc3.com/docs/configuration that talks about compose.yaml / compose.override.yaml and .env / .env.local. This capability could then also be called out in the compose.yaml.
- Add flag_enabled to init.sh so empty, 0 and false (any case) turn OPENC3_DEMO off; other install flags stay presence-only - Flip .env to OPENC3_DEMO=true and document the accepted values - Lead with true/false in the initsettings help, compose.override.yaml and setting_model.rb comments, noting 1/0 also work - Spell out in the help and compose.override.yaml why an upgrade needs one run with OPENC3_SETTINGS_OVERWRITE=true - Add a SettingsTab.vue header pointing at KNOWN_SETTINGS, since a setting missing there cannot be seeded Co-Authored-By: Claude [email protected]
…o-overwrite-settings-in-init
The script's shebang is /bin/sh and the Dockerfile RUNs it directly, so busybox ash on Alpine never parsed [[ ... ]]. Co-Authored-By: Claude [email protected]
- Add an Initsettings section to cli.md covering OPENC3_SETTING_<NAME> seeding, why an existing setting is left alone, OVERWRITE / ALLOW_UNKNOWN / STRICT, --dry-run, --export and Local Mode - Note that only server-side settings can be seeded, since Suppressed Warnings, Default Configs and the editor's Vim mode / default language live in browser local storage - Cross-link the Admin Settings tab to the new section with the same caveat, replacing nothing - the tab had no mention of deploy-time configuration Refs #3471 Co-Authored-By: Claude [email protected]
Assign "$1" to a local `value` so the case subject reads as what it is, which is also what SonarQube's positional-parameter rule asks for. Refs #3471 Co-Authored-By: Claude [email protected]
- Gate at --severity=warning over `git ls-files '*.sh'`, using the ShellCheck already in the runner image so there is no new action to SHA pin - Report the note-level findings in a second, non-gating step, since the release and setup scripts carry hundreds of SC2086 quoting notes Co-Authored-By: Claude [email protected]
The check name sat next to "Check Spelling" in the PR checks list and read as a spelling job. Renames the file to match python_lint.yml and follows the self-referencing paths: entry. Co-Authored-By: Claude [email protected]
Contributor
Author
|
Docs added. Two new pages under Configuration, as suggested: Docker Compose (
|
- Add configuration/compose.md: how compose.override.yaml merges over
compose.yaml, overriding versus adding a variable, the install flags,
and where OPENC3_SETTING_* variables go
- Add configuration/environment.md: .env versus .env.local, and the
precedence rule that ${VAR} interpolation and a container's
environment: block are separate steps
- Give the three OPENC3_SETTINGS_* control variables their own section
in cli.md, with defaults, a compose example, and the contrast with
the presence-only OPENC3_NO_* flags
- Note in compose.yaml that OPENC3_SETTING_* are deliberately unlisted,
since an override adds variables and a new setting needs no edit here
- Cross-link admin.md and cli.md to the new pages
Refs #3471
Co-Authored-By: Claude [email protected]
The job only reads the tree, so the checkout token has no use in .git/config and is one less thing for a later step to reach. Co-Authored-By: Claude [email protected]
jmthomas
requested changes
Aug 28, 2026
jmthomas
left a comment
jmthomas
left a comment
Member
There was a problem hiding this comment.
New docs are awesome. Only a few more nits.
- Require all five keys on classification_banner in KNOWN_SETTINGS, so initsettings rejects a partial blob at seed time instead of writing one every reader has to defend against - Default each key in ClassificationBannerSettings.parseSetting: an undefined color left formValid false with no invalid field to fix, disabling Save so the banner could not be edited at all - Merge over the defaults in ClassificationBanners rather than replacing them, since a missing height reached the stylesheet as "height: undefinedpx" and drew the banner top and bottom - Guard both JSON.parse calls, keeping the defaults and reporting the bad value rather than throwing out of a created hook - Document the required keys and the whole-scalar YAML quoting rule in compose.override.yaml, and fix a comment that still said two control variables Refs #3471 Co-Authored-By: Claude [email protected]
jmthomas
previously approved these changes
Sep 2, 2026
…o-overwrite-settings-in-init
- Add explicit else clauses to the handle_true_false_nil and handle_defined_constants case statements, the two CRITICAL smells failing the quality gate - Merge nested if statements in parse_loop for the progress callback and trailing-comment detection - Replace if ! / if not with unless in scope_model.rb and local_mode.rb#scan_plugin_dir - Drop trailing _ from multiple assignments in favor of .first - Mark the unused scope: keyword arguments # NOSONAR, matching metric_model.rb, since renaming them Co-Authored-By: Claude
|
jmthomas
approved these changes
Sep 3, 2026
| # - OPENC3_ALLOW_HTTP=1 | ||
| # # Remove 'unsafe-eval' from Content Security Policy script-src (stricter). | ||
| # # Only set this if no plugins need eval() or new Function(). | ||
| # - OPENC3_NO_EVAL=1 |
Member
There was a problem hiding this comment.
Not sure when OPENC3_NO_EVAL was merged but is this just a doc update?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #3471. Customers can configure default time zone and time format (and any other Admin Console setting) at deploy time instead of clicking through the Admin Console after every fresh install.
openc3cli initsettings, run byinit.shafterinitbuckets, which seeds settings fromOPENC3_SETTING_<NAME>environment variables. A prefix scan rather than an enumerated list, so a setting added by a later release needs no code change here.OPENC3_SETTINGS_OVERWRITEmakes the environment authoritative instead.OPENC3_SETTING_TIME_ZONESwould otherwise be a dead Redis key plus a setting the operator believes they configured and did not.OPENC3_SETTINGS_ALLOW_UNKNOWNopts out for a setting a newer tool added.ConfigParser.handle_true_false_strict, accepting1/TRUE/0/FALSEand raising otherwise.handle_true_falseitself is unchanged because table_config.rb:268 feeds item defaults through it, where mapping'1'totruewould corrupt a numeric default of 1. This makesOPENC3_SETTINGS_OVERWRITE=0mean off, unlike theOPENC3_NO_*presence flags where=0counts as on.LocalMode.sync_settings, which overwrote Redis on everylocalinit(reverting Admin Console edits) and stored file contents verbatim (so a boolean setting round-tripped to a truthy string). It now uses the same seed guard and JSON coercion.compose.override.yaml.compose.yamlis deliberately unchanged: an override'senvironment:block adds variables the base file does not list, so no compose.yaml edit is ever needed to add a setting.cli initsettings --helpso the list can't drift from the codeExample:
Test plan
Core:
Enterprise (OpenC3/cosmos-enterprise#713):
Associated https://github.com/OpenC3/cosmos-enterprise/pull/713