feat: triage workspace + PAT auth + upgrade infrastructure + seed dataset + OpenAPI hardening#731
Open
aaronlippold wants to merge 623 commits into
Open
feat: triage workspace + PAT auth + upgrade infrastructure + seed dataset + OpenAPI hardening#731aaronlippold wants to merge 623 commits into
aaronlippold wants to merge 623 commits into
Conversation
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 33788689 | Triggered | Generic Password | 4271c98 | spec/requests/api/auth_spec.rb | View secret |
| 33788688 | Triggered | Generic Password | 4271c98 | doc/openapi/paths/api_auth_login.yaml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes the comment/triage workflow and development seeding by introducing a split-pane triage UI (replacing the modal flow), modular/idempotent seed helpers + dev rake tasks, and a DRY’d comment composer state model (ReplyComposerMixin) with supporting factory/test improvements.
Changes:
- Add split-pane triage experience (queue navigation + rule context panel + extracted triage form) and related API support (
include_rule_content, HTML redirect to triage). - Modularize and validate seed pipeline (SeedHelpers, numbered seed files,
dev:*rake tasks, seed verification specs). - Consolidate composer/triage vocabulary utilities and standardize factories/spec fixtures around new traits.
Reviewed changes
Copilot reviewed 83 out of 83 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/services/import/json_archive_importer_spec.rb | Switch review setup to factories/traits for import lifecycle fixtures. |
| spec/services/import/integration/backup_round_trip_spec.rb | Use factory-created reviews in backup round-trip integration spec. |
| spec/services/export/serializers/backup_serializer_spec.rb | Use factory traits for review lifecycle/duplicate-of serialization coverage. |
| spec/services/export/base_spec.rb | Use factory traits for disposition-related export setup. |
| spec/seeds/seed_pipeline_spec.rb | Add end-to-end seed pipeline spec (tagged, truncation strategy). |
| spec/requests/users_spec.rb | Use review factory traits for “My Comments” request fixtures. |
| spec/requests/rules_spec.rb | Use review factory traits for component JSON sidebar/history tests. |
| spec/requests/reactions_spec.rb | Use factory-created reviews/replies for reaction request specs. |
| spec/requests/rack_attack_spec.rb | Use factory-created review for rack-attack reaction target fixture. |
| spec/requests/projects_import_backup_spec.rb | Use factory-created review for project import backup tests. |
| spec/requests/projects_disposition_matrix_export_spec.rb | Use factory traits for triage-status filtered disposition export tests. |
| spec/requests/projects_create_from_backup_spec.rb | Use factory-created review for include_reviews param coverage. |
| spec/requests/project_comments_aggregate_spec.rb | Use factory traits to build aggregate comment fixtures. |
| spec/requests/components_spec.rb | Add HTML redirect expectation for /components/:id/comments; use review factory. |
| spec/requests/components_disposition_matrix_export_spec.rb | Use review factory traits for component disposition export fixtures. |
| spec/requests/component_reviews_spec.rb | Use :component_comment factory trait for component-scoped review tests. |
| spec/rails_helper.rb | Exclude :seed_pipeline specs by default to avoid parallel DB corruption. |
| spec/models/user_spec.rb | Use review factory traits for imported attribution behavior tests. |
| spec/models/reviews_spec.rb | Standardize review creation on factories/traits across invariants + scopes. |
| spec/models/review_polymorphic_commentable_spec.rb | Use :comment/:component_comment traits for polymorphic coverage. |
| spec/models/review_cross_scope_validations_spec.rb | Use review factory traits for cross-scope validation fixtures. |
| spec/models/reaction_spec.rb | Use review factory traits for reaction invariants and summary tests. |
| spec/models/query_performance_spec.rb | Use factory-created reviews to drive component review query tests. |
| spec/models/project_pending_comment_counts_spec.rb | Use factory traits for pending/total count fixtures. |
| spec/models/paginated_comments_pii_spec.rb | Use review factory traits for paginated comment response/reaction counts. |
| spec/models/components_spec.rb | Add assertions for include_rule_content payload shape + updated_at presence. |
| spec/models/component_pending_comment_counts_spec.rb | Use review factory traits for per-component pending count fixtures. |
| spec/lib/seed_helpers_spec.rb | Add unit specs for SeedHelpers API (seed_xccdf/component/review/reply/status/verify). |
| spec/lib/disposition_matrix_export_spec.rb | Switch to review factory traits for export and CSV defang coverage. |
| spec/javascript/mixins/ReplyComposerMixin.spec.js | Add vitest coverage for unified composer state mixin behaviors. |
| spec/javascript/constants/triageVocabulary.spec.js | Add tests for buildStatusFilterOptions(). |
| spec/javascript/components/triage/TriageQueueNav.spec.js | Add tests for 2D triage queue navigation component. |
| spec/javascript/components/triage/CommentTriageForm.spec.js | Add tests for extracted triage form component validation/events. |
| spec/javascript/components/components/ComponentComments.spec.js | Update ComponentComments tests for split-mode behavior and sorting/filter visibility. |
| spec/javascript/components/components/CommentTriageModal.spec.js | Update modal tests to validate delegation to CommentTriageForm and new doTriage API. |
| spec/factory_specs/stig_deadlock_spec.rb | Add regression spec preventing STIG factory deadlock/import side effects. |
| spec/factory_specs/factory_traits_spec.rb | Add spec verifying new FactoryBot traits across multiple models. |
| spec/factories/stigs.rb | Add :skip_rules trait to disable rule import on STIG factory. |
| spec/factories/stig_rules.rb | Ensure stig_rule factory uses a skip-import STIG by default. |
| spec/factories/rules.rb | Add rule status/locked traits used by tests/seeds. |
| spec/factories/reviews.rb | Add review comment/triage/lifecycle traits + auto-membership wiring. |
| spec/factories/projects.rb | Add project membership traits (:with_admin, :with_members). |
| spec/factories/memberships.rb | Add explicit :viewer trait. |
| spec/factories/components.rb | Add component traits for comment period, PoC, released/locked rules. |
| spec/config/seed_idempotency_spec.rb | Update idempotency checks to match modular seed loader + SeedHelpers. |
| spec/blueprints/rule_blueprint_spec.rb | Use review factory traits for comment summary blueprint tests. |
| spec/blueprints/review_membership_blueprints_spec.rb | Use review factory traits for blueprint output coverage. |
| lib/tasks/dev.rake | Add dev:prime/status/verify/reset seed tasks. |
| lib/seed_helpers.rb | Introduce SeedHelpers module (XCCDF import, component seeding, comment seeding, verify/status). |
| docs/superpowers/plans/2026-05-19-comment-interaction-dry.md | Document DRY plan for composer state/event standardization. |
| docs/development/testing.md | Document available FactoryBot traits and usage patterns. |
| docs/development/seed-system.md | Document seed architecture, commands, and SeedHelpers API. |
| db/seeds/data/00_users.rb | Modular seed: demo admin + role-tier + filler users. |
| db/seeds/data/01_projects.rb | Modular seed: demo projects. |
| db/seeds/data/02_srgs.rb | Modular seed: SRG imports via SeedHelpers. |
| db/seeds/data/03_stigs.rb | Modular seed: STIG imports via SeedHelpers. |
| db/seeds/data/04_components.rb | Modular seed: demo components, overlays, dummy stress components, PoC backfill. |
| db/seeds/data/05_memberships.rb | Modular seed: demo RBAC wiring and counter cache reset. |
| db/seeds/data/06_rule_statuses.rb | Modular seed: vary rule statuses for demo coverage. |
| db/seeds/data/10_comments.rb | Modular seed: demo comment threads + triage states. |
| db/seeds/data/11_cross_project.rb | Modular seed: cross-project comment fixtures + triage examples. |
| app/models/review.rb | Improve sync_commentable_from_rule to backfill rule_id from commentable when needed. |
| app/models/component.rb | Add include_rule_content option to paginated_comments, include updated_at, and serialize rule content. |
| app/controllers/components_controller.rb | Redirect HTML /components/:id/comments to triage; add include_rule_content param handling for JSON. |
| app/javascript/mixins/ReplyComposerMixin.vue | Add unified comment composer state + open/close/post hooks. |
| app/javascript/constants/triageVocabulary.js | Add terminal/single-button status sets + buildStatusFilterOptions(). |
| app/javascript/composables/ruleFieldConfig.js | Add canonical FIELD_LABELS mapping for rule context display. |
| app/javascript/components/users/UserComments.vue | Adopt ReplyComposerMixin; switch truncation to CSS; use shared status options builder. |
| app/javascript/components/shared/CommentThread.vue | Switch date formatting to DateFormatMixin helper and remove ad-hoc formatter. |
| app/javascript/components/rules/RulesCodeEditorView.vue | Adopt ReplyComposerMixin and unify composer open/post handling. |
| app/javascript/components/components/ProjectComponent.vue | Adopt ReplyComposerMixin and unify composer open/post handling. |
| app/javascript/components/components/ComponentTriagePage.vue | Add command bar controls for split-mode context/admin panel; wire split-mode events. |
| app/javascript/components/components/ComponentComments.vue | Add split-mode triage integration; default sort by ID; hide filters in split mode; use status options builder; adopt ReplyComposerMixin. |
| app/javascript/components/components/CommentTriageModal.vue | Embed CommentTriageForm; refactor to doTriage API; use DateFormatMixin. |
| app/javascript/components/components/CommentDedupBanner.vue | Switch date formatting to DateFormatMixin helper and remove ad-hoc formatter. |
| app/javascript/components/triage/TriageQueueNav.vue | Add queue navigation UI (prev/next comment + prev/next rule + jump-to dropdown). |
| app/javascript/components/triage/RuleContextPanel.vue | Add rule context side panel with collapsible sections and related comments list. |
| app/javascript/components/triage/CommentTriageForm.vue | Extract triage form component used by modal + split view. |
| app/javascript/components/triage/TriageSplitView.vue | Add split-pane triage view with optimistic-lock support and admin sidebar actions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+47
| elsif is_stig | ||
| record = Stig.from_mapping(parsed) | ||
| existing = Stig.find_by(stig_id: record.stig_id) | ||
| if existing | ||
| puts " Already exists: #{existing.name} (Stig)" | ||
| return existing | ||
| end | ||
| record.xml = Nokogiri::XML(xml) | ||
| else |
Comment on lines
+29
to
+40
| trait :reply do | ||
| action { 'comment' } | ||
| comment { 'Reply to parent comment' } | ||
| triage_status { nil } | ||
|
|
||
| after(:build) do |review, _evaluator| | ||
| unless review.responding_to_review_id | ||
| parent = create(:review, :comment, user: review.user, rule: review.rule) | ||
| review.responding_to_review_id = parent.id | ||
| review.section = parent.section | ||
| end | ||
| end |
Comment on lines
+43
to
+60
| trait :component_comment do | ||
| action { 'comment' } | ||
| rule { nil } | ||
| section { nil } | ||
| comment { 'Component-level comment' } | ||
|
|
||
| after(:build) do |review| | ||
| unless review.commentable_type == 'Component' | ||
| component = create(:component, :skip_rules) | ||
| review.commentable = component | ||
| review.commentable_type = 'Component' | ||
|
|
||
| if review.user | ||
| project = component.project | ||
| create(:membership, user: review.user, membership: project, role: 'viewer') unless Membership.exists?(user: review.user, membership: project) | ||
| end | ||
| end | ||
| end |
Comment on lines
+119
to
+129
| trait :duplicate do | ||
| triage_status { 'duplicate' } | ||
|
|
||
| after(:build) do |review| | ||
| review.triage_set_by ||= create(:user) | ||
| review.triage_set_at ||= Time.current | ||
| unless review.duplicate_of_review_id | ||
| target = create(:review, :comment, rule: review.rule, user: review.user) | ||
| review.duplicate_of_review_id = target.id | ||
| end | ||
| end |
Comment on lines
+103
to
+135
| comments: { type: Array, required: true }, | ||
| currentId: { type: [Number, String], default: null }, | ||
| }, | ||
| computed: { | ||
| ruleGroups() { | ||
| const groups = []; | ||
| const seen = new Map(); | ||
| for (const c of this.comments) { | ||
| const key = c.rule_id || `component-${c.id}`; | ||
| if (!seen.has(key)) { | ||
| const group = { | ||
| ruleId: key, | ||
| ruleName: c.rule_displayed_name || "(component)", | ||
| comments: [], | ||
| }; | ||
| seen.set(key, group); | ||
| groups.push(group); | ||
| } | ||
| seen.get(key).comments.push(c); | ||
| } | ||
| return groups; | ||
| }, | ||
| currentPosition() { | ||
| for (let gi = 0; gi < this.ruleGroups.length; gi++) { | ||
| const group = this.ruleGroups[gi]; | ||
| for (let ci = 0; ci < group.comments.length; ci++) { | ||
| if (group.comments[ci].id === this.currentId) { | ||
| return { ruleIndex: gi, commentIndex: ci }; | ||
| } | ||
| } | ||
| } | ||
| return { ruleIndex: -1, commentIndex: -1 }; | ||
| }, |
Comment on lines
+218
to
+245
| props: { | ||
| rows: { type: Array, required: true }, | ||
| initialCommentId: { type: [Number, String], required: true }, | ||
| componentId: { type: [Number, String], required: true }, | ||
| effectivePermissions: { type: String, default: null }, | ||
| adminPanelOpen: { type: Boolean, default: false }, | ||
| contextMode: { type: String, default: "commented" }, | ||
| }, | ||
| data() { | ||
| return { | ||
| activeCommentId: this.initialCommentId, | ||
| isDirty: false, | ||
| saving: false, | ||
| conflictAlert: null, | ||
| adminAction: null, | ||
| adminAuditComment: "", | ||
| adminConfirmationId: "", | ||
| adminTargetRuleId: null, | ||
| }; | ||
| }, | ||
| computed: { | ||
| sortedRows() { | ||
| return [...this.rows].sort((a, b) => a.id - b.id); | ||
| }, | ||
| activeComment() { | ||
| return this.sortedRows.find((r) => r.id === this.activeCommentId) || null; | ||
| }, | ||
| canTriage() { |
Comment on lines
+59
to
+67
| it 'creates a review comment using FactoryBot' do | ||
| review = described_class.find_or_seed_review( | ||
| rule: rule, user: user, section: 'check_content', | ||
| comment: 'Test comment for seed helper' | ||
| ) | ||
| expect(review).to be_persisted | ||
| expect(review.action).to eq('comment') | ||
| expect(review.triage_status).to eq('pending') | ||
| end |
Comment on lines
+338
to
+374
| async onTriageSave(decision) { | ||
| await this.doSave(decision, false); | ||
| }, | ||
| async onTriageSaveAndNext(decision) { | ||
| await this.doSave(decision, true); | ||
| }, | ||
| async doSave(decision, advance) { | ||
| if (!this.activeComment) return; | ||
| this.saving = true; | ||
| this.conflictAlert = null; | ||
| try { | ||
| const payload = { | ||
| triage_status: decision.triage_status, | ||
| expected_updated_at: this.activeComment.updated_at, | ||
| }; | ||
| if (decision.response_comment) { | ||
| payload.response_comment = decision.response_comment; | ||
| } | ||
| if (decision.triage_status === "duplicate") { | ||
| payload.duplicate_of_review_id = decision.duplicate_of_review_id; | ||
| } | ||
|
|
||
| const triageRes = await axios.patch(`/reviews/${this.activeComment.id}/triage`, payload); | ||
| this.$emit("triaged", triageRes.data.review); | ||
|
|
||
| if (triageRes.data.response_review) { | ||
| this.$emit("response-posted", { | ||
| parentId: this.activeComment.id, | ||
| responseReview: triageRes.data.response_review, | ||
| }); | ||
| } | ||
|
|
||
| if (!SINGLE_BUTTON_STATUSES.has(decision.triage_status)) { | ||
| const adjRes = await axios.patch(`/reviews/${this.activeComment.id}/adjudicate`, {}); | ||
| this.$emit("adjudicated", adjRes.data.review); | ||
| } | ||
|
|
Contributor
|
Some notes after review:
Some side notes about locked fields:
|
Contributor
|
That 3NF doc design is a big enough change to warrant its own PR. Going to cherry pick it and make a new PR off main. |
Member
Author
|
@wdower — heads up, this PR is still in progress. All your review feedback items from the first round are addressed (admin inline, toggle moved, rename, locked comments, staleness badge, etc.) but I still have a few cards to finish:
Will push these in the next session. Don't need a full re-review yet — just wanted you to know the status. — Aaron |
aaronlippold
changed the title
aaronlippold
added a commit
that referenced
this pull request
May 20, 2026
Added/Changed/Fixed sections covering three-column layout, progress bar, DRY color palette, ARIA accessibility, InfoTooltip/InfoNotice adoption, and bug fixes Authored by: Aaron Lippold<lippold@gmail.com>
SRG-writing as a first-class document type (v5). Authored SRG requirements are expanded SrgRules with component linkage; catalog attachment at release. Records 14 design decisions: Moved terminal status + tombstone semantics, multi-parent lineage for SRG and STIG components (core vs derived eligibility), family technology token with ID minting at release, relocation marker + dual intake, reference benchmarks, mixed-project sections, full XCCDF export. Includes 7-phase implementation plan. Authored by: Aaron Lippold<lippold@gmail.com>
be_within(5.seconds) of a pre-call timestamp fails when the example runs slowly under load (observed at 6.7s on an unbalanced parallel run). The applier stamps last_sync_at with Time.current during the call, so assert it falls between before/after call bounds instead. Authored by: Aaron Lippold<lippold@gmail.com>
Rack::Attack uses fixed 60-second buckets; without freeze_time the requests can split across a bucket boundary on a slow run, so the counter never reaches the limit and the 429 assertion flakes. The reaction-throttling block already used this pattern — apply it to the login and comment-action blocks too. Authored by: Aaron Lippold<lippold@gmail.com>
Settings.reload! rebuilt the instance straight from vulcan.default.yml and dropped every backfill that 0_settings.rb applied at boot. The YAML renders those values nil when env vars are unset (no || false on smtp.enabled, no top-level oidc.discovery key, comment-only providers, empty contact_email/consent.content, unquoted #hex banner colors that YAML reads as comments), so any spec calling reload! left the rest of its parallel worker running against nil settings sections — a seed-order flake that failed settings_defaults and api_settings_contract on full runs. Move the backfills into an idempotent Settings.apply_defaults! and re-apply them from a reload! override (super.tap). The initializer now delegates to it, so no caller can obtain a Settings object without its defaults. Fix oidc_providers_spec's restore to reload after ClimateControl exits (the old after-hook reloaded the still-modified ENV, a no-op). Repoint the initializer-consistency spec at the new source location. Add a regression spec pinning the seven nil-from-YAML values across reload!. Authored by: Aaron Lippold<lippold@gmail.com>
14 assertions used be_within(N.seconds).of(Time.current) — under parallel load the gap between the action stamping the timestamp and the assertion reading Time.current can exceed the tolerance, causing false failures. Convert each to be_between(before_call, after_call) with before_call floored to microseconds; all timestamps are Ruby Time.current stamps, so the bounds are strictly tighter and load-proof. backup_round_trip's cross-record comparisons are left as-is. v2-107h.2 Authored by: Aaron Lippold<lippold@gmail.com>
Cut whole-suite output from ~4000 lines to ~520 by removing test-log noise at its roots: - Rake-task specs let task stdout leak (biggest: disa_guide dumped the full ~540-line guide). Add a shared capture_stdout helper (ActiveSupport:: Testing::Stream) and wrap the task invokes. disa_guide:update also converted the doc twice via a redundant convert invoke — extract validated_docx_path so both tasks validate and update converts once. - seed_helpers progress puts flooded the log during spec seeding. Route through an env-gated announce (silent in test unless VERBOSE). - 9 task specs each called Rails.application.load_tasks, re-defining every .rake constant and warning repeatedly. Add a load-once load_rake_tasks helper. Kills BackupAuditor + STATS_DIRECTORIES warnings. - Upgrade/db-rename specs' raw PG admin connections emitted NOTICE on DROP DATABASE IF EXISTS. SET client_min_messages TO warning on them. v2-107h.6, v2-107h.7 Authored by: Aaron Lippold<lippold@gmail.com>
Two specs hand-rolled the same modify-ENV / reload-Settings / restore-after dance. The restore must run after ClimateControl exits (in an ensure, not an after-hook) or the modified Settings leaks to later specs in the worker. Extract SettingsEnvHelpers#with_settings_env so no consumer can get the ordering wrong; migrate oidc_providers_spec and settings_reload_spec off their local copies. v2-107h.4 Authored by: Aaron Lippold<lippold@gmail.com>
Extract the component sync & merge engine (epic v2-480) out of PR #731 so the remaining triage/PAT/upgrade/OpenAPI work can merge without it. The engine is unfinished and prod-data-risky; it is preserved intact on branch feat/component-sync-merge and will be reconstructed off post-#731 master. Deleted (branch-only engine, 56 files): - app/services/import/json_archive/merge/ (20 files) + specs - MergeJob + ApplicationJob, ComponentSyncEvent/MergeOperation/MergeQuarantineRecord - lib/tasks/sync.rake, 7 merge migrations, merge OpenAPI paths/schemas Stripped merge-only hunks from shared files (kept all #731 work): - rule.rb: NESTED_MERGEABLE_ASSOCIATIONS (MERGEABLE_FIELDS/DERIVED_COLUMNS kept — load-bearing for RuleBuilder + backup round-trip) - review.rb: MERGEABLE_FIELDS constant - component.rb: component_sync_events / merge_operations associations - manifest_validator.rb: merge: param + branch (1.1 format support kept) - components_controller.rb: merge_status; projects_controller.rb: import_backup?merge + helpers - routes.rb: merge_status route; vulcan.default.yml: sync: block - db/schema.rb: 3 merge tables, last_sync_* columns, merge FKs - OpenAPI: merge params/paths/schemas removed, bundle regenerated Kept as #731's own work: ReviewBuilder/backup_serializer import hardening, rule_satisfactions FK constraints, backup format 1.1, and comment-merge (Review.merge_comments! / PATCH /reviews/merge). Schema.rb hand-edited (deterministic removal of merge DDL); authoritative db:migrate regen + full rspec run to follow in the split-validation step. Signed-off-by: Will Dower <will@dower.dev>
Resolve 2 conflicts from master advancing 2 commits (#736 ironbank, #740 gem bumps): release.yml keeps both the openapi-publish job (#731) and the Iron Bank release steps (master); Gemfile.lock takes master's security bumps reconciled with #731's gems via bundle install. Unblocks pull_request CI on the removal branch (was CONFLICTING with master). Signed-off-by: Will Dower <will@dower.dev>
Two CI failures on the merge-removal branch, both my own, neither from the engine removal itself: 1. Gemfile.lock: resolving the master merge with --theirs downgraded rack 3.2.6 -> 2.2.23 and concurrent-ruby 1.3.7 -> 1.3.6. #731 is ahead of master on gems (Rails 8 / rack 3). The rack 2.x downgrade broke :unprocessable_content, a rack 3.1+ status symbol, failing every spec asserting it; the concurrent-ruby downgrade re-introduced an advisory #731 had already fixed, failing bundle-audit. Restored #731's lock (rack 3.2.6, concurrent-ruby 1.3.7). 2. manifest_validator_spec: removing the merge: kwarg left keeper specs asserting merge-mode warnings and @merge. Dropped those examples; they belong with the merge PR. Signed-off-by: Will Dower <will@dower.dev>
Two amendments to the SRG Component Authoring ADR (v5 -> v6). Neither
overturns a §0 decision; the v5 storage core (authored SRG requirements
are expanded SrgRules) stands unchanged.
Scoping (§0.1 A) — retitled to "Generalized XCCDF Document Authoring —
SRG as the First New Profile". This is not "adding SRG support"; it is
generalizing Vulcan to author a document off any XCCDF upstream, with
SRG as the primary first-delivered profile. Vulcan's trajectory is to
scope beyond the DoD document space. Scope stays SRG + STIG — the ask is
that the seams be named and shaped for extension so a further profile is
additive config, not a schema migration. Records the five places the
STIG/SRG binary is currently hard-coded (document_type enum, parent
eligibility, the core flag, derived_from_srg_rule_id, the SRG catalog as
sole upstream registry) as naming/widening debt, and §2.2 defines the
per-profile policy seam.
User workflow (§0.1 B, §2.1, R6) — the v5 draft specified storage
exhaustively and the user's path not at all; the creation-time choice had
to be established by phone rather than read off the document. Now
normative: authoring type is chosen at creation and is exclusive (a
component is a STIG or an SRG, never both), and that choice gates the
source picker, the per-requirement status options, the field config,
Satisfied-By, the relocation marker, and the export mode. Adds the
creation walkthrough, the authoring loop, the immutability consequence,
and wires the workflow into the §14 phasing so it is carded rather than
lost between two storage phases.
Also records five review findings as open items (§10.4-8), none blocking
the core: the bare "Applicable" status value is a substring footgun in
the flat shared STATUSES list ('Not Applicable'.include?('Applicable')
is already true) and wants per-profile status vocabularies; Moved
conflates disposition with routing; mis-typed components have no
recovery path; release/catalog attachment risks aliasing published rows
with editable ones; primary-parent selection is undefined for N parents.
Signed-off-by: Will Dower <will@dower.dev>
Removes the component sync & merge engine (epic v2-480) from this PR so the remaining triage / PAT / upgrade / OpenAPI work can ship without it. The engine is unfinished (UI, disaster-recovery, integration tests and manifest v1.1 all still open) and carries production-data risk, so it is being landed as its own PR rather than holding this one up. The engine is preserved intact on branch feat/component-sync-merge (also pushed to origin). After this PR merges to master, that branch rebases onto the new master and reduces to the engine alone — which is why this PR should land as a merge commit, NOT a squash: a squash breaks that rebase. Removed (56 files): app/services/import/json_archive/merge/ + specs, MergeJob + ApplicationJob, ComponentSyncEvent / MergeOperation / MergeQuarantineRecord, lib/tasks/sync.rake, 7 merge migrations, merge OpenAPI paths and schemas. Stripped merge-only hunks from 12 shared files, keeping all of this PR's own work: ReviewBuilder / backup_serializer import hardening, Rule::MERGEABLE_FIELDS + DERIVED_COLUMNS (load-bearing for RuleBuilder and the backup round-trip), rule_satisfactions FK constraints, backup format 1.1, and comment-merge (Review.merge_comments! / PATCH /reviews/merge — unrelated to the component merge engine). Also merges origin/master, resolving this PR's conflict with master (#736 ironbank workflow, #740 gem bumps). release.yml keeps both the openapi-publish job and the Iron Bank release steps; Gemfile.lock keeps this PR's rack 3.2.6 / concurrent-ruby 1.3.7, which are ahead of master. Validated on CI via draft PR #741: the removal itself produced zero failures (lint, frontend, docker and 4 of 6 backend shards green; the one remaining failure is a pre-existing OIDC login-button spec that the removal cannot touch — it changes no auth file and the oidc config is byte-identical). Tracking: v2-cwy5 (split epic) — .1 dry-run and .3 classify closed, .4 removal this commit. Signed-off-by: Will Dower <will@dower.dev>
Blueprint has_many collections had no ORDER BY, so identical API reads could return rows in different order (root cause of the recurring 'jbuilder cache' consistency flake — a real production bug, not a test bug). Postgres row order without ORDER BY is arbitrary; a non-unique key still leaves ties non-deterministic, so every ordering ends in :id (total-order tiebreaker, per keyset-pagination practice). - BaseRule.canonical_order scope + canonical_sort in-memory sort: (version, rule_id, id) — version is the STIG/SRG-ID, DISA's published document order (verified against DISA XCCDF seeds + parser mapping) - ComponentBlueprint :show/:editor rules, StigBlueprint stig_rules, SrgBlueprint srg_rules use canonical_sort (zero re-query on the eager-loaded collection — no N+1, verified by query-count probe) - ApplicationRecord.sorted_by_id / .chronological shared sorters for the rest of the class: reviews, memberships, satisfies/satisfied_by, checks, descriptions, additional_answers, project collections - Component#reviews both paths + histories get :id tiebreak; available_components and access_requests order their queries - No global association mutation — all 34 existing .rules callers and the exports are untouched; regression specs assert sorted order with records created out of order Authored by: Aaron Lippold<lippold@gmail.com>
Two determinism fixes in the settings test surface: - settings_defaults_spec is now the single behavioral defaults contract: opt-in booleans (ldap/oidc/smtp/slack/banner/consent) asserted via with_settings_env unsetting the controlling env var (no skip-guards — a test that passes by skipping asserts nothing); password.min_* values asserted deterministically - settings_initializer_consistency_spec deleted: it asserted default VALUES by regex-matching config/settings.rb source text, which broke on a behavior-preserving refactor (18 false failures) while proving nothing about effective values. All 18 values now have behavioral homes (15 here, 3 lockout values in lockout_settings_spec) - consent_spec restores Settings via one top-level reload! after-hook, replacing six piecemeal restores (one leaked a String ttl where boot coerces Integer — the source of a cross-spec flake) Authored by: Aaron Lippold<lippold@gmail.com>
DISA-style XCCDF in both 1.1.4 (current) and 1.2 from one content model. Verified delta is namespace + object-ID encoding only (xccdf_[owner]_[type]_[name] pattern in 1.2); centralized in a VersionProfile strategy with format_id as the single ID-mapping source of truth. Bundles: structured version/status dates alongside DISA plain-text, satisfied-by as <requires idref> in both versions, one <ident> per CCI (conformance fix), multi-parent as repeated <reference>/<ident> (no schema change needed). Part of the SRG authoring initiative; same branch, possibly its own epic. Authored by: Aaron Lippold<lippold@gmail.com>
Brings in Will's SRG ADR v6 (generalized-XCCDF scoping + user workflow), the component sync/merge engine removal (split to feat/component-sync-merge), and latest master (#736, #740). Conflict resolved: spec/lib/tasks/sync_spec.rb deleted with the engine it tested. Authored by: Aaron Lippold<lippold@gmail.com>
Rule deletion soft-deletes via update_columns, which bypasses the counter_cache callbacks (they only track hard create/destroy) — the model's own recount is additionally gated to clone flows. Every rule deletion therefore left the component's rules_count stale (inflated controls badge). Recount inside the destroy transaction via Component.reset_counters, which counts through the association so Rule's default scope already excludes deleted rows. Authored by: Aaron Lippold<lippold@gmail.com>
Resolutions (Aaron, 2026-07-12) for Will's v6 review items 4-8: - Item 4: per-profile status vocabularies (authored rows only; catalog imports keep legacy inclusion; audit found zero substring status matching in production — prevention, not rescue) - Item 5: REDESIGN — relocation is a first-class record (requirement_relocations, pending=marker/executed=move) + soft-delete tombstone; the Moved status is removed entirely; reviews preserved frozen; history-only visibility. Revises §0.2/§0.3. - Item 6: delete-and-recreate accepted for v1, mitigated by the new §2.1.5 pre-delete backup design (existing JSON archive, bytea storage, lazy purge, 60-day default retention setting) - Item 7: copy-on-release via existing amoeba idiom + type-scoped authored-XOR-catalog CHECK — dual-link structurally impossible - Item 8: primary parent = based_on, user-designated (default first selected), validated as member of declared parents Authored by: Aaron Lippold<lippold@gmail.com>
Six sections still described the removed Moved-status design. R1 and the 2.1.1 gating table now state relocation is a record not a status; migration Phase 4 is the requirement_relocations table (no base_rules columns, no new status value); testing invariants list the one-way lifecycle (unique pending, executed implies soft-deleted source, immutable executed records, dangling-target nullify); phasing 4/5 describe pending-record intake and the executor transaction. Authored by: Aaron Lippold<lippold@gmail.com>
Pre-carding adversarial premise review, every finding verified against the code. Root cause of all five: SrgRule inherits BaseRule, not Rule — the deleted_at default scope, counter_cache, and amoeba clone idiom live on Rule only. SrgRule gains its own deleted_at scope; SRG recount is kind-routed (reset_counters(:rules) zeroes SRG components); release copy is new machinery (SrgRule's amoeba block converts to Rule); authored import cannot reuse from_mapping (sets srg FK, violates XOR); duplicate(new_srg_id:) must reconcile the parent join with based_on. No decision changes. Will approved v7 2026-07-13. Authored by: Aaron Lippold<lippold@gmail.com>
Same pre-carding review pass, all findings code-verified. Relocation FK deletion policy: source records dependent-destroy with the source rule (history survives in pre-delete backup + audit trail), target stays nullify — an unspecified FK would block component delete-and-recreate. XOR CHECK co-migrated with the optional srg FK plus a pre-flight both-NULL/both-set guard. component_id already exists on base_rules; only derived_from_srg_rule_id is new. based_on NOT NULL is an ordered Phase 3 step (column is nullable today with no presence validation), enforced before the parent-set invariant. Authored by: Aaron Lippold<lippold@gmail.com>
Same pre-carding review pass, all findings code-verified. Every comment/triage/lock/release scoping query routes through the Rule STI association and structurally excludes authored SrgRules — empty triage table, dashboard aggregates missing, lock-all locks nothing so an SRG component could never release; all sites migrate to Component#requirements (Phase 2, explicit). Authored requirements get a dedicated editor blueprint (RuleBlueprint calls Rule-only methods). Currency, backup, and the revision guard iterate all declared parents instead of based_on only. SRG export is a new mode plus authored fetch plus helper guard, not a filter swap. Authored by: Aaron Lippold<lippold@gmail.com>
Final review of the four-review pre-carding pass. The section-0 decision row no longer claims tombstoning rides an existing default scope; 2.1.3 and section 5 no longer contradict the v7.3 scoping migration; the live walkthrough is owned by Phase 7 with scoped partials for Phases 2-3 and the v7.3 regressions pinned as specs; Phase-5 no-schema gap stated; cross-ADR export sequencing pinned in both ADRs (dual-XCCDF rebuild first, SRG mode layers on, SRG owns the authored fetch fix and all three rule.satisfies guards); glossary for document_type/profile/kind vs STI type; target_technology_token rename. Review pass complete — ready to card. Authored by: Aaron Lippold<lippold@gmail.com>
Board review F1: BackupSerializer#rules_collection fetches component.rules, the same Rule-association-empty bug class the v7.3 collision review caught in export but never applied to backups. An SRG component's pre-delete backup would archive zero requirements, reviews, and comments, silently voiding the 2.1.4 remedy. Added to the section-8 scoping-site migration list. Authored by: Aaron Lippold<lippold@gmail.com>
Child rules covered by a parent now show it everywhere: the shared SatisfiedByIndicator emits a navigate event (Pinia coupling removed so the benchmarks pack can render it), UnifiedRuleForm routes navigation to the rule selection store, the sidebar icon tooltip names every parent, the triage rule panel renders the indicator with an editor deep link per parent, and released-component RuleDetails shows a read-only indicator. Triage needed the backend chain: satisfied_by (id, rule_id, component_prefix) serialized in rule_content with its preload, a new RuleContent OpenAPI schema wired into CommentRow, and a contract test pinning the shape. Live-verified in both themes at three container widths. Authored by: Aaron Lippold<lippold@gmail.com>
CommentsByRule adopts the shared CommentItem (checkbox, posted-on note, and triage button live in slots; grouped layout unchanged), and CommentDedupBanner becomes CommentList's first consumer — the container gains rule/type scope filters, a display cap, a header scoped slot, and in-place row patching for optimistic reactions. The wire-row normalizer moves from the comments store to utils/normalizeComment (one source of truth, no Pinia coupling in presentational components), and the spec follows the component to containers/. Fixes shipped along the way: CommentItem rendered a second initials avatar (CommentAuthorLine gains show-badge) and a second timestamp in its default body; CommentList now always sends an explicit triage_status because the server defaults omissions to pending, which silently hid triaged history; dedup dimming only applies when at least one row matches the composer's section and de-emphasizes at the 0.85 accessibility floor instead of a hover-to-read 0.45 overlay. Authored by: Aaron Lippold<lippold@gmail.com>
Phase 1 of SRG Component authoring (ADR adr-srg-component-authoring.md sections 3, 6, 8.2, 12.1). Zero behavior change for existing records — full suite green untouched is the faithfulness proof. - components.document_type discriminator (default stig, backfilled, immutable after create) - derived_from_srg_rule_id self-referential FK (indexed, nullify) - Type-scoped authored-XOR-catalog CHECK co-shipped with the optional catalog parent, preceded by a pre-flight guard (0 violations) - SrgRule soft-delete default scope (on SrgRule only, never BaseRule) - Kind-routed Component#requirements / #requirements_count (SRG = live scoped count, never rules_count) - VulcanAuditable wiring for authored SrgRules (Rule parity; bulk catalog import stays audit-free) - Fix: components with soft-deleted rules could not be destroyed (FK violation) — destroy cascade now runs through BaseRule so tombstones of every STI type are reached - Fix: rules factory wrote orphan catalog SrgRules (NULL srg FK) through the id-less based_on select scope — routes through the FK Authored by: Aaron Lippold<lippold@gmail.com>
Reviewer advisory from the comment-system reference implementation: $set is required for array-index reactivity under Vue 2 and becomes a plain assignment in Vue 3 — same documented debt as this component's render(h) / $scopedSlots usage. Authored by: Aaron Lippold<lippold@gmail.com>
|
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
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
Major feature branch: 247 commits, 668 files changed, 53K insertions. Builds on the triage split-pane workspace (PR #717 follow-up) and adds infrastructure standardization, API authentication, upgrade system, seed data, and documentation.
Core Features (earlier commits on this branch)
New in this push
{title, message: Array, variant}across all mutation endpointsvulcan_development/test/productionnaming, test DB name hardcoded to prevent collision, port registry (5435)drop_invalid_reviewsnow deletes children before parents (RESTRICT safety)Bug fixes
Test plan
bin/parallel_rspec spec/— full backend suiteyarn test:unit— Vue component testsyarn lint:ci— 0 warningsbundle exec rubocop— 0 offensesyarn openapi:bundle && yarn openapi:lint— 0 errorsbundle exec rspec spec/contracts/— 107 contract testsyarn docs:build— VitePress builds cleanbundle exec brakeman— 0 warningsAuthored by: Aaron Lippoldlippold@gmail.com