Skip to content

Conversation

@rileyseaburg
Copy link
Contributor

@rileyseaburg rileyseaburg commented Nov 22, 2025

This pull request introduces several improvements to the Rust widget renderer integration, environment configuration, and build/deployment processes. The main goals are to ensure robust handling of the Rust extension across environments, improve boolean serialization for widget rendering, and allow more flexible configuration of asset and session domains. Additionally, it updates CI/CD workflows and removes an obsolete test file.

Widget Renderer Integration Improvements

  • Updated the Rust extension loader to require lib/widget_renderer instead of the previous path, ensuring the correct file is loaded after build and runtime compilation. (config/initializers/widget_renderer.rb) [1] [2]
  • Improved the Ruby wrapper for the Rust extension to handle stale or incorrect WidgetRenderer module definitions, ensuring Rutie initializes the class properly. (ext/widget_renderer/lib/widget_renderer.rb) [1] [2]
  • Enhanced the Rust-side deserialization to treat missing or null booleans as false, preventing errors from legacy or incomplete Rails data. (ext/widget_renderer/src/form_data.rs) [1] [2]
  • Updated the Rails model to consistently serialize booleans and question text for the Rust widget renderer, and to skip Rust rendering in test environments. (app/models/form.rb) [1] [2]

Build and Deployment Process Updates

  • Improved the build artifact handling in the GitHub workflow and CircleCI config, ensuring the correct shared library is found and copied regardless of target or environment. (.github/workflows/build-widget.yml, .circleci/config.yml, .cfignore, .profile.d/build_widget_renderer.sh, ext/widget_renderer/extconf.rb) [1] [2] [3] [4] [5] [6] [7]

Environment and Configuration Flexibility

  • Added support for overriding asset host and session cookie domain via environment variables, improving deployment flexibility across staging and production. (config/environments/staging.rb, config/initializers/session_store.rb, touchpoints-staging.yml) [1] [2] [3] [4]

Testing and CI/CD Adjustments

  • Removed an obsolete controller export spec to clean up the test suite. (spec/controllers/admin/cx_collections_controller_export_spec.rb)
  • Improved feature spec reliability by using more robust selectors and waits. (spec/features/admin/forms/form_permissions_spec.rb)

These changes collectively improve the reliability, maintainability, and deployment flexibility of the widget renderer integration and overall application.

Copilot AI review requested due to automatic review settings November 22, 2025 16:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR merges the Rust widget renderer feature branch into develop, introducing Rust-based widget rendering capabilities with buildpack integration for staging deployment. The changes include infrastructure updates, configuration refinements, and bug fixes to support the new rendering architecture.

  • Integrates Rust buildpack for compiling the widget renderer extension in Cloud Foundry environments
  • Updates staging domain configuration from touchpoints-staging.app.cloud.gov to app-staging.touchpoints.digital.gov
  • Fixes bug in form model where question_text incorrectly referenced non-existent attribute (should be text)

Reviewed changes

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

Show a summary per file
File Description
touchpoints-staging.yml Updates redirect URI to new domain, adds ASSET_HOST and domain configuration, includes nodejs buildpack for build process
spec/controllers/admin/cx_collections_controller_export_spec.rb Removes duplicate test file (tests remain in cx_collections_controller_spec.rb)
ext/widget_renderer/lib/widget_renderer.rb Refactors module wrapper to top-level code with explicit class creation to resolve Rutie compatibility issues
config/initializers/widget_renderer.rb Updates require path to match new lib/ directory structure
config/initializers/session_store.rb Changes session domain configuration to use new SESSION_COOKIE_DOMAIN environment variable
config/environments/staging.rb Adds ASSET_HOST environment variable support with fallback to TOUCHPOINTS_WEB_DOMAIN
app/models/service.rb Fixes indentation consistency in CSV export attribute exclusion array
app/models/form.rb Corrects question attribute reference from question_text to text
.profile.d/build_widget_renderer.sh Adds comprehensive Rust toolchain detection and Ruby library linking configuration
.gitignore Expands patterns to ignore all .env.* and vars*.yml files
.github/workflows/build-widget.yml Simplifies build to use default target, improves artifact detection robustness
.cfignore Adds negation patterns to preserve prebuilt widget library while ignoring build artifacts

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

# Ignore Rust build artifacts
# Ignore Rust build artifacts, but keep the prebuilt widget library
target/
ext/widget_renderer/target/
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The negation pattern on line 44 won't work as intended because line 43 already ignores the parent directory ext/widget_renderer/target/. In gitignore-style patterns, once a parent directory is ignored, you cannot un-ignore it with a negation pattern before un-ignoring its contents.

