Skip to content

docs: Add the Action without confusing exit status and verdict (ENG-1349) - #8

Merged
Istvan Docsa (istvandocsa) merged 3 commits into
mainfrom
istvandocsa/eng-1349-docs-pages-simulating-a-change-before-apply-and-guardrails
Oct 26, 2026
Merged

Istvan Docsa (istvandocsa) merged 3 commits into
mainfrom
istvandocsa/eng-1349-docs-pages-simulating-a-change-before-apply-and-guardrails

Conversation

@istvandocsa

@istvandocsa Istvan Docsa (istvandocsa) commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Simulation and coding guidance needed executable paths and qualified results. Adds CLI/Action submission, workflow-versus-stored identity recovery, independent local/server coverage gates and untrusted self-reported guardrails.

Indexed lint passes for all 13 source components; live composition: 522 pages, zero dangling/missing/self links; browser regression47/47; all37 changed customer guides render at wide/narrow widths with no page overflow; SQL examples run through local MCP and Explorer (8/0/8 rows);23 additional CLI help/flag parses pass. Docs-tool Go suite,32 local fragment-pipeline tests and targeted onboarding alias/operation checks pass for their applicable changes.

Fresh staging-simulated Floci acceptance is still open. No prompt/dry-run/local rendering is presented as a certified fresh-install run. Simulator-owned auxiliary fragments require release packaging; the service concept includes its own usable submission path. Provider IAM, real OAuth/message delivery and cloud mutation tasks were source-checked, not executed against customer clouds.

Stacked on main. Cold operator/completeness review plus one Codex sweep and one final confirmation are recorded in the PR evidence comment. No merges performed.

@istvandocsa

Istvan Docsa (istvandocsa) commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor Author

Documentation review evidence — final candidate 97549ad06dab06d16002718140506e4197528ad1.

Simulation and coding guidance needed executable paths and qualified results. Adds CLI/Action submission, workflow-versus-stored identity recovery, independent local/server coverage gates and untrusted self-reported guardrails.

Indexed lint passes for all 13 source components; live composition: 522 pages, zero dangling/missing/self links; browser regression47/47; all37 changed customer guides render at wide/narrow widths with no page overflow; SQL examples run through local MCP and Explorer (8/0/8 rows);23 additional CLI help/flag parses pass. Docs-tool Go suite,32 local fragment-pipeline tests and targeted onboarding alias/operation checks pass for their applicable changes.

Fresh staging-simulated Floci acceptance is still open. No prompt/dry-run/local rendering is presented as a certified fresh-install run. Simulator-owned auxiliary fragments require release packaging; the service concept includes its own usable submission path. Provider IAM, real OAuth/message delivery and cloud mutation tasks were source-checked, not executed against customer clouds.

Every cold-review and sweep finding was corrected or explicitly bounded before the final confirmation. Confirmation verdict MERGE is a review result, not merge authorization. Reports mentioning expired GitHub auth describe their isolated sandbox; real GitHub checks are refreshed separately.

Reviewed head 97549ad06dab06d16002718140506e4197528ad1; final scoped diff SHA-256 9690bc5ec515daad0d868865b7f3f40ef2c6de2c641fe8e32d630b671c55744e. Final parent merges preserve the reviewed scoped diff.

Codex sweep
  • Major — add-the-github-action.md:43, also agent guide line 7: require the submitting Actions job alongside the simulation check. With strict: "true", the CLI uploads supported resources before failing for local skips (source). The submitted subset can pass, leaving a green simulation check and a failed Actions job. Requiring only the documented check therefore permits merging despite incomplete local coverage. Smallest fix: tell both readers to require the simulate job and the configured simulation check, explaining that they enforce different conditions.

Docs lint and all six PR CI checks pass. No files changed.

MERGE AFTER FIXES

Codex final confirmation

No critical or major findings remain in this isolated follow-up. Indexed documentation lint and diff checks passed. CI could not be rechecked because GitHub authorization expired.

MERGE

eng1349-simulation-review.md

ENG-1349 cold simulation review

One bounded P2 operational-completeness finding; no major false-pass/apply claim or unsupported MCP path found. Source-only review; no live simulation, GitHub check, cloud provisioning or UI execution performed.

Scope: service concepts/simulations pair/nav; linro-simulator how-to/simulate-locally pair/nav; simulation-github-action how-to/add-the-github-action pair/nav and the narrow README apply/exit correction; simulator-service how-to/follow-a-simulation pair/nav. Read the new repositories' AGENTS in full, including simulator unconditional rules; reused metarepo/service/frontend/CLI/IDL/SDK guidance previously read. Used /tmp/docs-integrations-simulation-facts.md as a discovery map and verified the claims below against primary implementation. Paths relative to /home/coder/linro-io.

Finding — P2: distinguish the workflow ID from the stored result ID in recovery instructions

simulator-service/docs/product/how-to/follow-a-simulation.agent.md:3–7 starts from the “canonical returned ID”, explains replica-safe workflow lookup, then directs stored-result/UI inspection without explaining that the IDs differ. service/docs/product/concepts/simulations.agent.md:5 lists the ordinary reads but delegates canonical-name discovery. Neither supplies the missing correlation step for the emphasized client-timeout recovery task.

Evidence:

  • simulator-service/internal/simulation/service.go:168–180 creates a simulation UUID and uses it as Temporal workflow ID; :205–213 returns that ID only on the polling path. GetSimulationStatus describes that workflow (:320).
  • service/internal/simulation/store_runs.go:27–46 generates a separate UUIDv7 for simulation.runs.id. simulator-service/internal/workflow/workflow_simulation.go:145–163 receives and retains this stored run ID after PersistSimulationStart; it is not the initiation response's ID.
  • service/internal/service/simulation/service.go:122–151 looks up GetSimulationRun by simulation.runs.id; :167 returns that ID as name. Resource/check reads take the same stored run ID as parent (:224–239, :341–353). Therefore the returned workflow UUID must not be substituted into these reads or /simulations/:id.
  • service/internal/simulation/store_runs.go:512 puts the stored run ID in the completed summary's /simulations/ link. Before completion, ListSimulationRuns exposes label/source/owner/repo/timestamps (:73–86, :99–118); workflow logs record the generated run_id (workflow_simulation.go:163). The inspected persistence input contains source/label/bucket/key, not a workflow-ID-to-run-ID lookup field.