Consider removing line 43 (ext/widget_renderer/target/) and keeping only line 42 (target/) which already covers this path. The specific negations on lines 45-46 should then work correctly.

Suggested change
ext/widget_renderer/target/

Copilot uses AI. Check for mistakes.
# frozen_string_literal: true

Rails.application.config.session_store :cookie_store, key: '_touchpoints_session', domain: ENV.fetch('TOUCHPOINTS_WEB_DOMAIN'), same_site: :lax, expire_after: 30.minutes
cookie_domain = ENV['SESSION_COOKIE_DOMAIN'].presence
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SESSION_COOKIE_DOMAIN environment variable is not defined in touchpoints-staging.yml, which means cookie_domain will be nil. This changes the cookie domain behavior from the previous explicit value (ENV.fetch('TOUCHPOINTS_WEB_DOMAIN')). A nil domain makes the cookie available only to the current host without subdomains.

Consider either:

  1. Adding SESSION_COOKIE_DOMAIN to the environment configuration in touchpoints-staging.yml, or
  2. Providing a fallback: cookie_domain = ENV['SESSION_COOKIE_DOMAIN'].presence || ENV.fetch('TOUCHPOINTS_WEB_DOMAIN')
Suggested change
cookie_domain = ENV['SESSION_COOKIE_DOMAIN'].presence
cookie_domain = ENV['SESSION_COOKIE_DOMAIN'].presence || ENV.fetch('TOUCHPOINTS_WEB_DOMAIN')

Copilot uses AI. Check for mistakes.
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