Smallest fix: add an agent-side distinction and recovery rule: use the initiation ID only for listener status; use the summary link or a positively correlated stored run's name for UI/ordinary reads. For a still-running or ambiguous run, correlate label/source/time or have the operator inspect workflow logs; do not guess that the UUIDs are interchangeable or submit a duplicate. Retain both IDs for support. This is an omitted recovery detail, not an existing explicit wrong CLI command.

Confirmed contracts and retained technical detail

  • MCP gap is correctly documented. service/internal/mcp/server.go:253–258 explicitly leaves SimulationService unregistered. Ordinary Connect reads are mounted at service/internal/service/service.go:734. cli/internal/cli/root.go:89–90 registers simulation initiation and read commands. No guide invents gateway operations for them. Initiation/status listener and business result API are correctly distinguished.
  • Verdict versus exit is correct. simulator-service/internal/simulation/service.go:343–366 returns COMPLETED for both pass and violations, with summary; unreadable workflow result returns FAILED. linro-simulator/internal/emit/grpc.go:180–185 returns success after out-of-band submission, and :249–254 returns nil on COMPLETED. The README's opening no longer guarantees a passing apply and its cli-mode paragraph explicitly requires a result-aware gate. Product pages consistently reject exit-zero/green-step compliance inference.
  • Local strict is bounded correctly. linro-simulator/internal/cli/root.go:52 defaults strict false; internal/cli/run.go:180–208 counts local skips, emits first, then returns StrictCoverageError. It does not certify server coverage. service/internal/simulation/check_runner.go:54–73 explains per-check errors are skipped rather than failing evaluation; service/internal/simulation/store_runs.go:534–535 adds a skipped-check warning to the summary. Keep both local and server coverage evidence agent-side. Optional clarification: strict failure can occur after work was submitted, so it is not evidence that no run exists.
  • CLI task and compatibility: root.go:35–55 supplies environment/token/TLS/scope/integration flags and five-minute polling default. internal/cli/simulate.go:53–56 uses repeated StringArray plan/preview flags; :107–132 rejects duplicate paths. Mixed sources produce one payload, not independent verdicts. internal/compat/release.go:43 retains floor 2026.08.2; servicefloor.go:25–40 retains fallback floor 2026.10.1/service v0.3.88. Product text appropriately asks for actual compatible versions and retains fail-closed unknown/unorderable-release behavior, provider scope distinctions and Pulumi --show-sames. A generic version floor is not evidence of every provider's full protocol/coverage compatibility.
  • Submission authentication: simulator-service/internal/auth/auth.go:60–89 introspects active PATs and accepts SIMULATIONS/API scope; missing/inactive token is UNAUTHENTICATED, wrong scope PERMISSION_DENIED and token-store failure UNAVAILABLE. This listener does not implement the business API's role/access-level gates. Product agent text correctly avoids importing a blanket admin/operator or PREPARE requirement. Stored-result API access remains a separate authorization surface.
  • Transport/timing: simulator-service/internal/s3/s3.go:103 gives upload URL two-minute TTL. linro-simulator/internal/emit/grpc.go:670–680 uploads application/octet-stream with a 30-second timeout; :225–243 applies minimum polling interval five seconds and ten-second status-request timeout; :266–270 reports timeout with work still running. simulator-service/cmd/server/main.go:40–43 sets independent cleanup1h/watchdog30m/stuck1h defaults. Keep these distinctions, and avoid calling the five-second interval a minimum total client wait.
  • Action template: action.yml:146–164 defaults label linro-simulate, integration github-ci and strict false; :328 supplies token through environment; :378–398 processes paths; :400–459 forwards source/scope/strict flags and invokes the CLI. Product workflow explicitly assumes prior plan production and intentionally uses operator-reviewed Action/CLI placeholders; neither is a defect. Its token secret, unique label, OIDC/direct-App distinction, result-aware gate and nonoverlapping-source guidance should remain. No published-version availability or real PR-check creation was certified.
  • UI/source-backed navigation: app-frontend/src/router/register.tsx:363–370 registers /simulations and /simulations/:id. simulations/index.tsx:306 tells empty-state users to run CI/local; it is not an upload wizard. simulations/detail.tsx:253–262 has a collapsed Summary; :445–475 has Evaluated Checks and explicit no-checks text; :375–405 has Resource Providers, whose rows expand via :562–580 to per-resource Action, Status, Failing Checks (:625–634). Optional small human-step improvement: name Summary and the expandable Resource Providers row in both inspection tasks so users can expose the evidence being requested. Skipped-check warning is in Summary; the check list is not a complete list of failed-to-execute checks.
  • Nav and rendition boundaries: service/docs/product/nav.yaml:61 includes the concept; each of the three component nav files includes its how-to. Human pages keep tasks short while agent renditions retain transport/auth, versions/scopes, coverage and exit semantics. The service concept's guardrail link correctly keeps advice/self-report separate from simulation evidence. tf-simulator formal retirement is not asserted.

Validation and remaining boundaries

Service docs lint with the existing /tmp/docs-enrichment-index.json exited 0, with no diagnostics for the reviewed page. The three component lints each exited 1 solely because that supplied index lacks the newly added service/concepts/simulations page (both renditions in each component). Treat this as an index/publication dependency: lint the coordinated content with a current combined index before release. No corrected cross-component lint result is claimed; no index file was written.

The fact map's old suggestion to use MCP gateway simulation reads is superseded by current server registration evidence; the product docs correctly reject it. Local init, dry-run, static lint or another local smoke test does not certify the separate staging-simulated/Floci acceptance requirement. Published marketplace artifacts, installation connectivity, real GitHub check/branch-protection behavior and live Explorer/Simulations execution were not exercised here.

Only /tmp/eng1349-simulation-review.md was written. No repository content edits, Git/Linear mutations, CI changes, cloud calls, builds or tests.

eng1349-simulation-fixed-review.md

ENG-1349 bounded fixed review

No remaining findings within the ID-recovery correction. The prior P2 operational-completeness finding is closed; no further fix is required in this scope.

Reviewed service/docs/product/concepts/simulations.agent.md:15 and simulator-service/docs/product/how-to/follow-a-simulation.agent.md:11 against primary local source. Reused repository instructions already read in full during the cold review. Paths below are relative to /home/coder/linro-io.

  • Both corrections explicitly distinguish the initiation workflow UUID, used only for listener GetSimulationStatus, from the separately generated stored simulation run UUID used by /simulations/:id and ordinary result reads. Source: simulator-service/internal/simulation/service.go:168–180,205–213,310–323; service/internal/simulation/store_runs.go:27–46; service/internal/service/simulation/service.go:122–151,224–240,341–353.
  • Completed-summary recovery is correct: service/internal/simulation/store_runs.go:508–512 links /simulations/. Before completion, ListSimulationRuns exposes label/source/owner/repo/timestamps for positive correlation (service/internal/service/simulation/service.go:73–85), and workflow logs record run_id after persistence (simulator-service/internal/workflow/workflow_simulation.go:162–163). Neither correction promises a guaranteed direct workflow-to-stored-run lookup.
  • Both retain the required ambiguity handling: preserve both identities/evidence, never substitute UUIDs, and avoid duplicate submissions while correlation is unresolved. This supplies the missing recovery boundary without inventing an MCP operation or lookup API. The earlier “canonical returned ID” wording is now explicitly scoped by the added distinction.

Source-only confirmation. No live listener/result calls, UI execution, GitHub checks, tests or lint reruns. Prior cross-component index/publication and live acceptance boundaries remain unverified by this bounded pass; they are not reopened ID-content findings.

Only /tmp/eng1349-simulation-fixed-review.md was written. No repository, Git, Linear or cloud mutations.

eng1349-guardrails-review.md

ENG-1349 guardrails cold review

Verdict: one P2 agent-completeness correction and one P3 availability/recovery omission. Core lookup, trust-boundary and self-reporting claims match source. No simulation companion pages reviewed.

Scope: service/docs/product/concepts/guardrails{,.agent}.md and its nav entry. Service HEAD 6a1844f9, ENG-1349 branch; clean working tree when checked. Read-only; only this report was written for this review. No tool calls to lookup/report against a live installation, no Git/Linear/cloud mutations.

Reviewed hashes:

  • Human: c1c3570e403ea7661ee742dd88d0f9c27b9af890483a683e727a6584c61c330b.
  • Agent: d2a84a39b0491b2d3f7a657f5467b0906a359d3b9432301c9fe735144a52fbb6.

Paths below are relative to /home/coder/linro-io.

Findings

1. P2 — Define reporting outcomes and preserve the user's bypass decision

Docs: service/docs/product/concepts/guardrails.agent.md:7; related human task guardrails.md:20-23.

The agent rendition lists prevented|passed|overridden but omits their exact meanings. “Record only what the agent actually did” does not distinguish code that already complied from code changed because of guidance, or preserve that an override is the user's choice. This distinction is necessary for the human page's promise of honest reporting; it is also what the recorded prevention counts mean.

Source: service/internal/mcp/server.go:374-394 defines one report per returned guardrail and:

  • prevented: the agent changed code to satisfy it;
  • passed: code already satisfied it;
  • overridden: the user chose to bypass it.

The server's advertised workflow also says to call once per resource type and report once per guardrail, and to explain a critical/high conflict and seek explicit acknowledgment before proceeding (service/internal/mcp/server.go:423-427). That is a workflow instruction, not a server-enforced approval gate. The handler validates the enum and records the assertion; it does not inspect code or establish who authorized a bypass (service/internal/mcp/results.go:19-42,64-69).

Correction: spell out all three meanings in the technical rendition, require the same returned check identity and one report per guardrail after finalizing the code, and retain the same lookup/report ID for retries. Preserve the user-decision boundary for overridden; do not record a unilateral agent choice as a user-approved bypass. If describing the critical/high acknowledgment behavior, distinguish the advertised workflow from enforced authorization. Keep the existing independent-review/simulation and untrusted-text caveats.

2. P3 — Lookup availability does not imply result-reporting availability

Docs: service/docs/product/concepts/guardrails.agent.md:3,7; human known gap at guardrails.md:25.

The pair describes a lookup/report loop and mentions configured guidance generally, but does not preserve the independent reporting prerequisite or a fallback when only lookup is available.

Source: service/internal/service/service.go:901-909 creates GuardrailService when AI is configured, but creates ResultRecorder only when a ClickHouse connection exists. service/internal/mcp/server.go:340-368 registers list_guardrails first and returns before registering report_guardrail_result when its recorder is nil. Thus a supported configuration can expose guidance without the report tool.

Correction: agent preflight should inspect available tools and distinguish missing AI/lookup capability from missing result recorder/report capability. Human known gaps can briefly say that result reporting must also be enabled. When reporting is unavailable or fails, state that the outcome was not recorded; preserve the reviewed guidance and separate verification rather than claiming the loop completed or attempting an unrelated mutation to enable it.

Verified claims and exact evidence