rileyseaburg and others added 30 commits December 19, 2025 12:23
* Production release: Fix CX Collections export CSV error (#1911)

* Add Cargo caching and library linkage verification to CircleCI

- Add restore/save cache for Cargo target and registry directories
- Use Cargo.lock checksum for cache key to ensure proper invalidation
- Verify Rust library linkage with ldd before deploy
- Fail fast if library has unresolved dependencies
- Should significantly speed up Rust builds on subsequent runs

* Fix cx_collections export_csv 500 error

- Handle nil organization in User#cx_collections to prevent NoMethodError
- Add eager loading for service_provider.organization and cx_collection_details
- Add missing database indexes on cx_collections, cx_collection_details, cx_responses

Fixes the 'We're sorry, but something went wrong' error at:
/admin/cx_collections/export_csv

* Update schema.rb with new indexes for CircleCI

* Update schema version to include new migration

* Add tests for User#cx_collections method

* fix: use git URL for rust buildpack in production manifest

* fix: correct redis service name in production manifest

* Release: WidgetRenderer load fix (#1914)

* Add Cargo caching and library linkage verification to CircleCI

- Add restore/save cache for Cargo target and registry directories
- Use Cargo.lock checksum for cache key to ensure proper invalidation
- Verify Rust library linkage with ldd before deploy
- Fail fast if library has unresolved dependencies
- Should significantly speed up Rust builds on subsequent runs

* Fix cx_collections export_csv 500 error

- Handle nil organization in User#cx_collections to prevent NoMethodError
- Add eager loading for service_provider.organization and cx_collection_details
- Add missing database indexes on cx_collections, cx_collection_details, cx_responses

Fixes the 'We're sorry, but something went wrong' error at:
/admin/cx_collections/export_csv

* Update schema.rb with new indexes for CircleCI

* Update schema version to include new migration

* Add tests for User#cx_collections method

* Fix widget renderer load when native lib missing (#1913)

* Fix User#cx_collections specs for Service owner (#1915) (#1917)

* Release: set prod disk quota to 2G (#1919)

* Add Cargo caching and library linkage verification to CircleCI

- Add restore/save cache for Cargo target and registry directories
- Use Cargo.lock checksum for cache key to ensure proper invalidation
- Verify Rust library linkage with ldd before deploy
- Fail fast if library has unresolved dependencies
- Should significantly speed up Rust builds on subsequent runs

* Fix cx_collections export_csv 500 error

- Handle nil organization in User#cx_collections to prevent NoMethodError
- Add eager loading for service_provider.organization and cx_collection_details
- Add missing database indexes on cx_collections, cx_collection_details, cx_responses

Fixes the 'We're sorry, but something went wrong' error at:
/admin/cx_collections/export_csv

* Update schema.rb with new indexes for CircleCI

* Update schema version to include new migration

* Add tests for User#cx_collections method

* Fix widget renderer load when native lib missing (#1913)

* Fix User#cx_collections specs for Service owner (#1915)

* Fix production manifest for rust buildpack (#1918)

* fix: remove empty secret keys from manifest to prevent wiping env vars on deploy

Empty values in the manifest were overwriting secrets set via cf set-env.
Secrets should only be managed via cf set-env, not in the manifest.

* fix: Use fba-usa-modal class for USWDS Modal compatibility

* fix: Add CSS support to Rust widget renderer to fix modal positioning

* fix: Use parent_id instead of parent in Organization factory

* Increase Cloud Foundry start timeout to 180s and fix Sidekiq health check type

* Fix Sidekiq crash and optimize Rust build script

- Added rust-buildpack to Sidekiq deployment
- Updated build_widget_renderer.sh to handle workspace paths and avoid rebuilds
- Added rust-buildpack to touchpoints-demo manifest

* build(cf): simplify widget_renderer build script and ignore rules

Refactors the `.profile.d/build_widget_renderer.sh` script to remove complex logic for locating Rust dependencies, building from source, and checking library linkage. The script now focuses solely on setting `LD_LIBRARY_PATH` and ensuring the prebuilt `libwidget_renderer.so` is in the correct location.

Additionally, updates `.cfignore` to exclude the `target/` directory and specific release artifacts, while ensuring the root `libwidget_renderer.so` is preserved. This suggests a shift towards deploying a precompiled binary rather than building the Rust extension during the Cloud Foundry deployment process.

* Increase deployment timeout and add widget_renderer fallback to resolve startup crashes

* fix(widget_renderer): restore LoadError and adjust app timeout

- Re-enable raising `LoadError` in `widget_renderer.rb` when the native library is missing, removing the previous fallback log message. This ensures the application fails fast if the required extension is absent.
- Reduce application timeout from 600 to 180 seconds in `touchpoints.yml` to better align with platform constraints or performance expectations.
- Remove the hardcoded `WEB_CONCURRENCY: 1` environment variable from `touchpoints.yml`, allowing the buildpack or platform defaults to manage concurrency.

* ci: increase deployment wait time and enable static files in dev

- Increase the `max_wait` time in `.circleci/deploy.sh` from 600 to 800 seconds to prevent timeouts during longer deployment processes.
- Update `config/environments/development.rb` to conditionally enable the public file server based on the `RAILS_SERVE_STATIC_FILES` environment variable, aligning development behavior with other environments when needed.

* ci(deploy): increase timeouts and enable static file serving

- Increase Cloud Foundry push timeout from 180s to 600s in deployment scripts and `touchpoints.yml` to prevent timeouts during startup.
- Enable `RAILS_SERVE_STATIC_FILES` in production configuration and manifest to allow the application to serve precompiled assets directly.
- Update `deploy.sh` to accept a manifest path argument and refactor retry logic for better error handling.

* Decouple db:migrate from deploy: migrations must be run separately to avoid CF 180s timeout

* Add automated pre-deploy migrations via cf run-task to avoid 180s timeout during app start

* Set health check type to process for sidekiq worker before rolling deploy

* Fix sidekiq worker timeout: explicitly set to 180s before rolling deploy

* Fix flaky logo upload test: add wait time to prevent Selenium stale element race condition

* Fix cf set-health-check: use --invocation-timeout instead of --timeout

* Fix Rack::Attack test: create actual form fixture to avoid 404 responses

* Scale sidekiq worker to 1 instance during rolling deploy to avoid org memory quota exceeded

* Stop sidekiq worker before push to free memory for staging (avoids org quota exceeded)

* Fix cf run-task syntax: add --command flag for migrations

* Skip WidgetRenderer load during migrations - library not built in task droplet

* Fix Rack::Attack test: add valid submission params to avoid 400 errors

* Temporarily disable pre-deploy migrations to unblock deployment

* Fix widget_renderer initializer - use simpler skip detection logic

* Fix deployment: restore db:migrate in manifest and enable migrations in deploy script

* Revert migrations to start command - Rust library not available in cf tasks

* Build Rust library at runtime in .profile.d script

* Revert to working widget_renderer script that copies prebuilt library

* Keep prebuilt Rust library during deployment to ensure correct linking with CF Ruby installation

* Fix deploy-sidekiq.sh: remove explicit buildpack flags to avoid re-installing Rust

- Remove -b flags from cf push in deploy-sidekiq.sh
- Let CF auto-detect buildpacks from app metadata like deploy.sh does
- Prevents unnecessary reinstallation of Rust during staging
- Matches web deployment behavior for consistency

* Fix touchpoints.yml: comment out buildpacks to prevent Rust reinstallation

- Comment out explicit buildpacks in manifest
- Let CF auto-detect buildpacks from app metadata
- Prevents re-running supply phase (Rust installation) during deployment
- Rust library already built in CircleCI via bundle install (gem extensions)
- Matches deploy-sidekiq.sh approach for consistency

* Fix flaky timing test in submission_digest mailer spec

Use a single time_threshold let variable to ensure consistent timestamp
comparison instead of calling days_ago.days.ago multiple times which can
result in 1-second differences in CI environments.

* Fix custom-button-modal USWDS initialization

- Add try/catch error handling
- Add conditional checks for fbaUswds methods
- Initialize custom button element for custom-button-modal
- Prevents modal from appearing visible at page bottom

* Bump Cargo version to force Rust rebuild

* Bump widget_renderer version to force Cargo rebuild

* Force cargo clean before build to ensure recompilation

* Bump widget_renderer gem version to 0.1.2 to force rebuild

* Force Rust rebuild with BUILD_ID and version bump

* Bump widget_renderer to 0.1.2 to force CF to rebuild native extension

* Update Gemfile.lock for widget_renderer 0.1.2

* Add Rust library verification before CF push

* Prioritize workspace-level Rust library and bump to 0.1.3

* Invalidate CircleCI cargo cache to force fresh Rust build

* Fix Rust widget renderer modal button initialization

The Rust widget renderer was missing the USWDS Modal initialization
for the #fba-button element used in 'modal' delivery method forms.
This caused the toast/feedback button to not open the modal when clicked,
instead rendering the form inline at the bottom of the page.

Added initialization for #fba-button to match the ERB template behavior
in _fba.js.erb (lines 858-875).

Fixes Zendesk ticket #37620

* Fix breaking changes from PR review

1. Fix modal_class prefix logic: Now respects load_css setting
   - When load_css=true: uses 'fba-usa-modal' prefix
   - When load_css=false: uses 'usa-modal' (no prefix)
   - Matches ERB template behavior in _fba.js.erb line 110

2. Fix CSS backtick escaping: Added escape for backticks in CSS
   - Prevents JavaScript syntax errors when CSS contains backticks
   - CSS is inserted into JS template literals using backticks

3. Remove expired certificate file: tmp_expired_login_gov_cert.pem
   - Certificate expired Aug 2023
   - Added *.pem to .gitignore to prevent future accidental commits
1. Fix element_selector handling in template_renderer.rs
   - Added null check for element_selector before calling getElementById
   - Prevents potential JS errors with empty/null selectors

2. Remove unnecessary cargo clean from extconf.rb
   - Removing cached artifacts significantly increases build time
   - Clean CI environment doesn't need cargo clean before build

3. Extract mock request setup to helper method in form.rb
   - Created build_controller_with_mock_request private method
   - Reduces code duplication between touchpoints_js_string and render_widget_css
   - Improves maintainability
Empty string '' is truthy in JavaScript when interpolated from Rust.
Use length check to properly handle empty element_selector values.
Added wait_for_ajax after 'form title saved' message appears to ensure
the backend save completes before page refresh. Also removed duplicate
wait_for_builder call.
Documents the incident where the Rust widget renderer was missing
the #fba-button USWDS Modal initialization, causing modal widgets
to not open when users clicked the feedback button.

Zendesk ticket #37620
- Add --no-wait flag to cf push so it succeeds after first instance is healthy
- Add wait_for_deployment_complete function to explicitly wait for all instances
- Improve retry logic to detect and wait for active deployments instead of
  canceling them with a new push
- Increases effective deployment timeout from 180s to 15 minutes

This fixes the infinite loop where rolling deployments kept getting canceled
before all 18 instances could be replaced, leaving old instances running.
The Cargo cache key was based only on Cargo.lock, not the Rust source code.
This meant changes to template_renderer.rs were not triggering rebuilds.
Bump cache version to v3 and widget_renderer version to 0.1.3 to force rebuild.
The cache key uses Cargo.lock checksum, so we need to change the file
to invalidate the cache. Updated dependencies to latest versions.
The cache key now includes both Cargo.lock AND template_renderer.rs checksums.
This ensures source code changes trigger a rebuild, not just dependency changes.

Previous cache keys only used Cargo.lock which didn't change when the Rust
source code was modified.
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Riley Seaburg <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Riley Seaburg <[email protected]>
Fix cf-cli installation using APT repository
The production deployment was failing because new instances crashed on startup.
Root cause: the manifest was missing the touchpoints-s3-uploads service that
the app expects. CF was unbinding this service during rolling deployments,
causing Rails to exit with status 1.
The GPG key must be converted to binary format using gpg --dearmor
and stored in /usr/share/keyrings/ for modern apt to recognize it.
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.

2 participants