Claim Implementation evidence / assessment
MCP transport/authentication; support excluded service/internal/auth/oidc.go:1082-1118: authenticated principal, support denied, credential scope MCP or API; ordinary session authority also passes.
Helpers do not inherit centralized RPC role/access-level gates service/internal/mcp/authz.go:49-62,209-217: both are helper tools requiring authentication, not RPC procedure authorization. The doc correctly does not invent operator/PUBLISH requirements for reporting.
AI prerequisite service/internal/mcp/server.go:196-198; service/internal/service/service.go:901-909; reporting has the additional prerequisite in finding 2.
Exact lookup input fields/limits service/internal/mcp/guardrails.go:20-21,42-51: iac_provider matches ^[a-z0-9-]{1,63}$; resource_identifier matches ^[a-zA-Z0-9_\-.:]{1,256}$. These are lexical constraints, not an enumerated simulator-support list.
Type mapping service/internal/guardrail/service.go:119-154: cache first, resolver on miss, caches empty mappings, no resolved types -> empty lookup. service/internal/guardrail/resolver.go:116-153,207-214 calls the classifier and retains only catalog names. This is guidance selection, not plan execution.
Which checks supply guidance service/internal/guardrail/service.go:51-76: enabled, undeleted checks, non-null/nonempty guardrail, overlapping referenced resource/component types. No claim of complete security coverage is warranted.
Empty result service/internal/mcp/guardrails.go:108-109 returns “No guardrails apply to this resource type.” Docs correctly qualify this as possibly unresolved/no authored guidance, not a pass. An empty lookup returns no lookup ID.
Lookup failure service/internal/mcp/guardrails.go:91-98: tool error asks to retry; failure must not be reclassified as an empty successful lookup.
Trust boundary service/internal/mcp/guardrails.go:132-141 concatenates stored title/check/guardrail text into the response, including guardrail body verbatim. Docs correctly treat it as untrusted reference data and make no implemented injection-prevention guarantee. Input/classifier prompting at service/internal/guardrail/resolver.go:121-129 does not sanitize returned check text.
Reporting input names and outcomes service/internal/mcp/results.go:19-42: iac_provider, resource_identifier, check, outcome; valid outcomes exactly prevented, passed, overridden. Same lexical provider/type rules, nonempty check required. Definitions are missing from the guide as finding 1.
Retry identity service/internal/mcp/guardrails.go:112-129 mints a UUIDv7 lookup_id for nonempty guidance and asks to echo it as report_id. service/internal/mcp/results.go:44-61 rejects malformed supplied UUIDs, allowing retry with the original identity. Docs preserve this distinction.
Reporting is an assertion, not verification service/internal/guardrail/results.go:52-81 resolves check identity and inserts outcome/provider/type/report ID; no code scan, plan evaluation or cloud action. The report handler returns a tool error if recording fails (service/internal/mcp/results.go:64-69).
UI/CLI gap No dedicated route/menu/CLI root registration for this helper pair; app-frontend/src/views/admin/menu.ts, app-frontend/src/router/register.tsx, cli/internal/cli/root.go:73-95. The documented absence is appropriate.
Nav/reference validity service/docs/product/nav.yaml:52-56 includes concepts/guardrails under Act. Both covers tool names agree with registration (service/internal/mcp/server.go:342,371); checks link resolves. Simulation workflow wording is generic and its future companion pages are explicitly out of this review.

House contract, parity and limits

Human rendition uses an operator prompt and plain explanations, no JSON/field/enum table. It explicitly states UI/CLI gaps instead of inventing task paths. Both renditions preserve enabled-check guidance versus simulation, empty-result uncertainty, untrusted output, authority boundaries and self-reporting versus attestation. No literal human instruction was lost in the agent rendition; findings concern omitted executable outcome semantics and capability recovery.

Body typed-reference parity passes; service docs lint with /tmp/docs-enrichment-index.json exits 0 with only the four existing size warnings elsewhere. No tests were rerun for this documentation-only read review. No live MCP availability, report persistence, external classifier behavior or simulation was exercised. The agent page's assertion about ENG-1121's current issue status was not independently verified; the underlying verbatim-output implementation gap was verified directly.

eng1349-guardrails-fixed-review.md

ENG-1349 guardrails fixed review

Verdict: both prior findings addressed; no remaining actionable issue found in this bounded fixed review.

Reviewed service/docs/product/concepts/guardrails{,.agent}.md at service HEAD 3c0390fb, including agent fix 50ec5950 and the subsequent committed human known-gap update. Working tree was clean. Scope was the two findings in /tmp/eng1349-guardrails-review.md; no tutorial or simulation companion review, no live tool calls, and no mutations except this report.

Prior finding Verified fix and source evidence
P2 outcome semantics and user decision Agent lines 7 and 11 now define prevented = code changed to satisfy guidance, passed = already satisfied, overridden = user chose bypass. They require one lookup per type, one report per returned check after finalization, the returned identity and stable retry ID, and distinguish workflow acknowledgment from enforced approval. Matches service/internal/mcp/server.go:374-398,423-427; service/internal/mcp/guardrails.go:112-129; service/internal/mcp/results.go:19-61. Reporting remains explicitly an assertion, not a scan or proof of consent.
P3 independent reporting availability/recovery Agent line 13 separately preflights lookup/reporting; human line 25 now states that reporting must be enabled separately and an unavailable/failed report has not been recorded. Matches AI/recorder wiring at service/internal/service/service.go:901-909, nil-recorder registration branch at service/internal/mcp/server.go:367-371, and report failure at service/internal/mcp/results.go:64-66. The fallback retains guidance and separate verification without claiming a completed loop.

Both renditions retain untrusted-text, empty-result uncertainty and independent-verification caveats. Body typed-reference parity passes. Service docs lint with /tmp/docs-enrichment-index.json exits 0, with only four existing agent-size warnings elsewhere.

Validation was source/contract inspection and lint; live capability discovery, persistence and retries were not exercised. ENG-1121 issue status remains outside this source-only confirmation.

docs-integrations-simulation-facts.md

ENG-1348 / ENG-1349 — primary-source map

Paths relative to /home/coder/linro-io. Current working-tree evidence, not a released-version assertion. Observed HEADs: linro-simulator 5e15736, simulation-github-action 2b6d519, integration-webhook 751293c (uncommitted changes may also be present). Read both tickets and ENG-1121 through Linear. Reused shared/service/frontend/CLI guidance; read new integration-webhook, integration-sdk, integration-slack, simulator-service, linro-simulator (including its unconditional rule files), simulation-github-action, tf-simulator, regression-test and local-dev instructions. No product drafts, test execution, deployments, Git/Linear changes or live provider calls. Only this report written.

Current review addendum — 2026-10-25

This pre-existing source map is retained as an earlier gathering, with integrations facts corrected during the ENG-1348 cold review. The earlier process notes and simulator/Action/deployment sections describe that gathering, not fresh execution in this pass. This pass made no Linear calls, deployments, provider calls or repository mutations. Simulation executable flags and external deployment assumptions were not independently revalidated here; new service-side evidence is below. See /tmp/eng1348-review.md for content findings.

  • Correction: security-log CLI gap. Current cli/internal/cli/root.go:73 registers AuditService, IntegrationService and Simulation services, but not AuthAdminService. Manual auth contains login/logout/status/whoami only (cli/auth/auth.go:29, :41). Therefore linro auth-admin list-user-events is not a valid documented current path. AuthAdmin is also absent from MCP registration (service/internal/mcp/server.go:260); use the authorized Connect API, registered at service/internal/service/service.go:859. GetUserEvent and ListUserEvents are admin reads (:160). Generated API stubs alone do not establish CLI/MCP registration.
  • Tests: enabled subscription required; optional UUID request_id returns the same captured test on retry (service/internal/integrations/deliveries.go:241, :244). Response is TestSubscriptionResponse.delivery, not proof of receipt. Test budget is three attempts (:298); ordinary captured events default ten (service/internal/integrations/capture.go:26). A test directly queues its synthetic payload, bypassing event matching; verify a representative real routed event separately.
  • Retries retain captured destination/config. Capture serializes subscription config into immutable delivery payload (service/internal/integrations/capture.go:80). Retry resets state/attempt count/cycle/error fields without rebuilding payload (service/internal/integrations/deliveries.go:367). Editing a route's channel or event selection does not retarget/refilter an already captured delivery. New test/new events verify new configuration. Retrying a captured message is an intentional resend and can duplicate a provider side effect.
  • Actual route task: Add channel route for Slack (app-frontend/src/views/admin/linro/integrations/detail.tsx:361), destination/event/resource selection, then Start sending for a new route or Save changes for an edit (RouteDrawer.tsx:771, same directory). Route creation offers a test-after-save checkbox (:745). Saves apply immediately and can succeed only in part across create/update/delete operations (:399); reopen/read persisted subscriptions before retrying, rather than duplicate the whole route. Existing routes show On / Off (RoutesSection.tsx:136, same directory). Queued message UI label is Waiting (app-frontend/src/dal/integrations.ts:970).
  • Event detail is not a delivery view. app-frontend/src/views/admin/linro/audit/event-detail.tsx:132 renders event metadata/subjects and :256 Raw Event; it does not fetch/list related deliveries. Use the integration's recent messages or AuditService List/GetDeliveries. List filter supports state/test/error_code/connection, not an arbitrary event-name expression (service/internal/integrations/deliveries.go:74). Page and correlate returned identity fields rather than invent unsupported joins/filters.
  • Confirmed legacy webhook boundary: declaration protocol1 (integration-webhook/internal/plugin/plugin.go:14) is rejected by service protocol2/runtime-level1–2 validator (service/internal/integration/validate.go:18, :35, :42). Handler outcomes, HTTPS-only config and body-HMAC are legacy behavior only (integration-webhook/internal/webhook/handler.go:58, internal/webhook/config.go:21, internal/webhook/signer.go:17); they do not establish current installability.
  • Service-side simulation evidence refreshed: result reads default20/max100 rows with offset pagination (service/internal/service/simulation/service.go:26); evaluation counts failed checks as skipped without failing the whole evaluation (service/internal/simulation/check_runner.go:56, :70, :111). Result completion and policy clearance remain distinct. Other simulator facts below retain their earlier-snapshot status.

Supported-path decisions

Question Evidence / conclusion
Which simulator should the docs use? linro-simulator is the declared multi-IaC successor (linro-simulator/README.md:9). The current Action explicitly downloads its marketplace artifact and invokes it (simulation-github-action/action.yml:244, :276, :459). Document this CLI and Action. tf-simulator/README.md:23 still describes the old linro-tf-simulator/simulator-github-app path. Formal retirement/archival was not established; do not equate presence of that old checkout with a currently supported customer path.
Is generic webhook setup equivalent to Slack? No. Webhook pins integration-sdk v0.1.1 (integration-webhook/go.mod:7), declares protocol 1 and only legacy violation/system handlers (integration-webhook/internal/plugin/plugin.go:14). Current service accepts only protocol 2 and runtime API levels 1–2 (service/internal/integration/validate.go:18, :32). Its install/upgrade validator rejects protocol 1. Document webhook as a legacy implementation/gap, not a runnable v2 guided-setup path.
Is the security log built? Yes: append-only hash-chained auth EventStore exists (service/internal/auth/events.go:73) with login success/failure, logout, PAT create/revoke, role changes and support-access/session events (:23). This is separate from routable audit-trail notifications, which Slack declares only as planned (integration-slack/internal/app/app.go:86); the router excludes planned topics (service/internal/audit/router.go:203). Respect ENG-1348's warning by not promising auth-event delivery over AUDIT_TRAIL.

ENG-1348 — integrations, subscriptions, delivery and logs

Area Actual contract / file:line
Routes /integrations, /integrations/:integrationId, /integrations/:integrationId/setup; connection deep links forward to the owning integration page. Event log /audit/events, detail /audit/events/:eventId; subscriptions /audit/subscriptions. app-frontend/src/router/register.tsx:382, :387, :392, :399, :163, :168, :173. Old /settings/integrations/* redirects at :406.
UI controls Provider-specific connection/destination nouns come from presentation metadata. Slack uses workspace/channel. Connect, Connect another …, Continue setup / Retry connection, Send test, recent-message Retry. app-frontend/src/views/admin/linro/integrations/detail.tsx:491, :536, :591; app-frontend/src/views/admin/linro/integrations/WorkspacesSection.tsx:212, :226; app-frontend/src/views/admin/linro/integrations/RoutesSection.tsx:169; app-frontend/src/views/admin/linro/integrations/RecentMessages.tsx:303. Message labels: Waiting, Sending, Retrying, Delivered, Failed, Cancelled (app-frontend/src/views/admin/linro/integrations/badges.tsx:19). The subscription list points users to the connection page to add a channel (app-frontend/src/views/admin/linro/audit/subscriptions.tsx:453).
Lifecycle Installation/version, connection authorization/health, subscription routing and individual delivery are distinct objects. Current connection states: pending_authorization, starting, ready, degraded, disconnected, deleting; authorization states tracked separately. service/internal/integrations/store.go:33. A recorded external wait is the admin's report of waiting, not provider approval; current help tells users to retry/grant after approval (app-frontend/src/views/admin/linro/integrations/WorkspacesSection.tsx:212).
Integration RPCs IntegrationService.ListIntegrations/GetIntegration, InstallIntegration, UpgradeIntegration, UninstallIntegration (the latter lifecycle calls return LROs); ListConnections/GetConnection/CreateConnection/UpdateConnection/DeleteConnection/AuthorizeConnection, ReportExternalWait/ClearExternalWait, SearchFieldOptions. idl/public/linro/integration/v1/integration_service.proto:27, :63, :76, :100, :113, :127, :137, :153, :165, :179, :188, :201. Connection deletion may be asynchronous; do not equate accepted operation with pod removal or external credential revocation.
Audit RPCs AuditService.ListSubscriptions/GetSubscription/CreateSubscription/UpdateSubscription/DeleteSubscription/UndeleteSubscription, batch variants, TestSubscription, ListDeliveries/GetDelivery/RetryDelivery, ListEvents/GetEvent/BatchGetEvents. idl/public/linro/audit/v1/audit_service.proto:20, :37, :46, :82, :88, :100, :111, :117, :124, :133.
MCP / CLI Discover Service.Method via search_operations; use call_read_operation, call_write_operation, call_destructive_operation according to classification. Ordinary CLI mappings: linro integration list-integrations/list-connections/create-connection/authorize-connection, linro audit list-subscriptions/create-subscription/test-subscription/list-deliveries/retry-delivery/list-events, No linro auth-admin family is registered in the current CLI. Names derive from RPC registration and kebab naming, not live execution here. Structured requests use --from-file; lists --paginate -o json. Sources: cli/internal/cli/root.go:77; service/internal/mcp/gateway_tools.go:28, :94, :118; previously traced aip-cli naming rule. OAuth browser consent remains a human/provider step, not a completed MCP task merely because an authorization URL was returned.
Roles Connection mutations, authorization, field-option search, external-wait writes, subscription mutations/test and RetryDelivery are admin-only. Reads stay authenticated baseline. AuthAdminService.GetUserEvent/ListUserEvents also admin-only. service/internal/service/service.go:157, :168, :181. UI write controls require admin and exclude support (app-frontend/src/views/admin/linro/integrations/index.tsx:215). Do not infer blanket admin-only install/upgrade/uninstall: those RPCs are absent from this per-procedure gate list.
PAT levels Additional to role: reads=READ; non-destructive integration/connection and audit writes default PREPARE; Delete/Remove/etc.=PUBLISH. service/internal/auth/access_level.go:57, :62. Thus current TestSubscription/RetryDelivery require an admin owner but are not automatically PUBLISH solely because they send. MCP scope SCOPE_MCP or SCOPE_API and support rejection remain as mapped earlier (service/internal/auth/oidc.go:1099, :1110).
Subscription shape Connection immutable; config validated by integration schema and must not hold secrets. Event selection uses event_filter, not removed topics/minimum_severity request fields. Empty filter means everything that integration actually renders. idl/public/linro/audit/v1/subscription.proto:46, :64, :100, :106. Scope selections: OR within a dimension, AND across dimensions, bounded by connection selections (:58). No labels.* in event_filter.
Update/disable traps Supplied stale etag → ABORTED; re-read before retrying. Disabling cancels queued deliveries; events arriving while disabled are not replayed later. idl/public/linro/audit/v1/subscription.proto:69, :110. Prefer read-modify-update with exact field mask, especially repeated selections.
Event pipeline Producers publish to River; router writes event history, matches subscriptions, then captures deterministic delivery rows. Event publication/queued capture is not provider delivery. service/internal/audit/publisher.go:17, :33; router.go:102, :149, :171; service/internal/integrations/capture.go:98.
Routing Requires rendered topic, valid event_filter and connection+subscription selections. Events lacking resource subjects only match when neither layer has selections. Planned/unreadable manifests are skipped. service/internal/audit/router.go:186, :198, :203. Stored topics violation_opened, violation_resolved, system, audit_trail, sync; enum names are distinct from stored strings (service/internal/audit/topics.go:8, :19). An enum existing is not proof of an active producer.
Delivery limits Ordinary captured delivery max attempts=10 (service/internal/integrations/capture.go:26); TestSubscription uses max_attempts=3 (service/internal/integrations/deliveries.go:298). Receive max32, lease10s–15m/default60s, long-poll20s, retry-delay cap1h, retry-cycle age ceiling24h, single throttle cap15m. Correlated group window5s, threshold5, max100. service/internal/integrations/queue.go:23. SDK retry backoff starts5s and doubles (integration-sdk/integration/outcome.go:143). Throttling pauses subscription without spending an attempt (service/internal/integrations/queue.go:614; SDK outcome contract outcome.go:70). Delivery is at least once, not a universal exactly-once provider side effect.
Delivery inspection/retry ListDeliveries newest first; only explicit order create_time desc. Its filter subset is state/test/error_code/connection joined by AND, not unrestricted AIP grammar. service/internal/integrations/deliveries.go:75, :131, :188. Retry only a failed delivery: FAILED_PRECONDITION otherwise; reset attempts/cycle, preserve delivery identity and original capture time (:336, :367, :371). Test returns a delivery to inspect, not proof of provider receipt (:274; idl/public/linro/audit/v1/audit_service.proto:358).
Security log boundary Auth EventStore hash chain/advisory-lock serialized writes, append-only protections, best-effort append (action may succeed if logging fails). service/internal/auth/events.go:73, :84. Read through admin GetUserEvent/ListUserEvents (idl/public/linro/auth/v1/auth_admin_service.proto:63). No dedicated complete security-log UI established; /account/support-access uses a filtered support-event subset (app-frontend/src/views/account/support-access/index.tsx:319). Do not describe /audit/events as every login/PAT/security action.

Slack vs legacy webhook

  • Slack is v2, active violation-opened/resolved/system/sync topics, audit-trail planned; OAuth scopes chat:write, channels:read, groups:read, channels:join, reactions:write. Subscription config channel ID, not display name. Primary definition integration-slack/internal/app/app.go:74; behavior/error details integration-slack/README.md:22 onward (README evidence, not independently replayed here). Picker pagination may produce empty pages plus continuation/retry delay; private channels require bot membership. Reconnect may be needed for newly requested scopes.
  • Legacy webhook URL must use https; config {url, headers?, secret?}; schema secret minimum16 characters (integration-webhook/internal/schema/subscription_config.json:7). Runtime validates https but does not itself enforce the schema's minimum key length (internal/webhook/config.go:21). Do not transplant these secret-bearing legacy config fields into a v2 subscription.
  • It POSTs protojson, Content-Type application/json, User-Agent linro-webhook/1.0; custom headers applied afterward; client timeout30s. 2xx succeeds, >=500/network errors retry, other final statuses fail. In particular HTTP429 is terminal in this handler, unlike Slack's throttled outcome. integration-webhook/internal/webhook/handler.go:18, :34, :57.
  • Optional signature header X-Linro-Signature: sha256=<hex HMAC-SHA256(secret, exact body)>; absent secret means unsigned. No timestamp/replay window provided by this signer. integration-webhook/internal/webhook/signer.go:9. Successful external reference has URL/title but no provider ID (handler.go:57). HTTPS/headers/signature are observed legacy behavior, not proof that current Linro can install it.

ENG-1349 — simulator, Action, guardrails

Area Source facts / limitations
Inputs/paths linro-simulator terraform simulate --plan …; OpenTofu sibling command; pulumi simulate --preview …; mixed simulate accepts repeatable --plan/--preview (not comma-separated), one combined payload/verdict. linro-simulator/internal/cli/simulate.go:14, :53; internal/cli/pulumi.go:17. Generate Pulumi previews with --show-sames --json; omitted unchanged siblings cannot be reconstructed. Dry-run prints assembled payload, submits nothing and proves no server result.
Coverage Linked plugins define supported bindings, not a universal IaC interpreter. AWS/GCP and Terraform OTC are linked; unsupported/unknown/redacted inputs may skip. --strict (default false) returns code3 for local synthesis skips; it is not a proof that all server resources/checks were evaluated. linro-simulator/internal/cli/run.go:204; report.go:12; internal/synth/ implementations and COVERAGE.md (generated snapshot). Inspect runtime coverage and installed versions.
Scope --account/--region for AWS; --project/--organization GCP fallbacks that reject contradictions; --domain OTC must exactly match connector spelling. Mock account/domain flags default false and generate identities that do not shadow real inventory. linro-simulator/internal/cli/root.go:45 through :53. Wrong but plausible scope can produce misleading new-resource results.
Transport/auth CLI -> simulator-service via Connect HTTP: InitiateSimulation, upload payload to presigned S3 URL, then poll GetSimulationStatus for CLI mode. PAT with SCOPE_SIMULATIONS or SCOPE_API; missing/inactive→UNAUTHENTICATED, missing scope→PERMISSION_DENIED, token-store failure→UNAVAILABLE. linro-simulator/internal/emit/grpc.go:137; simulator-service/internal/auth/auth.go:38, :70. This interceptor checks active token+scope; no separate operator/admin or READ/PREPARE/PUBLISH check is shown here. Do not assume central business-service role gates apply to this separate listener.
Defaults/flags --server / LINRO_SIMULATION_SERVER; --token / LINRO_SIMULATION_TOKEN (then stored token); TLS default, --ca-cert extra trust, --insecure only loopback; --poll-timeout default5m. linro-simulator/internal/cli/root.go:35 through :55; validation flags.go. init submits an empty initialization and polls; it verifies auth/transport/workflow, not check coverage (emit/grpc.go:191).
Compatibility Meta discovery must report a release; global floor2026.08.2; submissions carrying fallback columns need2026.10.1/service v0.3.88. Unknown or unorderable same-month stable-vs-dated releases fail closed. linro-simulator/internal/compat/release.go:43, :224; servicefloor.go:25; internal/cli/run.go fallback gate. .marketplace is the intended floor source; no claim that its older global floor closes every historical protocol gap.
Upload/poll Presigned upload TTL2m; upload Content-Type application/octet-stream; CLI upload timeout30s. Poll min5s, each status request timeout10s; client poll timeout leaves server simulation running. simulator-service/internal/s3/s3.go:96; linro-simulator/internal/emit/grpc.go:225, :267, :677. Server status via Temporal workflow ID survives replicas (simulator-service/internal/simulation/service.go:309).
What is evaluated Overlay rows shadow live identities and retain other live rows. Relevant checks run against overlay and live; new violations = sim minus live. Check execution failures are counted/skipped rather than failing entire evaluation. service/internal/simulation/overlay.go:59, :111; check_runner.go:54, :66, :108, :126. This neither applies a plan nor guarantees provider API acceptance, complete coverage or unchanged inventory while the run proceeds.
Verdict vs exit GetSimulationStatus returns COMPLETED for both pass and violations; caller examines summary. CLI returns nil/exit0 on COMPLETED; no polling ID (GitHub check delivery) also returns success after submission. simulator-service/internal/simulation/service.go:343, :359; linro-simulator/internal/emit/grpc.go:182, :250. Do not write “exit0 means compliant” or “passing sim guarantees passing apply.” Check Result, skipped resources/checks and server composition failures.
Cleanup defaults Watchdog30m, delayed schema cleanup1h, stuck-run threshold1h; configured env vars may override. simulator-service/cmd/server/main.go:40, :131, :145, :158. These are separate from CLI's5m wait and S3's2m upload TTL.
UI/read API /simulations, /simulations/:id; list is results/history, not an upload wizard. Empty view tells users to run CI/local. app-frontend/src/router/register.tsx:363, :368; app-frontend/src/views/admin/linro/simulations/index.tsx:306. SimulationService.ListSimulationRuns/GetSimulationRun/ListSimulationResourceResults/ListSimulationEvaluatedChecks on ordinary API (idl/public/linro/simulation/v1/sim_service.proto:15). Use MCP gateway read operations or linro simulation … for inspection; these differ from initiation listener/status API.
Action Marketplace CLI download supports Linux/macOS, AMD64/ARM64; current default binary v0.2.1, marketplace https://marketplace.linro.io. Inputs mirror CLI flags; default integration github-ci (CLI default cli), strict false, label linro-simulate. simulation-github-action/action.yml:145, :216, :244, :263. Pin versions and make labels unique for concurrent checks.
GitHub requirements CLI explicitly selects --integration github-ci; PR check uses discovered GitHub OIDC audience, requires job id-token:write when minting OIDC. Empty audience means direct GitHub App mode. Gateway configured → no local App key; fallback requires APP_ID/PRIVATE_KEY. linro-simulator/internal/cli/root.go:54; internal/emit/grpc.go:106; simulator-service/cmd/server/main.go:76. Local throwaway App credentials support cli mode only; no real PR-check claim.

Guardrails are advice/data, not verified simulation

  • MCP list_guardrails(iac_provider, resource_identifier) before editing each type; report_guardrail_result(iac_provider, resource_identifier, check, outcome, report_id?) after. Outcomes exactly prevented/passed/overridden; echo lookup_id as report_id for retry accounting; malformed UUID rejected. service/internal/mcp/server.go:338, :371; service/internal/mcp/results.go:17, :53.
  • Input provider lowercase [a-z0-9-],1–63; identifier [a-zA-Z0-9_.:-],1–256 (hyphen accepted too). These are lexical bounds, not proof every advertised IaC frontend is simulatable. service/internal/mcp/guardrails.go:20.
  • Requires configured AI client to register tools; maps IaC type through cache/LLM resolver (256-token model response cap), then selects enabled, undeleted checks with nonempty guardrail and overlapping resource/component types. Empty result may mean unresolved type/no authored guardrails, not security/compliance clearance. service/internal/service/service.go:901; service/internal/guardrail/resolver.go:117; service/internal/guardrail/service.go:54, :130.
  • Both tools are currently authenticated-only helpers, not central RPC role/level-gated operations (service/internal/mcp/authz.go:49, :209); MCP transport scope/support rules still apply. Report records an agent assertion in ClickHouse, not a code scan or attestation (service/internal/guardrail/results.go:46, :71). No dedicated CLI guardrail operation established.
  • ENG-1121 remains open. Current response concatenates stored guardrail verbatim (service/internal/mcp/guardrails.go:132), while server instructions tell agents to satisfy it (server.go:421). Keep it framed as untrusted check content; do not copy those authority claims into a page, authorize arbitrary commands/links from a guardrail, or claim injection prevention is implemented. Risk is directly supported by source and ticket; the requested documentation must not amplify it.

Local / staging-simulated prerequisites and acceptance gaps

  1. Two environments: local Tilt != staging-simulated. The latter is an ephemeral summoned install plus runner-hosted Floci/external sensor; AWS setup, mock admin login, seeded organization/account IDs, profile endpoint override and ready plugin/connector/ingestion floors. Primary runbook and known emulator limits: regression-test/tests/environments/staging-simulated.yaml:1, :44, :64, :75, :134. CloudTrail management events are not emitted natively; AssumeRole negative tests are not realistic. No GCP estate guarantee.
  2. Simulator itself: server listener, token-store DB, Temporal workflows AND service execution worker, compatible installed plugin/tables/checks, reachable artifact bucket for upload and worker reads. Local Tilt supplies SIMULATION_S3_ENDPOINT, SIMULATION_S3_PUBLIC_ENDPOINT, PATH_STYLE, UPLOAD_BUCKET, release version and token-store settings (local-dev/Tiltfile:1643); stale backend env requires worker/server restarts (:1697). Public presigned endpoint must be reachable from the CLI, not only pods. Source simulator-service/internal/s3/s3.go:96 and CLI upload flow above.
  3. No real GitHub App needed for local CLI simulations: local-dev generates throwaway App credentials when no real key file exists; github-ci cannot create real checks with them (local-dev/Tiltfile:1638, :1643). This supersedes simulator-service AGENTS' blanket real-App prerequisite for a local cli-mode smoke check. Gateway vs App deployment remains separate.
  4. Existing demo gate: local-tilt manifest simulator: {server: localhost:8090, cliVersion: v0.6.28} (regression-test/tests/environments/local-tilt.yaml:375). Set executable LINRO_SIMULATOR_BIN to that pin, use correct target PAT/auth fixture, require GCP plugin READY and init round trip; loopback plaintext only. tests/preflight/simulator-demo.setup.ts:18, :27, :40; tests/support/simulator-demo.ts:113, :131. The demo uses Terraform + bridged/native Pulumi GCP fixtures and checks actual server results, not just CLI exit0.
  5. Acceptance gap: simulator-demo is allowed only on local-tilt (regression-test/tests/bundles.yaml:152); staging-simulated default bundles are ui/ui-inventory/mcp/labels-contexts (:170) and its manifest has no simulator capability. Existing local GCP demo evidence does not satisfy ENG-1349's “run block against Floci in staging-simulated.” That needs explicit compatible AWS fixture/checks, simulator endpoints/capability/pinned binary and bundle wiring, then an observed run; none performed here.
  6. Execution boundary: LIVE_PROVISION=1 setup/teardown owns destructive fixture resets; this source pass did not execute it. Use disposable staging-simulated, not the persistent live tenant, for any later provisioning. No integration provider connectivity, webhook receiver reachability or real Slack OAuth acceptance was established here.

Unverified external state: tf-simulator archival/maintenance policy; currently published marketplace releases vs working-tree pins; live host availability; security-log retention guarantees; provider-side rate/delivery SLAs. Local source cannot settle these. No blanket “every topic/log/provider is supported” claim is justified.

Builtin Codex CLI sweep result (in addition to persona verdicts above)

The documentation-only changes introduce no actionable regressions. Static checks passed for the example’s Action inputs, navigation mapping, and diff whitespace; CI status could not be checked because GitHub authorization has expired.
The documentation-only changes introduce no actionable regressions. Static checks passed for the example’s Action inputs, navigation mapping, and diff whitespace; CI status could not be checked because GitHub authorization has expired.

Finding disposition: The CLI sweep reported no actionable findings.

@istvandocsa
Istvan Docsa (istvandocsa) marked this pull request as ready for review October 26, 2026 03:47
@istvandocsa
Istvan Docsa (istvandocsa) merged commit 6ee643d into main Oct 26, 2026
12 checks passed
@istvandocsa
Istvan Docsa (istvandocsa) deleted the istvandocsa/eng-1349-docs-pages-simulating-a-change-before-apply-and-guardrails branch October 26, 2026 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant