Skip to content

feat: 兼容oh my pi - #8183

Closed
nolaurence wants to merge 60 commits into
pingdotgg:mainfrom
nolaurence:codex/bundle-midscene-preview
Closed

feat: 兼容oh my pi#8183
nolaurence wants to merge 60 commits into
pingdotgg:mainfrom
nolaurence:codex/bundle-midscene-preview

Conversation

@nolaurence

@nolaurence nolaurence commented Aug 25, 2026

Copy link
Copy Markdown

What Changed

Why

UI Changes

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Note

High Risk
Removing the main CI workflow and replacing upstream release/deploy automation with a fork-only release path is a major operational change; mistaken merges could stop checks and production deploys on the main repo.

Overview
This PR reshapes fork maintenance and agent workflows more than product runtime code. It adds a shared .agents/skills tree (web/mobile test playbooks, iOS Simulator MCP debugging, pair-client.sh) and wires Claude/Codex to those skills plus pinned xcodebuildmcp@2.6.2 in .mcp.json / .codex/config.toml.

GitHub automation is heavily replaced: several upstream workflows (ci.yml, relay deploy, EAS, PR size/vouch, issue-label sync) are removed, and release.yml becomes a “Fork Release” pipeline (daily nightly schedule, standard GitHub-hosted runners, desktop artifacts + GitHub release only—no npm CLI publish, Vercel web deploy, relay config job, or Discord announce). New thread-transfer comment tooling ( thread-transfer-report.cjs + tests) and a triage playbook / via-triage issue template support agent-filed bugs; feature requests move to Discussions and feature_request.yml is dropped.

Contributor defaults shift: .env.example now documents production Clerk/relay public IDs by default, .gitignore covers showcase/mobile artifacts, stale .plans/* docs are deleted, and Macroscope gains a UI consistency check agent. The PR title mentions Oh My Pi compatibility, but this diff does not touch OmpDriver or provider code—only infra/docs/agent tooling here.

Reviewed by Cursor Bugbot for commit 3495ffa. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add compatibility with oh my pi in web app and worker

  • Expands session-logic.ts with new session behaviors and states to support oh my pi
  • Updates main.tsx and __root.tsx to initialize additional providers and layout elements during boot and render
  • Extends Worker.ts with new runtime branches and exports
  • Updates vite.config.ts with additional build plugins and options
📊 Macroscope summarized 3495ffa. 137 files reviewed, 343 issues evaluated, 320 issues filtered, 15 comments posted

🗂️ Filtered Issues

.repos/alchemy-effect/benchmark/container/src/microvm-worker.ts — 2 comments posted, 4 evaluated, 2 filtered
  • line 205: The Worker retries an unreachable MicroVM with Schedule.exponential("250 millis") for 14 attempts but has no timeout. From the included schedule implementation this accumulates roughly 68 minutes of backoff; after RunMicrovm succeeds, a failed readiness probe can therefore leave the request hanging for that duration and delay the onError termination, leaking a running MicroVM and tying up the Worker request. [ Failed validation ]
  • line 231: The /shutdown handler suppresses every TerminateMicrovm failure with Effect.ignore and still returns { ok: true }. A transient AWS error therefore leaves the VM running while the driver believes cleanup succeeded; repeated benchmark rounds can accumulate leaked MicroVMs until the account quota is exhausted and subsequent samples fail or become contaminated. [ Failed validation ]
.repos/alchemy-effect/benchmark/container/src/orchestrator.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 63: url: true creates a public Lambda Function URL, but this handler has no authentication or rate limit before boot launches a fresh MicroVM. Anyone who discovers the URL can repeatedly call /boot (and choose among all six images), consuming the account's MicroVM quota and incurring AWS compute/build-related usage; this benchmark endpoint needs an access control mechanism rather than exposing the launch operation anonymously. [ Cross-file consolidated ]
  • line 73: rawReachable treats every HTTP response as a successful readiness probe: after client.get(...), it only reads res.text and never checks res.status or the expected body. HttpClient preserves non-2xx responses rather than failing them, so an unauthorized/404/500 response from the MicroVM proxy or server ends the retry and records a false readyMs, making the raw baseline results invalid. [ Cross-file consolidated ]
  • line 217: The readiness retry at v.reachable uses uncapped Schedule.exponential("250 millis"): 14 retries can wait about 68 minutes, while this Lambda is configured with a 120-second timeout. If the probe does not become reachable, Lambda stops the invocation before the Effect.onError cleanup can call v.term, leaving the successfully launched MicroVM running and causing later benchmark boots to hit quota. [ Cross-file consolidated ]
  • line 243: The /shutdown handler suppresses every TerminateMicrovm failure with Effect.ignore and still returns { ok: true }. A transient AWS error therefore leaves the VM running while the driver believes cleanup succeeded; repeated benchmark rounds can accumulate leaked MicroVMs until the account quota is exhausted and subsequent samples fail or become contaminated. [ Cross-file consolidated ]
.repos/alchemy-effect/packages/alchemy-test/src/StrayOutput.ts — 6 comments posted, 8 evaluated, 1 filtered
  • line 69: toText is called independently for every intercepted chunk, so a UTF-8 character split across two Buffer/Uint8Array writes is decoded as replacement characters instead of being reconstructed. For example, two writes containing the first byte and remaining bytes of produce corrupted log text. Keep a streaming decoder per capture (or buffer incomplete trailing sequences) when diverting byte output. [ Out of scope (post-validation triage) ]
.repos/alchemy-effect/packages/alchemy-test/src/Tui.ts — 1 comment posted, 7 evaluated, 6 filtered
  • line 579: The drag-selection handler only copies when text.trim().length > 1. A legitimate one-character selection (for example selecting a single glyph or character in the detail pane) is non-empty but is silently ignored, even though the comment says only a zero-width click should be excluded. The check should distinguish empty text from a one-character selection. [ Out of scope (post-validation triage) ]
  • line 725: Row clipping uses UTF-16 String.length/slice against renderer.terminalWidth, which is measured in terminal cells. A test title or path containing an emoji can be cut between its surrogate pair at the width boundary, producing a lone surrogate/replacement glyph (and wide Unicode text is otherwise mis-sized), so the TUI's claimed width clipping corrupts labels for valid Unicode names. [ Out of scope (post-validation triage) ]
  • line 893: When the skipped group is enabled, selecting a skip or todo row and pressing r reaches controller.retryTest, but the runner never indexes skipped tests in testIndex, so the controller silently does nothing. The TUI still flashes retrying ..., misleading the user and providing no way to run that row; these statuses should be rejected or handled explicitly. [ Out of scope (post-validation triage) ]
  • line 896: For an import or file-hook failure, the visible file row can be selected and r calls controller.retryFile, but the runner's testIndex contains no entries for a file that never collected/runs successfully. The call is therefore a silent no-op while the footer flashes retrying <file>, so the TUI falsely offers retry for the most important file-level failures. [ Out of scope (post-validation triage) ]
  • line 989: The q handler resolves quit immediately, but the only consumer (waitForExit) is invoked by the CLI after run() has already emitted RunEnd. Therefore pressing the advertised q quit key during collection or test execution does nothing until the entire run finishes; the TUI cannot be exited early (apart from Ctrl+C), despite the footer showing q quit throughout the run. [ Out of scope (post-validation triage) ]
  • line 1157: After RunEnd, state.summary is defined, so the interval stops calling updateFooter() once flashUntil + 100 has elapsed. Any post-run action such as y or r sets a flash message, but the footer then remains permanently stuck on copied to clipboard/retrying ... instead of reverting to the toggle bar as documented by flash. [ Out of scope (post-validation triage) ]
.repos/alchemy-effect/packages/alchemy/src/AWS/ACM/Certificate.ts — 1 comment posted, 2 evaluated, 1 filtered
  • line 396: The new region prop allows a certificate to be requested in any AWS region, and reconcile can create one there via withCertRegion(news.region). However, list only enumerates us-east-1 and the ambient Region; a certificate deployed with (for example) region: "eu-west-1" while the stack runs in us-west-2 is never returned. Account-wide consumers such as alchemy unsafe nuke therefore cannot discover or delete that certificate, leaving it orphaned after state loss. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/ACMPCA/CertificateAuthority.ts — 0 comments posted, 6 evaluated, 6 filtered
  • line 407: listCaTags converts every tag-listing failure—not just a missing CA—into {}. When the cached ARN is unavailable, findManagedCa therefore interprets a transient ListTags timeout, throttling error, or permission error as “no ownership tags” and reconcile proceeds to createCertificateAuthority. An already-managed CA can consequently be duplicated and left orphaned (and continue incurring Private CA charges); ownership lookup should fail rather than create on an indeterminate tag read. [ Exceeded comment limit ]
  • line 487: waitUntilCreated treats every status other than CREATING as a successful observation. ACM PCA can transition from CREATING to FAILED; in that case reconcile continues through tag sync and returns attributes with status: "FAILED" instead of failing or recreating the CA. The engine then records the failed, unusable CA as successfully provisioned and subsequent reconciles keep reusing it. [ Exceeded comment limit ]
  • line 487: The post-create poll treats a handled ResourceNotFoundException as completion: describeCa returns undefined, then waitUntilCreated returns it successfully and reconcile substitutes { Arn: arn }. During the normal eventual-consistency window after creation, this skips the wait and immediately performs tag operations against an ARN that DescribeCertificateAuthority still cannot see, causing the deployment to fail instead of retrying until the CA is observable. [ Exceeded comment limit ]
  • line 571: The createCertificateAuthority request omits ACM PCA's IdempotencyToken. If the request is accepted by AWS but its response is lost (or the SDK retries a transient failure), each retry can create another CA before the tag-based recovery search runs. The extra CA is not represented by the returned ARN/state and remains an orphan that continues billing; derive a stable token from the resource instance when creating it. [ Exceeded comment limit ]
  • line 637: The revocation convergence comparison is not normalized for AWS defaults. For example, enabling CRL with expiration omitted sends ExpirationInDays: undefined, while AWS stores/returns the default 7 days; similarly omitted CrlType is returned as COMPLETE and omitted S3ObjectAcl as PUBLIC_READ. JSON.stringify therefore differs on every reconcile, so an ACTIVE/DISABLED CA receives UpdateCertificateAuthority on every deploy even though its configuration already matches the requested defaults. [ Exceeded comment limit ]
  • line 694: canSetWindow omits the EXPIRED state, even though AWS allows an expired CA to be deleted with a 7–30 day restoration period. When a managed CA has expired and olds.permanentDeletionTime is configured, this branch sends PermanentDeletionTimeInDays: undefined, silently ignoring the requested retention window (and falling back to AWS's default rather than the user's value). [ Out of scope (post-validation triage) ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AIOps/InvestigationGroup.ts — 0 comments posted, 7 evaluated, 7 filtered
  • line 271: toAttributes only returns name, arn, roleArn, and retentionInDays, while reconcile treats encryptionConfiguration, tagKeyBoundaries, chatbotNotificationChannel, isCloudTrailEventHistoryEnabled, crossAccountConfigurations, the resource policy, and tags as managed state. Sync compares the result of read to the persisted attributes and skips reconcile when they are equal, so out-of-band drift in any of those fields (for example a changed policy or removed ownership tag) is invisible and is never repaired unless some separately represented attribute changes. [ Exceeded comment limit ]
  • line 320: The documented default retention is 90 days, but the replacement check compares the raw wire conversions: omitted retention becomes undefined while explicit "90 days" becomes 90. Changing between those equivalent configurations therefore triggers a delete-first replacement, causing an unnecessary outage and new ARN instead of treating the effective retention as unchanged. [ Exceeded comment limit ]
  • line 355: The create path can successfully return with an invalid state record when the follow-up observation is absent. In the created.arn === undefined branch, observe(name, undefined) is allowed to return undefined; the code then computes arn as live?.arn ?? output?.arn, where both are undefined on a first create, but still returns arn! and commits it. A transiently not-yet-listable newly created group therefore reports success without an ARN, skips policy/tag sync, and leaves later reads/recovery without the identifier needed to manage it. [ Failed validation ]
  • line 361: The ConflictException recovery accepts any resource returned by observe(name, undefined) as the newly-created group without checking its Alchemy ownership tags. If this reconcile's initial observe sees no group and another stack/operator creates a same-named group before createInvestigationGroup, the handler adopts that foreign group, then the later sync unconditionally updates its role/configuration, policy, and tags. This bypasses the ownership check performed by read and can overwrite another stack's investigation settings or access policy. [ Exceeded comment limit ]
  • line 386: The reconciliation treats omitted optional fields as unmanaged, but the public props document gives defaults for them. If a deployed group has a customer-managed encryptionConfiguration and the user removes that prop, the news.encryptionConfiguration !== undefined guard prevents resetting it to the documented AWS-owned key. Likewise, changing isCloudTrailEventHistoryEnabled from false to omitted leaves CloudTrail history disabled instead of restoring its documented true default. These prop changes can reach reconcile but silently do nothing. [ Exceeded comment limit ]
  • line 469: The policy drift test compares JSON.stringify(JSON.parse(observedPolicy)) with the desired string, which is sensitive to object key ordering (and to equivalent scalar-versus-singleton-array forms). A semantically identical policy returned with a different key order is therefore treated as drift and re-written on every reconciliation, causing needless policy mutations and possible throttling rather than converging to a no-op. [ Out of scope (post-validation triage) ]
  • line 513: The provider marks retention/name changes as deleteFirst replacements, but delete returns immediately after deleteInvestigationGroup is accepted and never waits for the singleton to disappear. Apply invokes this delete and then immediately calls reconcile for the replacement; while the old group is still visible, the new create gets ConflictException, and this handler only re-observes the desired name (which is absent), rethrows the conflict, and leaves the replacement failed. The provider therefore cannot reliably perform the replacement it explicitly requires. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AMP/AnomalyDetector.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 197: waitSettled stops polling for any status other than CREATING/UPDATING, so terminal failure statuses such as CREATION_FAILED or UPDATE_FAILED are treated as successful settlement. After a create or put reaches one of those states, reconcile still tags the failed detector, notes the session, and returns its attributes, making a failed provisioning/update appear successful instead of surfacing the failure. [ Exceeded comment limit ]
  • line 293: desiredTags lets user-provided news.tags overwrite the ownership tags from createInternalTags. A resource configured with a tag such as "alchemy::id": "other" is created/synchronized without the real logical-id tag; read then calls hasAlchemyTags and returns Unowned, so the provider can no longer recognize its own detector (and may refuse to manage it or try to create another one after state loss). Reserved ownership keys must take precedence over user tags. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AMP/Scraper.ts — 0 comments posted, 5 evaluated, 5 filtered
  • line 261: The immutable-source replacement check is guarded by olds !== undefined (lines 260-264), but the provider lifecycle explicitly supports adoption with output !== undefined and olds === undefined. When an existing owned scraper is adopted and its EKS/VPC source differs from the desired news.source, diff emits no replacement and reconcile has no source update path, so the deployment succeeds while continuing to scrape from the wrong source. The source must be included in the observed attributes or adoption must force/perform replacement. [ Failed validation ]
  • line 281: desiredTags spreads user news.tags after the ownership tags, allowing a user tag named alchemy::stack, alchemy::stage, or alchemy::id to overwrite the value returned by createInternalTags. The scraper is created with the overwritten value, then the next read fails hasAlchemyTags and brands the still-live scraper Unowned, so subsequent updates/destroy operations refuse to manage it and leave the scraper orphaned. Internal ownership keys need to be reserved or merged after user tags. [ Exceeded comment limit ]
  • line 287: reconcile treats output?.scraperId === undefined as proof that the scraper is absent and immediately calls createScraper (lines 285-305). The provider contract explicitly allows output to be missing after a successful API call whose state write failed, and this resource already stamps unique Alchemy tags. A retry in that state creates a second scraper instead of locating the existing tagged scraper, leaving the first scraper running and duplicating metric collection/writes; repeated retries can accumulate orphan scrapers. [ Exceeded comment limit ]
  • line 313: reconcile only invokes waitActive after a create or an update. If an existing scraper is already in a terminal failure state (for example after a prior interrupted/failed operation) and its fields otherwise match news, the code reads its blob, skips the update, syncs tags, notes success, and returns status: "*_FAILED" without failing or recreating it. The provider therefore reports a successful deployment while the scraper is not collecting metrics. [ Exceeded comment limit ]
  • line 324: roleDrifts is explicitly false whenever news.roleConfiguration is omitted (line 324), even if the observed scraper has a cross-account roleConfiguration. Consequently, changing a declared scraper from cross-account roles to the documented service-linked-role mode with no simultaneous alias/config/destination change causes no UpdateScraper call and leaves the old roles active forever, so the cloud state does not match the resource props. AWS documents that switching back requires an update without RoleConfiguration. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AMP/Workspace.ts — 0 comments posted, 8 evaluated, 8 filtered
  • line 137: toAttrs omits both retentionPeriod/wire retention and limitsPerLabelSet, and read returns only these attributes. Consequently the engine's normal deploy/sync drift check cannot observe an out-of-band retention or label-limit change: the persisted attributes remain equal, so reconcile is never called and the desired configuration is silently left drifted. The configuration state must be included in the observable attributes or otherwise consulted during read/drift detection. [ Exceeded comment limit ]
  • line 137: Managed tags are absent from toAttrs, so tag drift is invisible to the engine. If an operator removes or changes a workspace tag out of band, read either returns the same attrs (or Unowned(attrs), which sync strips before comparison), and a normal deploy/sync with unchanged properties never calls reconcile; therefore syncAmpTags cannot repair the drift and ownership markers may remain broken indefinitely. The observed tag set needs to participate in drift detection. [ Exceeded comment limit ]
  • line 175: The recovery key is only alchemy::stack, alchemy::stage, and the bare logical id. Alchemy permits the same logical ID in different namespaces (the resource identity is the FQN), so two Workspace("Metrics", ...) resources in one stack/stage can receive identical internal tags. If state for either resource is lost, findByInternalTags picks the first match and this provider can attach the wrong workspace to the resource, then update its alias/tags/config and persist the wrong ID while orphaning the original. Recovery needs a namespace/FQN-unique marker or another identity check. [ Exceeded comment limit ]
  • line 233: toWireDays rounds durations to whole days, so valid Duration.Input values shorter than 12 hours become 0; for example, retentionPeriod: "1 hour" produces retentionPeriodInDays: 0. AMP requires retentionPeriodInDays to be at least 1, so reconciliation fails with a validation error even though this property's documentation says it accepts any Duration.Input. The provider should reject/validate this range before issuing the update (or enforce a minimum conversion). [ Exceeded comment limit ]
  • line 280: The post-update poll is bounded but never checks its final result. If DescribeWorkspaceConfiguration remains UPDATING, reaches a failed status, or otherwise never satisfies the predicate before Schedule.recurs(30) is exhausted, Effect.repeat returns the last configuration and this function still succeeds. The reconcile then reports the workspace updated even though the requested retention/limits were not applied; because those fields are not in the returned attributes, a later unchanged deploy may not retry it. [ Exceeded comment limit ]
  • line 307: waitActive repeats until the status is exactly ACTIVE and does not stop for terminal failure statuses. If AMP reports a failed creation, the provider keeps issuing DescribeWorkspace calls for the full 30-recurrence schedule (about a minute) before surfacing the failure, despite the comment promising fail-fast behavior. This unnecessarily delays every deployment that encounters a terminal workspace failure and can be fixed by making the stop predicate include *_FAILED/other terminal states. [ Exceeded comment limit ]
  • line 351: desiredTags is built as { ...internalTags, ...news.tags }, so a user-supplied tag can overwrite alchemy::stack, alchemy::stage, or alchemy::id. For example, setting tags: { "alchemy::id": "other" } causes the newly created workspace to fail hasAlchemyTags on the next read; normal planning then treats the resource as unowned and the tag-based recovery path can no longer find it, leaving the workspace orphaned/unmanageable. User tags must not be allowed to override ownership tags. [ Exceeded comment limit ]
  • line 360: The tag-recovery path can return a workspace that is still CREATING (for example, after a crash after createWorkspace but before state persistence), but reconcile only calls waitActive for a newly created workspace. When findByInternalTags finds that in-progress workspace, the workspace === undefined branch is skipped, so alias/tag/config work proceeds and toAttrs can persist/emit a CREATING workspace as successfully reconciled. This defeats the recovery path's readiness guarantee and lets dependents proceed with an unsettled workspace; recovered workspaces should be passed through waitActive before synchronization/return. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AccessAnalyzer/Analyzer.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 147: The default name is generated directly from createPhysicalName({ id, maxLength: 255 }), whose prefix starts with stack.name. Stack names are not constrained to begin with a letter, while Access Analyzer requires every analyzer name to match [A-Za-z][A-Za-z0-9_.-]*. A valid stack such as 123-prod therefore produces a generated analyzer name beginning with 1, and every deployment of an analyzer without an explicit analyzerName fails with AWS ValidationException rather than creating the resource. [ Exceeded comment limit ]
  • line 184: unusedAccessAge is a declared create-only managed property, but read drops analyzer.configuration.unusedAccess.unusedAccessAge from the returned attributes (and list does the same). sync compares the returned attributes with the persisted attributes and only calls reconcile when they differ, so an out-of-band tracking-period change is invisible and will never trigger the replacement needed to restore the configured age. The getAnalyzer result already contains the configuration; the provider needs to expose it in Attributes/read (or otherwise explicitly detect drift). [ Exceeded comment limit ]
  • line 203: Changing type returns a replacement without deleteFirst, but createName is independent of type, so the replacement uses the same analyzer name as the old generation. Access Analyzer addresses analyzers by name (GetAnalyzer has no type discriminator), so the create-first replacement collides; the ConflictException handler then reuses the old analyzer, and the replacement flow later garbage-collects that old generation. A type change can therefore finish with the old type deleted and no correctly typed analyzer (or silently retain the old type). This replacement must delete first (and wait for deletion) when the name is unchanged. [ Failed validation ]
  • line 246: The ConflictException handler treats every create conflict as a safe concurrent create and then adopts whatever observe(name) returns. If a foreign actor creates an analyzer with this name after the initial observe (or the name is already occupied in a race), this path proceeds to tagResource/untagResource with desiredTags, removing the foreign tags and adding this stack's ownership tags, then records the foreign analyzer as its own. The conflict recovery must verify ownership (or otherwise distinguish the provider's own raced create) before mutating the analyzer. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/Amplify/App.ts — 0 comments posted, 6 evaluated, 6 filtered
  • line 216: syncTags performs tagResource/untagResource directly, outside the amplifyWriteRetrySchedule used for the other Amplify mutations. When the account-wide Amplify quota returns its documented BadRequestException with message Rate exceeded, a tag change fails the reconcile immediately (and can leave the preceding updateApp applied), rather than retrying as intended. Wrap both tag mutations in the same bounded retry policy. [ Exceeded comment limit ]
  • line 216: syncTags adds/upserts tags before removing obsolete keys. When an existing app already has Amplify's 50-tag limit and the desired set replaces one key with another (still 50 total), tagResource temporarily attempts 51 tags and is rejected before untagResource runs. Remove obsolete keys first, or update replacements in a way that never exceeds the service limit. [ Exceeded comment limit ]
  • line 232: read immediately returns undefined when output.appId is absent instead of using the supplied olds/deterministic identity to recover the app. After an interrupted create has persisted creating state with no attr, a later reconcile receives olds: undefined; if the app previously had an explicit name that is now changed, findOwnApp searches only the new name and creates a second app, leaving the successfully-created old app orphaned. Recover the prior app by its persisted identity/tags before creating. [ Exceeded comment limit ]
  • line 248: desiredTags lets news.tags overwrite the reserved alchemy::stack, alchemy::stage, or alchemy::id ownership tags. For example, passing { tags: { "alchemy::id": "other" } } creates/retags the app with the wrong owner, so the next read returns Unowned and the resource becomes unmanageable (or deployment fails unless --adopt is used). Merge user tags before the internal tags or reject reserved keys. [ Exceeded comment limit ]
  • line 263: When state/output is missing, the engine's cold-start probe cannot adopt this app because read returns undefined without output.appId (line 232), so reconcile reaches findOwnApp. If it finds an owned app, this branch simply returns it and skips updateApp and syncTags; a redeploy after state loss with changed description, platform, build settings, or tags reports the desired attributes as applied while the cloud app remains on the old configuration. Apply the desired settings/tags to an app found by this recovery path before returning. [ Exceeded comment limit ]
  • line 274: The provider forwards desiredTags to CreateApp without reserving space for its three internal tags. AWS Amplify limits an app's tag map to 50 entries, so a valid AppProps.tags map with 48–50 user tags becomes 51–53 tags after merging and CreateApp/tag synchronization fails with a validation error. Reject or trim user tags to the available budget before adding internal tags. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/Amplify/Branch.ts — 0 comments posted, 6 evaluated, 6 filtered
  • line 220: read builds attrs with only appId, branchName, branchArn, and tags, omitting every mutable branch setting such as description, stage, enableAutoBuild, environment variables, auth, and ttl. The engine's sync compares this read result with the stored attributes to detect cloud drift, so an operator changing any of those settings out-of-band produces no detected drift and a subsequent sync leaves the branch incorrect. [ Exceeded comment limit ]
  • line 233: news.tags is spread after the required Alchemy ownership tags, so a user can supply alchemy::stack, alchemy::stage, or alchemy::id and overwrite them. The branch is then created with tags that fail hasAlchemyTags on the next read, causing the provider to report it as Unowned and fail normal subsequent deploys (or repeatedly treat it as an adoption), even though the resource was just created by this stack. [ Exceeded comment limit ]
  • line 237: ttl accepts the full Duration.Input type, including the valid ` [ Out of scope (triage) ]
  • line 237: ttl accepts the full Duration.Input type, including the valid "Infinity" and "-Infinity" inputs supported by Effect. String(toWireSeconds(news.ttl)) sends "Infinity" or "-Infinity", but AWS Amplify's branch TTL field accepts only digit strings, so creating or updating a branch with those supported duration inputs fails with a validation error. [ Out of scope (post-validation triage) ]
  • line 256: On a first deployment (or replacement), output is undefined, so the engine's cold read returns undefined without checking the requested name and proceeds to reconcile. The reconcile path then observes any existing branch with news.appId/branchName and treats it as owned, calling updateBranch and later overwriting its tags. Thus a pre-existing branch created outside this stack (including one with different Alchemy tags) is silently adopted and mutated instead of being branded Unowned and blocked unless adoption is explicit. [ Exceeded comment limit ]
  • line 267: When output is missing but observe finds an existing branch, the create/recovery branch returns existing directly and skips both updateBranch and syncTags. This occurs after state loss or an interrupted prior deployment, and also when a branch already exists with the same identity but stale settings; the provider then persists desiredTags as if applied even though the cloud branch may still have old settings/tags, leaving silent drift. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/ApiGatewayV2/Stage.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 341: The ENSURE block claims to tolerate a concurrent ConflictException and fall through to observation, but retryOnTooManyRequests only retries that error and then re-fails it; there is no catchTag("ConflictException", ...) or subsequent getStageSafe. If another deploy creates the same stage during this operation, reconciliation fails rather than adopting the existing stage. [ Exceeded comment limit ]
  • line 366: When a stage does not exist, createStage is executed at line 341 but its response is never assigned back to observed. The subsequent snapshotFromStage(apiId, observed, urls) dereferences stage.StageName, so every first-time stage creation reaches this path with observed === undefined and crashes instead of returning the created stage state. [ Skipped comment generation ]
  • line 374: description is treated as managed even when it is omitted: snapshot.description !== news.description makes drift true for any existing stage with a description, while updateStage sends Description: undefined (which is omitted from the AWS request), so the description is never changed. The same non-converging behavior occurs for clientCertificateId at line 386. Such stages are updated on every reconcile without reaching the declared partial configuration. [ Exceeded comment limit ]
  • line 408: The stage tag synchronization at syncTags passes stageArn(...), whose helper constructs an arn:aws:... ARN. AWS uses partition-specific prefixes such as arn:aws-us-gov in GovCloud (and arn:aws-cn in China), so reconciliation of a stage with tag drift in those regions sends a resource ARN for the wrong partition and tag/untag operations fail instead of converging. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppConfig/ConfigurationProfile.ts — 0 comments posted, 10 evaluated, 10 filtered
  • line 136: When configurationProfileName is omitted, toName derives the lookup name from the ambient InstanceId via createPhysicalName. On a cold-start read the planner supplies a newly generated adoption instance ID, so the lookup name differs from the name created by the previous instance. A state-loss/import recovery therefore fails to find the existing profile and creates another one, orphaning the original profile (and any hosted versions under it). [ Exceeded comment limit ]
  • line 214: All profile ARNs are built through configurationProfileArn, which hard-codes the aws partition. In aws-cn or aws-us-gov, the correct ARN partition is aws-cn or aws-us-gov; the resulting ARN is therefore invalid for readAppConfigTags/syncAppConfigTags and is also persisted as the profile output ARN. Profile creation and subsequent ownership checks fail or produce unusable attributes in those partitions. [ Exceeded comment limit ]
  • line 236: Cold-start adoption can silently accept an incompatible profile. read finds a tagged profile but does not return its Type, and reconcile uses locationUri only on create while updateConfigurationProfile cannot change either immutable field. When an existing tagged profile is adopted with a different desired locationUri or type, the forced adoption update leaves the physical profile unchanged, while state is recorded from the desired props, so later plans report no replacement or drift. [ Exceeded comment limit ]
  • line 238: desiredTags lets user tags overwrite the ownership markers because news.tags is spread after internalTags. A valid tag such as { "alchemy::id": "custom" } causes the newly created profile to fail hasAlchemyTags(id, tags) on the next read, so the provider treats its own resource as Unowned and normal subsequent deploys fail or require adoption. Reserved internal keys should win or be rejected. [ Exceeded comment limit ]
  • line 245: reconcile treats any profile returned by findByName as owned without checking its Alchemy tags. The engine's earlier ownership probe is not atomic with apply: if another stack creates a same-name profile after planning (or replaces a missing output before retry), this fallback finds that foreign profile, calls updateConfigurationProfile, and then rewrites its tags. This bypasses the Unowned/adoption guard and can mutate another stack's configuration and ownership metadata. [ Exceeded comment limit ]
  • line 245: Changing locationUri or type is marked as a replacement, but the replacement is not deleteFirst and uses the same profile name under the same application. During the create-first replacement, output is empty while the old profile still exists, so findByName(applicationId, name) returns that old profile; the provider then updates it without changing either immutable field and records it as the new generation. The requested source/type change is therefore silently ignored. [ Exceeded comment limit ]
  • line 267: The update request passes Description: news.description even when the desired optional description has been removed. The AWS API treats Description as an optional PATCH field, and the SDK omits an undefined value, so the old description remains in the profile instead of being cleared (an empty string is the valid zero-length value). Subsequent deploys continue to report success while the cloud profile drifts from the declared props. [ Exceeded comment limit ]
  • line 269: Removing validators from the desired profile is also not convergent: toWireValidators(undefined) yields undefined, so the SDK omits Validators from the PATCH and AWS retains the existing validators. Because the provider does not return or diff validator state, the stale validation rules remain silently after a successful deployment; an explicitly empty list is needed to clear them. [ Exceeded comment limit ]
  • line 270: KmsKeyIdentifier is handled the same way: removing kmsKeyIdentifier from the desired props sends undefined, which is omitted from the PATCH request, so an existing hosted profile keeps using its previous KMS key. The API permits an empty identifier when updating, but this provider never sends that clear value and does not expose the drift in its returned attributes. [ Exceeded comment limit ]
  • line 329: The delete path performs a single version listing and then deletes the profile, even though the code explicitly supports runtime writers that can create hosted versions. If a version is created after listHostedConfigurationVersions completes, it is not deleted and deleteConfigurationProfile rejects because a hosted version remains, so destroy leaves the profile behind. Re-list/retry the cleanup or otherwise coordinate writers before the final delete. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppConfig/Environment.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 190: When an existing environment is renamed out of band, reconcile deliberately chooses output.environmentName as name, so the observed drifted name is reused rather than the desired toName(id, news) value. Because diff compares only the desired props, a normal deploy with unchanged props will not notice it; alchemy sync will detect the changed environmentName but call this reconcile and persist the wrong name without restoring it. AWS UpdateEnvironment supports Name, so this is a reachable drift case. [ Exceeded comment limit ]
  • line 192: desiredTags lets user-supplied news.tags overwrite the reserved alchemy::stack, alchemy::stage, or alchemy::id ownership markers. For example, tags: { "alchemy::id": "other" } creates/syncs an environment whose ownership check at read fails, so a later cold-start/adoption refuses it as Unowned and sync cannot repair tags when attributes are otherwise unchanged. User tags should not be able to replace the internal ownership values. [ Exceeded comment limit ]
  • line 243: delete calls DeleteEnvironment without setting DeletionProtectionCheck: "BYPASS". When the account has AppConfig deletion protection enabled and the environment was recently read by GetLatestConfiguration/GetConfiguration, AWS rejects this request with BadRequestException; the environment then cannot be destroyed (and the same path is used by unsafe nuke) until the protection window expires or an operator deletes it manually. The provider exposes no property that lets callers choose the check behavior. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppConfig/Extension.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 261: desiredTags lets user tags override the reserved ownership tags because { ...internalTags, ...news.tags } spreads news.tags last. A caller supplying any alchemy::stack, alchemy::stage, or alchemy::id tag creates an extension whose ownership metadata is wrong; subsequent hasAlchemyTags checks return false, so the next deploy treats this stack-owned extension as Unowned and fails or requires adoption, while tag sync preserves the bad values. [ Exceeded comment limit ]
  • line 270: When output.extensionId is absent, reconcile treats any extension returned by findByName(name) as owned and proceeds to update it and sync tags. This bypasses the read/Unowned ownership check for unresolved-input deployments (the planner skips its adoption probe while extensionName is unresolved), and also for a create-vs-create name race. An existing foreign extension with that eventual name can therefore be silently overwritten, and later managed/deleted, without adopt being enabled. [ Exceeded comment limit ]
  • line 301: updateExtension creates a new extension version, but this reconcile path only returns the new version and never updates existing ExtensionAssociation resources. AppConfig materializes the association's ExtensionVersionNumber when the association is created, so associations that omitted a version remain bound to the old version and continue invoking the old actions/parameters after an extension update. [ Exceeded comment limit ]
  • line 324: deleteExtension is called without VersionNumber, but AppConfig deletes only the highest extension version when that query parameter is omitted. After any updateExtension has created version 2 (and so on), destroy removes only the newest version and leaves older versions and the extension resource behind, causing leaked cloud resources and name conflicts on later deployments. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppConfig/ExtensionAssociation.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 140: findByIdentifiers resolves an extension ID but never checks the requested extensionVersionNumber against the matched association's versioned ExtensionArn. On a cold-start adoption, an existing same-extension/resource association at an older version is therefore accepted as the desired resource; the planner adopts it and reconcile only updates parameters/tags, leaving the requested pinned version unapplied. [ Exceeded comment limit ]
  • line 144: findByIdentifiers uses item.ExtensionArn.includes(extension.Id!) as the identity test. Because the ARN contains the extension ID as one path segment, substring matching can select a different extension whose ID is a prefix of the requested ID (or whose ID merely appears elsewhere in the ARN). The provider can then update parameters/tags or return attributes for the wrong association, corrupting another extension's configuration and state. [ Exceeded comment limit ]
  • line 198: The provider blindly merges three internal ownership tags into news.tags and sends the result to AppConfig. AppConfig permits at most 50 tags, so a valid user configuration containing 50 user tags is expanded to 53 and CreateExtensionAssociation/tag sync fails validation instead of creating the resource. The provider needs to reserve/validate capacity for its internal tags. [ Exceeded comment limit ]
  • line 210: Changing extensionVersionNumber is declared replacement-only, but the provider does not make that replacement delete-first. A replacement create for the same extension/resource pair cannot create a second association; it fails (or, after recovery, findByIdentifiers rediscovers the old pair) rather than producing the requested version. Consequently version changes leave the old association in place or fail the deployment instead of converging to the new extension version. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppIntegrations/Application.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 222: An explicit empty approvedOrigins array is forwarded as ApprovedOrigins: [] in desiredSourceConfig. The AppIntegrations ApplicationApprovedOrigins shape has a minimum of one member, so creating or updating with approvedOrigins: [] is rejected by AWS rather than clearing the list; this is especially exposed by the update comparison, which deliberately treats [] as a desired empty state and sends it. Either reject this prop locally or represent removal using the API's supported omission semantics. [ Exceeded comment limit ]
  • line 233: reconcile falls back to findByNamespace(news.namespace) and treats any matching application as the replacement resource, without checking hasAlchemyTags/Unowned. This is reachable when an existing managed resource changes to a namespace already occupied by an unrelated application: the engine creates the replacement directly (without the cold-start adoption probe), this lookup finds the foreign app, and the subsequent update/tag/return path mutates and takes ownership of it. A namespace collision should be rejected unless ownership is verified. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppIntegrations/DataIntegration.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 171: The lost-state recovery is keyed by the generated physical name, but generated names include the instance-ID suffix, which changes on a lost-state re-run (the sibling Application provider documents this exact behavior and instead recovers by its stable namespace). With no cached output, read/reconcile compute a new name and findByName cannot find the existing integration, so reconciliation creates another integration and leaves the old one orphaned (potentially causing duplicate ingestion). Recovery must use a stable identity or enumerate/verify ownership rather than the generated name. [ Exceeded comment limit ]
  • line 303: The create path does not handle the API's DuplicateResourceException race. Two deploys (or a retry after the first call succeeded but state persistence failed) can both observe no resource and call createDataIntegration; the loser receives DuplicateResourceException and the reconciliation fails even though the integration now exists. The other AppIntegrations providers re-enumerate by identity when this race occurs, so this provider should do the same. [ Exceeded comment limit ]
  • line 380: deleteDataIntegration is called with DataIntegrationIdentifier, but the AppIntegrations DeleteDataIntegration API requires the request field Identifier (the same field used by getDataIntegration and updateDataIntegration above). Consequently every destroy attempt sends no identifier and fails with an invalid request instead of deleting the integration, leaving the cloud resource behind. [ Failed validation ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppIntegrations/EventIntegration.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 200: For an explicitly named integration, changing source or eventBridgeBus returns replace, but replacement reconciliation still observes by the unchanged name at lines 200–206. Because the old integration must remain during the default create-first replacement and names are unique, observe(name) finds the old object, so no new immutable configuration is created; subsequent garbage collection can then delete that same object, leaving the stack with no integration. [ Exceeded comment limit ]
  • line 251: Event integration tag reconciliation delegates to diffTags at line 251, whose for...in/in checks treat inherited names such as toString as already present. AWS tag keys allow these names, so removing a user tag named toString (or similar prototype key) is never placed in removed and the stale cloud tag remains indefinitely. [ Out of scope (post-validation triage) ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppRegistry/ResourceAssociation.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 187: read omits the association's options from the returned attributes, so drift in APPLY_APPLICATION_TAG versus SKIP_APPLICATION_TAG is invisible to sync: when the association still exists, the returned object can equal persisted old.attr, causing Sync.ts to classify it as unchanged and never call reconcile. Thus an out-of-band option/tag change is not repaired despite reconcile claiming to synchronize options. [ Exceeded comment limit ]
  • line 210: The associate effect swallows every ConflictException, and the final read only checks that observed.resource.arn exists; it never verifies observed.options against news.options. If the initial read races with another association, or the recreate call races while the old association is still present, the conflict is treated as success and the provider returns attributes for an association that may still have the old/different tag option. A later apply can therefore report success without converging the requested option. [ Exceeded comment limit ]
  • line 223: When an existing association has explicit options and the desired news.options is later removed, this branch skips synchronization entirely because it only enters the recreate path when news.options !== undefined. Since omitting options on associateResource uses the documented service default APPLY_APPLICATION_TAG, removing options should restore that default, but the old SKIP_APPLICATION_TAG association remains unchanged and the provider reports success. Reconcile the omitted case against the default option as well. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppRunner/AutoScalingConfiguration.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 221: desiredTags lets user-supplied news.tags overwrite the ownership markers produced by createInternalTags. A tag set such as { "alchemy::id": "other" } is written during creation, but the next read checks the real logical ID and returns Unowned, causing later deploys to fail ownership checks (or require an unsafe adopt) for a resource this stack created. [ Exceeded comment limit ]
  • line 233: When a previously configured scaling option is removed from news, havePropsChanged schedules an update, but drifted only compares fields whose new value is non-undefined. For example, changing maxConcurrency from 50 to omitted leaves the existing revision at 50 instead of creating a revision with the documented default, so the cloud configuration no longer matches the desired props and subsequent deployments silently keep the stale value. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppRunner/ObservabilityConfiguration.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 241: desiredTags lets news.tags overwrite the reserved alchemy::stack, alchemy::stage, and alchemy::id ownership tags. A user who supplies any of those keys creates a configuration that read immediately classifies as Unowned, so subsequent deploys fail unless adoption is forced; with a matching name/id, spoofed tags can also make another stack treat the configuration as its own. Merge user tags first and internal tags last (or reject reserved keys). [ Exceeded comment limit ]
  • line 252: Removing traceConfiguration from an existing resource never disables tracing. When the active revision has a vendor and news.traceConfiguration is undefined, the condition at drifted is false, so reconciliation skips createObservabilityConfiguration and returns the existing X-Ray revision unchanged. This contradicts the prop contract that omitting the configuration means tracing is disabled and leaves the resource permanently out of sync after a vendor-to-omitted update. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/AppRunner/Service.ts — 0 comments posted, 13 evaluated, 13 filtered
  • line 845: ensureRepository treats any RepositoryAlreadyExistsException as an owned repository: it describes repositoryName and never checks for the stack/id ownership tags before returning its URI. A pre-existing repository with the deterministic name is therefore used for image pushes, and the resulting state causes delete to later call deleteRepository(..., force: true) on it. A name collision or leftover foreign repository can consequently be overwritten and permanently deleted by destroying this service. [ Exceeded comment limit ]
  • line 890: attachBindings strips all non-alphanumeric characters from each policy statement's Sid but does not ensure the resulting IDs remain unique. Two valid binding statements such as AllowFoo-Bar and AllowFooBar (or two bindings with the same Sid) become the same Sid in the generated inline policy; IAM requires Sid values to be unique within a policy, so putRolePolicy rejects the deployment whenever such bindings are combined. [ Exceeded comment limit ]
  • line 985: The generated entrypoint runs only exports.program, and createHostRuntimeContext constructs that from registered host.run/serve runners. For an otherwise valid Effect-native implementation that returns {} (the fetch field is optional and no host.run is registered), Effect.all([]) completes immediately, so Effect.runPromise(program) exits and App Runner repeatedly restarts an image with no listening server. The provider should reject a service with no runner or keep the process alive. [ Exceeded comment limit ]
  • line 1073: The generated Dockerfile copies only index.mjs and *.js, even though BundleOutput.files explicitly includes rolldown assets (for example .css, .map, or plugin-emitted/static files) and materialize writes every emitted file into the context. A native service whose bundle references such an emitted asset will build successfully but the asset is absent from the image, causing runtime module/file loads to fail. [ Exceeded comment limit ]
  • line 1073: The generated Dockerfile always contains COPY *.js /app/, but the bundle can legitimately contain only the renamed index.mjs and no .js chunks (the comment immediately above even says minimal bundles emit none). Docker treats an unmatched COPY glob as an error (no source files were specified), so deploying a minimal Effect-native App Runner service fails during image build instead of producing an image. Emit this instruction only when chunks exist or copy a directory/pattern that is guaranteed to match. [ Exceeded comment limit ]
  • line 1167: waitUntilGone converts DELETE_FAILED into a plain Error, while its retry predicate only retries AppRunnerServiceNotSettled; therefore a service entering AWS's DELETE_FAILED state immediately aborts deletion. Because ECR and IAM cleanup occurs only after waitUntilGone, stack.destroy() leaves the managed repository and roles behind, even though App Runner documents retrying DeleteService for this status. [ Exceeded comment limit ]
  • line 1252: read builds attributes only from toAttrs(service, platformAttributesOf(output)); it never records or compares SourceConfiguration, instance, health, network, observability, or scaling settings. Since sync decides whether to call reconcile by deep-comparing these attributes, an out-of-band image/configuration change that leaves the service's IDs/status unchanged appears unchanged forever and sourceDrifted/the other drift checks are never reached. The provider therefore does not actually self-heal the drift its reconcile logic claims to handle. [ Exceeded comment limit ]
  • line 1266: desiredTags lets user-supplied news.tags overwrite the ownership markers from createInternalTags because the spread order is { ...internalTags, ...news.tags }. Supplying any alchemy::stack, alchemy::stage, or alchemy::id tag makes the newly created service fail hasAlchemyTags on the next read and be treated as Unowned (and can spoof another resource's identity), causing later deploys to reject or take over the service and undermining ownership protection. [ Exceeded comment limit ]
  • line 1280: The reconcile branch selects the Effect-native path solely on news.main !== undefined and silently ignores a simultaneously supplied news.imageRepository. The public ServiceProps contract documents these inputs as mutually exclusive, so a configuration containing both unexpectedly builds/pushes a managed image and deploys it instead of rejecting the invalid combination; the caller's low-level image is not the one used. [ Exceeded comment limit ]
  • line 1299: The Effect-native reconcile treats persisted output as proof that managed dependencies still exist: it skips ensureRole whenever output.instanceRoleArn/accessRoleArn is present and skips ensureRepository whenever output.repositoryUri matches the name. If an operator deletes one of those IAM roles or the ECR repository out of band, readService still succeeds but this path reuses the stale ARN/URI; policy attachment or image push then fails and the provider never recreates the missing dependency, leaving the service unrecoverable through a normal deploy. [ Exceeded comment limit ]
  • line 1345: User news.env values override the runtime-critical variables because the merge puts ...news.env after ...alchemyEnv. In particular, setting env.PORT to a value different from news.port makes the generated BunHttpServer read the wrong PORT and bind there while App Runner routes/health-checks the configured port, leaving the service unhealthy; overriding ALCHEMY_STACK_NAME/ALCHEMY_STAGE likewise gives the runtime an incorrect stack identity. Reserve these keys or apply framework values last. [ Failed validation ]
  • line 1553: Changing an existing resource from main to the low-level imageRepository form is not marked replace by diff, and the low-level reconcile returns toAttrs(observed, emptyPlatformAttributes). That overwrites the saved managed ECR/role names with undefined; the old managed repository and IAM roles remain in AWS, but a later delete has no names to clean them up. Either reject/formally replace this transition or retain and reap the old platform attributes. [ Exceeded comment limit ]
  • line 1654: list() returns every App Runner service with emptyPlatformAttributes, including Effect-native services whose ECR repository and IAM role names are required by delete. Account-wide alchemy unsafe nuke passes these listed attributes directly to delete, so it removes the service but cannot enter the repository/role cleanup branches; every native service discovered by nuke leaks its managed ECR repository and both IAM roles. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/Cognito/User.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 125: The generated username is incompatible with pools configured with usernameAttributes. createName always produces a physical name when props.username is omitted, but this resource's own contract says such pools require the username to be a value of an attribute (for example an email address). A User with userPoolId pointing to an email/phone-username pool and no explicit username therefore sends an invalid generated identifier to AdminCreateUser and cannot be created. [ Exceeded comment limit ]
  • line 178: Out-of-band mutable drift is never detected on an otherwise unchanged deployment. reconcile contains the attribute and Enabled synchronization, but diff only checks the username and pool identity; attributesOf also omits enabled and the managed attributes from the output. After someone changes a user attribute or disables/enables the user in Cognito, a normal redeploy with identical props plans a noop and never invokes reconcile, leaving the user permanently divergent unless the user props change or a forced run is used. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/EC2/KeyPair.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 146: diff treats keyType and keyFormat changes as replacement triggers even when publicKeyMaterial is supplied, although both props are documented as ignored for imported keys. For example, changing only keyFormat on an imported key returns { action: "replace" }, causing Alchemy to delete and re-import an otherwise unchanged key and potentially interrupt SSH consumers. [ Exceeded comment limit ]
  • line 186: list calls ec2.describeKeyPairs({}) only once instead of consuming its paginated results. In accounts with more than one response page, branded key pairs on later pages are omitted from the provider's enumeration, so account-wide operations such as alchemy unsafe nuke cannot discover and delete them and the provider's required exhaustive listing is violated. Use describeKeyPairs.pages({}) and flatten every page. [ Failed validation ]
  • line 234: Both create/import paths convert InvalidKeyPair.Duplicate into undefined and then blindly describe the name, after which reconcile updates tags and returns the key. If the initial describe raced with another stack/actor creating that name (or two resources use the same explicit keyName), this provider adopts the other key without checking its Alchemy ownership tags; the stacks then share one keyPairId, and destroying either can delete the other's SSH key while tag sync also overwrites its ownership metadata. [ Exceeded comment limit ]
  • line 243: The imported-key branch hardcodes info.KeyType to "rsa" at line 243, even though publicKeyMaterial can be an OpenSSH/Ed25519 key and the props documentation says imported keys retain their own algorithm. A first deployment of an Ed25519 imported key therefore returns and persists keyType: "rsa" until a later read happens to recover the cloud value, giving consumers incorrect key metadata. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/ECS/Service.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 532: Making vpcId and subnets independently optional permits callers to provide only one, but the provider resolves any partial pair against the account's default VPC/network instead of resolving the missing side within the explicitly supplied VPC. For example, vpcId: myVpc with no subnets uses default-VPC subnets, and subnets: myVpcSubnets with no vpcId composes target groups/security groups in the default VPC; these mismatches can make service or managed-ingress deployment fail (or attach resources to the wrong VPC). [ Failed validation ]
.repos/alchemy-effect/packages/alchemy/src/AWS/ELBv2/ListenerRule.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 153: read returns only ruleArn, listenerArn, priority, and isDefault, while reconcile manages conditions, actions, and tags. The engine's sync compares the attributes returned by read; consequently, an out-of-band change to a rule's conditions, actions, or tags produces an identical attribute snapshot and never invokes reconcile, so drift is silently left in production. The observed attributes/read path must include the managed rule configuration (or otherwise make those fields part of drift detection). [ Exceeded comment limit ]
  • line 231: reconcile forwards news.actions to serializeActions without enforcing the required ordering. The serializer assigns Order from the caller's array order, so a valid ListenerAction[] such as [forward, authenticateOidc] produces the terminal action first. AWS requires exactly one routing action (forward, redirect, or fixed-response) and requires it to be last, so creating or updating such a rule fails instead of normalizing the actions or rejecting the input clearly. [ Exceeded comment limit ]
  • line 247: The recovery path cannot find a rule after state persistence fails: read returns undefined whenever output lacks ruleArn, and reconcile then calls non-idempotent createRule without looking up the existing rule by listenerArn and priority. If AWS created the rule but the process crashed before persisting its attributes, the next deploy retries createRule, receives PriorityInUse, and leaves the already-created rule orphaned/unmanageable. Use a natural-key lookup (or another idempotent recovery mechanism) before creating. [ Exceeded comment limit ]
  • line 280: Priority updates are issued one rule at a time with setRulePriorities. A valid reordering that swaps two priorities (for example rule A 10 -> 20 and rule B 20 -> 10) cannot converge: the first call sees the other rule still occupying its target and returns PriorityInUse, so the second rule is never updated. AWS supports submitting multiple rule-priority pairs together; the provider needs a coordinated/batched or temporary-priority strategy for swaps. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/ELBv2/TrustStore.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 182: When output is absent, reconcile searches by Names: [name] and treats any matching trust store as its own without checking the internal ownership tags (or any other ownership marker). A first deployment with an explicit name that already belongs to another stack/operator therefore calls modifyTrustStore on that object, rewrites its CA bundle, and later rewrites/removes its tags instead of failing the name collision. [ Exceeded comment limit ]
  • line 230: The bounded wait at until: (ts) => ts?.Status === "ACTIVE" does not verify that the predicate ever became true. Effect.repeat returns the last successful description when its times budget is exhausted, so a trust store that is still CREATING after the ten polls continues through tag sync and is returned as a successful resource with a non-ready status; downstream listeners can then be reconciled against an unusable trust store instead of the deployment retrying/failing. [ Exceeded comment limit ]
  • line 278: delete catches TrustStoreInUseException after the bounded retry schedule and converts it to success. If a listener is still attached (or detachment takes longer than the retry window), the trust store remains in AWS while Alchemy records the resource as deleted and drops its output, leaving an orphaned in-use trust store that future destroys will no longer target. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/FraudDetector/DetectorVersion.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 172: Changing a rule's expression, outcomes, language, or description with the same ruleId is detected as a detector-version replacement, but ensureRules reuses the latest existing rule version instead of creating the requested rule version. Because replacements are create-first and the new reconcile receives olds: undefined, the replacement detector version continues referencing the old rule and the desired fraud logic is silently never deployed; subsequent runs see the new props and stop trying. [ Exceeded comment limit ]
  • line 318: ensureRules reuses any pre-existing rule version for a matching ruleId, but delete later unconditionally calls deleteRule for every rule returned by the detector version. Deleting an Alchemy version can therefore delete a rule that was created by another stack/user (and is merely referenced here), destroying shared fraud-detection configuration; shared references may instead make the delete fail and leave cleanup incomplete. Track ownership/creation before deleting rules, or do not delete reused rules. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/GlobalAccelerator/Accelerator.ts — 0 comments posted, 6 evaluated, 6 filtered
  • line 291: ipAddresses is declared stable across every accelerator update, but reconcile allows ipAddressType to change between IPV4 and DUAL_STACK. The resource docs say dual-stack assigns both IPv4 and IPv6 addresses, and toAttributes flattens every returned IpSets entry into ipAddresses, so that update can change this output. The engine will preserve the old stable output for downstream consumers, leaving them with stale addresses after a valid IP-type update. [ Exceeded comment limit ]
  • line 300: list maps ListAccelerators results directly through toAttributes without fetching describeAcceleratorAttributes, so every listed accelerator is reported with flowLogsEnabled: false and no flow-log bucket/prefix even when flow logs are configured. This violates the provider's full-attributes list contract and gives callers an incorrect inventory snapshot (the current nuke path happens not to use those fields). [ Out of scope (post-validation triage) ]
  • line 316: read fetches tags only to decide whether to brand the same attrs as Unowned; tags are not included in Accelerator attributes. sync strips that brand and deep-compares the tag-less attributes, so removing or changing an accelerator tag is invisible and never calls the tag-repair logic in reconcile. The provider therefore does not converge tag drift unless some unrelated accelerator field also changes. [ Exceeded comment limit ]
  • line 392: The prop documents ipAddressType as defaulting to "IPV4", but reconciliation only updates it when news.ipAddressType !== undefined. If a deployed accelerator is changed from explicit "DUAL_STACK" to omitting the prop, the generic diff schedules an update, yet this branch skips the update and leaves the accelerator dual-stack indefinitely. Compare against news.ipAddressType ?? "IPV4" instead. [ Failed validation ]
  • line 417: When flow logs remain enabled but news.flowLogs.prefix is removed, the drift check deliberately skips prefix comparison because it only checks the prefix when it is defined. An accelerator previously configured with a non-empty prefix therefore keeps writing to the old prefix forever, despite the props documenting an omitted prefix as the root/default location. The reconciler should compare against an explicit default and send a valid value that resets the prefix. [ Exceeded comment limit ]
  • line 428: flowLogs.prefix is optional and the documented { flowLogs: { bucket } } form is allowed, but enabling logs sends FlowLogsEnabled: true with FlowLogsS3Prefix: undefined. AWS requires both FlowLogsS3Bucket and FlowLogsS3Prefix whenever flow logs are enabled, so this request is rejected and any accelerator configured without a prefix can never reconcile. Supply the documented default prefix (or otherwise ensure a valid prefix is sent). [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/GuardDuty/IPSet.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 145: toName calls createPhysicalName without passing the lifecycle instanceId, so its generated name uses the newly provided InstanceId from the plan context on each deploy. When an IP set is created successfully but state persistence then fails (or state is later lost), the next read/reconcile computes a different name, findByName misses the existing set, and createIPSet creates another active set while leaving the original orphaned. Use the prior instance identity or another stable recovery key when deriving the lookup name. [ Failed validation ]
  • line 250: The recovery path in reconcile accepts any IP set returned by findByName and never verifies hasAlchemyTags, even though read brands an untagged set as Unowned. If a previously managed output.ipSetId is gone and another actor creates an IP set with the same name, an ordinary deploy follows this fallback and updates its location/activation and rewrites its tags, bypassing the engine's adopt guard (existing-state plans do not rerun read). This can silently take over and mutate a foreign security list. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/KMS/Key.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 267: The output === undefined path always calls createKey and never searches for an existing key carrying the resource's internal tags. If createKey succeeds but the process crashes or state persistence fails before the output is written (a lifecycle case the provider contract explicitly requires providers to handle), the next reconcile creates another customer KMS key; the first key is orphaned and cannot be found or deleted by this provider. [ Exceeded comment limit ]
  • line 280: After a successful createKey, the provider immediately calls readKey, whose DescribeKey/tag/policy/rotation reads are not retried as a post-create consistency window. AWS documents KMS as eventually consistent and permits transient NotFoundException/InvalidStateException after changes; readKey converts NotFoundException to undefined, so a transient read can make this branch die at !state even though the key exists, leaving an untracked customer key (and the next retry creates another because there is no tag-based recovery). [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/KinesisVideo/SignalingChannel.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 228: The ResourceInUseException recovery after createSignalingChannel treats any channel that appeared after the initial observe as the stack's resource: it waits for ACTIVE and proceeds to TTL/tag synchronization without re-checking hasAlchemyTags. If another actor creates the same named channel in that race window, this provider mutates that unowned channel (including overwriting/removing tags), causing cross-stack resource corruption. [ Exceeded comment limit ]
  • line 228: When the initial observe sees no channel but createSignalingChannel keeps returning ResourceInUseException because a previous incarnation is still DELETING, the retry is exhausted and the error is converted to success at line 228. The code then only calls waitForChannelActive and never retries creation; once the deleting channel is purged, the name remains absent and the bounded wait fails with KinesisVideoNotConverged, so a normal replacement can fail instead of creating the channel. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/LakeFormation/DataCellsFilter.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 230: The replacement check compares raw tableCatalogId values instead of their effective value. A filter deployed with the documented default (omitted, which means the caller account) is marked replace when the user later writes that same account ID explicitly, even though the target catalog has not changed. This unnecessarily deletes and recreates the filter and can briefly disrupt grants that reference it. [ Out of scope (post-validation triage) ]
.repos/alchemy-effect/packages/alchemy/src/AWS/LakeFormation/DataLakeSettings.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 320: The provider identifies this resource as an account/region singleton (list always returns one settings row and delete only restores managed fields), but it does not set nuke: { singleton: true }. alchemy unsafe nuke therefore discovers the row, invokes delete with the un-managed attributes from list, gets a successful no-op, and reports the singleton as deleted even though the data-lake settings remain. It should be excluded from nuke like other singleton settings. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/MQ/Configuration.ts — 0 comments posted, 7 evaluated, 7 filtered
  • line 187: engineType and engineVersion are documented as immutable but are omitted from the provider's stables list. When this configuration has an in-place update (for example a data revision), the planner exposes only configurationId, configurationArn, and configurationName to downstream resources; references to configuration.engineType or configuration.engineVersion then resolve as missing during that plan, potentially sending incomplete values to dependent resources. Include the immutable engine fields as stable attributes. [ Exceeded comment limit ]
  • line 200: Replacement diffs for immutable engineType, engineVersion, and authenticationStrategy do not set deleteFirst: true. Those changes retain the same configurationName, so the engine's default create-first replacement calls createConfiguration while the old configuration with that unique name still exists; the create is rejected and the replacement can never complete. Name-changing replacements can remain create-first, but same-name immutable changes need delete-first handling (or another coexistence strategy). [ Failed validation ]
  • line 209: The immutable-auth comparison does not normalize the documented default. With authenticationStrategy omitted, Amazon MQ uses SIMPLE, but changing the props from omitted to explicit authenticationStrategy: "SIMPLE" (or back) compares undefined with "SIMPLE" and forces a replacement. This unnecessarily replaces a live configuration—and can disrupt brokers referencing it—even though the effective setting has not changed. [ Failed validation ]
  • line 213: The configuration provider's reconciliation logic cannot repair out-of-band drift on an unchanged deployment. reconcile syncs data and tags, but diff only checks immutable identity fields and read/toAttrs expose neither tags nor the revision content. Consequently, an operator changing the latest configuration revision or tags in AWS followed by a redeploy with identical props produces a noop and never runs the sync code. [ Exceeded comment limit ]
  • line 229: User tags can overwrite the ownership tags because desiredTags spreads internalTags first and news.tags second. For example, tags: { "alchemy::id": "other" } causes the created configuration to fail hasAlchemyTags on the next read, so the resource is treated as Unowned and normal updates/adoption cannot converge (the same collision can also break stack/stage ownership). Reserved alchemy::* keys must be applied after user tags or rejected. [ Exceeded comment limit ]
  • line 242: The create path has a check-then-create race. Two resources/deploys targeting the same configuration name can both see findByName(name) as absent, then one createConfiguration succeeds while the other receives the service's duplicate-name conflict; unlike the neighboring MQ broker provider, this error is not caught and re-observed. The losing reconciliation fails even though the desired configuration now exists, and a failed first apply/retry can leave the stack wedged until another run. [ Exceeded comment limit ]
  • line 273: description changes are never applied when the configuration data is unchanged. A deploy that changes only news.description still reaches reconcile because the props changed, but this condition only calls updateConfiguration when currentData !== news.data; with identical data it skips the API call and then persists the new props, so the old revision description remains in AWS permanently and future deploys no longer retry it. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/RedshiftData/StatementsHttp.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 83: StatementsHttp always adds redshift-serverless:GetCredentials to the host policy, even when options.secretArn selects Secrets Manager authentication. This gives the function an unnecessary alternate credential-minting capability (the comment itself says this permission is used only when no SecretArn is supplied); condition this statement on the IAM-auth path so secret-backed bindings do not receive it. [ Exceeded comment limit ]
  • line 220: When the bounded poll in query exhausts its times budget while Redshift still reports SUBMITTED, PICKED, or STARTED, this code falls through with that nonterminal response and emits RedshiftStatementFailed without calling cancel. The caller sees a failed query even though the SQL can continue running (and may continue consuming capacity or applying writes) after the request has returned; timeout handling should use a distinct timeout error and cancel the statement or otherwise make the orphaned execution explicit. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/SSMContacts/Rotation.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 128: rotationArn hard-codes the aws partition. In GovCloud and China, AWS SSM Contacts ARNs use arn:aws-us-gov or arn:aws-cn; the provider therefore cannot resolve an existing rotation (and, after creating one, the follow-up getRotation(arn) returns undefined before rotation.ContactIds is accessed). Deploys in those partitions fail and can leave the newly created rotation orphaned. [ Exceeded comment limit ]
  • line 164: RotationProps.startTime is optional and AWS CreateRotation permits omitting StartTime, but buildAttrs unconditionally calls rotation.StartTime.toISOString(). A rotation created without a start time (or an API response with the optional field absent) throws during the post-create return and on later reads, so reconciliation fails after creating the resource and can leave it orphaned. [ Exceeded comment limit ]
  • line 228: The ConflictException path assumes the concurrently existing rotation is safe to manage: after swallowing the conflict, line 228 fetches it and the provider immediately syncs contacts, recurrence, time zone, start time, and tags without rechecking hasAlchemyTags. If another stack/process owns the same explicit name and wins the create race, this provider overwrites that rotation instead of rejecting it, defeating the ownership/adoption safeguard. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/SageMaker/ClusterSchedulerConfig.ts — 0 comments posted, 5 evaluated, 5 filtered
  • line 307: Changing name returns a normal { action: "replace" }, which the engine performs create-first by default. SageMaker permits only one scheduler config per cluster, so the old config still occupies the cluster when the replacement create runs and AWS returns ClusterSchedulerConfigAlreadyExists; the name change can never converge. This replacement needs delete-first semantics (at least for same-cluster name changes). [ Exceeded comment limit ]
  • line 318: desiredTags is built as { ...internalTags, ...news.tags }, allowing user-supplied tags to overwrite alchemy::stack, alchemy::stage, or alchemy::id. A policy created with those reserved keys no longer carries this stack's ownership marker, so later reads classify its own resource as Unowned (or can attribute it to another stack), causing adoption failures and potentially incorrect ownership-driven lifecycle actions. [ Exceeded comment limit ]
  • line 327: When a persisted output.clusterSchedulerConfigId is gone, this fallback adopts whichever policy has the deterministic name without checking hasAlchemyTags. If the old policy was deleted and another actor creates a same-named policy, reconcile updates its scheduler config and rewrites its tags, even though read would classify that object as Unowned; this can silently mutate a foreign SageMaker policy. [ Exceeded comment limit ]
  • line 351: The create race handler catches only ConflictException, but SageMaker actually returns the typed ClusterSchedulerConfigAlreadyExists error for the one-policy-per-cluster conflict (the repository's own ClusterSchedulerConfig.test.ts asserts this tag). Therefore a concurrent create, or any existing policy on the cluster, propagates as a failed deployment instead of being re-observed by name as this code intends. [ Failed validation ]
  • line 368: reconcile treats any non-undefined description as an existing resource, but describeConfigOrUndefined preserves Status === "Deleted" (the provider itself handles that status elsewhere). With stale state pointing at a deleted policy, the else branch calls waitForConfig(..., "Ready"); waitForConfig classifies Deleted as SchedulerConfigNotReady, so deployment retries for the full bounded wait and fails instead of creating a replacement. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/SecurityHub/AutomationRule.ts — 0 comments posted, 8 evaluated, 8 filtered
  • line 146: getRule maps only Rules?.[0] and ignores the documented UnprocessedAutomationRules response from batchGetAutomationRules at line 146. When an existing ARN is temporarily or selectively unprocessed, it is misclassified as absent; reconciliation then falls through to name discovery/create and can create a second active rule instead of retrying or failing. [ Exceeded comment limit ]
  • line 150: getRule maps InvalidAccessException to undefined at line 150, and listRules similarly maps access failure to an empty list. A role that is allowed to create rules but not to batch-get/list them will therefore see an existing rule as absent and create another rule on every deployment, accumulating duplicate active automation rules until the quota is exhausted instead of surfacing the permission error. [ Exceeded comment limit ]
  • line 172: listRules treats InvalidAccessException as an empty account at lines 171–174. Since AWS uses this error for lack of permission, AutomationRuleProvider.list() will tell alchemy unsafe nuke that there are no rules and skip all deletion when credentials can list neither rules nor access the Security Hub account, silently leaving active automation rules behind. [ Exceeded comment limit ]
  • line 180: readTags converts every failure from listTagsForResource into an empty tag map at line 180. A transient/permission/API error therefore makes the subsequent diffTags treat every existing non-Alchemy tag as removed and call untagResource, silently deleting cloud metadata rather than failing safely. [ Failed validation ]
  • line 205: The provider uses the first listRules match by RuleName as the resource identity at lines 205 and 230, but Security Hub permits duplicate rule names (the API's own example lists two sample rule entries). Declaring two Alchemy rules with the same explicit ruleName can therefore make both state rows point to one ARN, update the wrong rule, and leave the other unmanaged; the same ambiguity occurs during state recovery/adoption. [ Exceeded comment limit ]
  • line 261: batchUpdateAutomationRules can return HTTP success with the target in UnprocessedAutomationRules, but this response is ignored at line 261. The provider then returns as if the update succeeded (and may persist stale attributes), so a rule can remain with its old criteria/actions/status without the deployment surfacing a failure. [ Exceeded comment limit ]
  • line 303: batchDeleteAutomationRules also reports per-ARN failures in UnprocessedAutomationRules, but the response is discarded and the effect succeeds at line 303. If AWS cannot delete the ARN, Alchemy removes the resource from state anyway, leaving the automation rule orphaned and still applying actions to future Security Hub findings. [ Exceeded comment limit ]
  • line 308: The delete path treats InvalidAccessException as successful deletion at lines 306–309. That error represents lack of permission (not proof that the rule is absent), so a role that cannot delete rules will lose the Alchemy state while the ARN remains in AWS and continues to run. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/StateStore/State.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 197: Stack and stage names are interpolated directly into S3 prefixes without escaping or boundary encoding. For example, state for stack foo/bar is stored below foo/, so deleteStack({ stack: "foo" }) also deletes the nested stack's objects; likewise a stage named prod/backup is deleted by deleting stage prod. The StateService accepts these names as strings and this implementation provides no validation, so one stack/stage can silently erase another. [ Exceeded comment limit ]
  • line 204: The S3 key mapping uses the non-injective encodeFqn/decodeFqn pair: both an FQN such as a/b and a logical ID/FQN such as a__b become the same object key, and decoding a__b.json always yields a/b. Because resource IDs are accepted as arbitrary strings, list() can return a different FQN (or two resources can overwrite one another), causing state lookups and lifecycle operations to target the wrong resource. [ Exceeded comment limit ]
  • line 204: OUTPUT_FILE is a reserved S3 key, but resourceKey does not reserve or escape it. A valid resource whose FQN/logical ID is __stack_output__ maps to exactly __stack_output__.json; list() filters that resource out, and setOutput/resource set overwrite each other's JSON. Subsequent reads can treat stack output as PersistedState or lose the resource/output entirely. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/VpcLattice/AccessLogSubscription.ts — 0 comments posted, 5 evaluated, 5 filtered
  • line 142: findByDestinationType filters only on the delivery service and ignores each summary's serviceNetworkLogType, even though that value is part of the subscription and is immutable. During cold-start adoption, a same-resource subscription for the same destination service but the opposite SERVICE/RESOURCE mode is adopted as if it matched; reconcile cannot change that mode, so the resource silently remains configured for the wrong kind of access logs. [ Exceeded comment limit ]
  • line 189: The replacement comparison treats omitted serviceNetworkLogType as different from explicit "SERVICE", even though this resource documents "SERVICE" as the default. Thus changing only the spelling from omitted to explicit default (or back) plans a replacement, causing needless subscription churn and, with the create-first/recovery path above, potentially deleting the existing subscription. Compare both sides after applying the "SERVICE" default. [ Exceeded comment limit ]
  • line 231: The provider merges three internal ownership tags into news.tags without accounting for VPC Lattice's tag limit. A user-supplied map at the API's 200-tag maximum becomes 203 tags and the create/tag operation is rejected, so this otherwise valid edge-case configuration cannot be provisioned. [ Out of scope (post-validation triage) ]
  • line 264: When serviceNetworkLogType changes, diff requests a replacement, but the replacement is create-first and the resource allows only one subscription per destination type. The create therefore ends in ConflictException, and this handler adopts the old subscription via findByDestinationType instead of forcing a delete-first replacement. The old subscription is then treated as the replacement generation and garbage-collected, so the deployment can delete the existing logging subscription without ever applying the requested log type. [ Exceeded comment limit ]
  • line 298: read returns the API's subscription.destinationArn, which this file documents as being normalized by AWS with a trailing :* for CloudWatch log groups, but reconcile persists the unnormalized news.destinationArn. With a normal log-group ARN (no suffix), every later sync compares arn:...:log-group:name:* to arn:...:log-group:name, reports drift, and re-enters reconcile even though normalizeDestinationArn says they are equal. Persist a canonical form or normalize the read attribute. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/AWS/Website/StaticSite.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 208: StaticSite now delegates builds to Command.Build, whose CommandExecutor defaults shell to false, but StaticSiteBuildProps has no shell field and this call does not set it. The previous Build.Command implementation always executed the command through a shell, so valid build commands such as cd app && npm run build, pipes, redirects, environment assignments, or quoted arguments now get split into literal argv tokens or fail with ENOENT; any site using those commands cannot build or deploy. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Bundle/Vite.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 117: The external-module check uses id.startsWith(root) rather than a path-boundary check. For a project rooted at /tmp/app, a linked workspace module at /tmp/application/... is outside the project but still passes this prefix test and is omitted from maybeExternalWorkspaces. hashViteInput then hashes only the root, so changes in that external workspace can be missed and a cached deployment can reuse stale worker output. [ Exceeded comment limit ]
  • line 192: fileName inserts path.relative(...) directly into Cloudflare Worker module names. On Windows the path service returns backslash-separated paths (for example dist\ssr), while the bundle contract requires POSIX module paths and the generated JavaScript imports use /. The resulting module names no longer match the import specifiers, so Vite/Cloudflare deployments built on Windows can fail to resolve chunks at worker startup. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/AI/GatewayDynamicRouting.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 353: The cold-read recovery at findByName only examines the first 50 routes, even though the same provider explicitly paginates listDynamicRoutings in listRoutes. With more than 50 routes and the target route on a later page, a lost routeId is treated as absent; reconcile then attempts a duplicate create, and its RouteAlreadyExists recovery uses the same incomplete lookup and rethrows instead of recovering the existing route. [ Exceeded comment limit ]
  • line 431: When renaming a route, the RouteAlreadyExists handler unconditionally deletes whatever route findByName returns before retrying the patch. If the requested name is already used by a legitimate, independently managed route (not a stale gateway ghost), this destroys that route and then transfers its name to the current route. The code has no ownership check that would justify deleting the holder. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Access/Application.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 331: The read path treats any failure from observeById as undefined (the helper swallows errors generically), and undefined is the engine's “resource is missing” signal. A transient GET failure such as a timeout or throttling response can therefore make a still-existing application enter the create path; if the subsequent create succeeds, Cloudflare gets a duplicate application and the persisted ID/audience is replaced, potentially breaking JWT validation and orphaning the original app. [ Exceeded comment limit ]
  • line 359: The cold-recovery path returns Unowned(attrs) for a domain match, but ApplicationProps.adopt is never read by this provider or mapped into the engine's Resource.Adopt policy. Consequently an application declared with adopt: true still fails with OwnedBySomeoneElse unless the unrelated global --adopt/adopt(true) scope is supplied, so the provider's documented per-application adoption option has no effect. [ Exceeded comment limit ]
  • line 440: The reconcile path uses bodyEqualsObserved to decide whether to call the update API, but its policy comparison only checks policy IDs/order (and sometimes precedence). The provider accepts per-application policy overrides such as approvalRequired, isolationRequired, purposeJustificationRequired, sessionDuration, and approvalGroups; changing any of those while keeping the same policy ID makes this condition report “in sync”, so the new override is never sent to Cloudflare. [ Exceeded comment limit ]
  • line 535: delete catches every failure from deleteAccessApplicationForAccount and turns it into success. A permission error, rate-limit/network failure, or any other non-NotFound error therefore still lets the engine delete the resource's state, leaving the Access application orphaned in Cloudflare while future deploys no longer know to manage it. Only an explicitly handled not-found condition should be ignored. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Access/Policy.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 240: The create path catches every failure, not just a duplicate-name/race failure, and then accepts any policy returned by findPolicyByName as success. For example, a validation, authorization, or service error during createAccessPolicy is silently converted into adoption of an existing same-named policy (and the later sync may PUT into it), hiding the original create failure and potentially mutating the wrong resource. [ Exceeded comment limit ]
  • line 323: Policy.delete converts every failure from deleteAccessPolicy into success via Effect.catch(() => Effect.void), rather than only ignoring a not-found response. A revoked/invalid credential, permission error, network failure, or Cloudflare outage therefore removes the resource from Alchemy state even though the Access policy remains deployed and may continue affecting applications. [ Exceeded comment limit ]
  • line 353: The cold-read path returns a plain attribute object for any same-named existing Access policy, without Unowned branding or another ownership check. The planner therefore silently adopts that foreign policy when adoption is disabled, and reconcile can then PUT news into it. A stack with a colliding name can consequently overwrite another policy's Access decision/rules instead of failing or requiring adopt. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Access/ServiceToken.ts — 0 comments posted, 7 evaluated, 7 filtered
  • line 170: previousClientSecretExpiresAt is documented as independently mutable, but diff only checks the account and otherwise returns undefined. Changing just this property therefore never schedules reconcile, so users cannot extend or immediately expire the previous secret unless they also change an unrelated field or bump clientSecretVersion. [ Failed validation ]
  • line 200: The cold name-recovery path silently adopts an existing service token even though Cloudflare never returns its secret from list/get. With no persisted output, read returns the token with clientSecret: undefined, and reconcile likewise reuses a name match without rotating it; a fresh deploy against a pre-existing token (or lost state) therefore succeeds but exposes an unusable credential to downstream bindings. The version fallback can also suppress rotation when it comes from olds. [ Exceeded comment limit ]
  • line 201: The read fallback also treats a same-name token as the cached token after the persisted ID disappears. Because names are not unique, a replacement/unrelated token can be selected and returned with output?.clientSecret from the deleted token, producing a persisted clientId/secret mismatch while the plan may otherwise be a no-op. Recovery by name must not be used as proof of identity for this write-only secret. [ Exceeded comment limit ]
  • line 241: After the cached serviceTokenId is gone, reconcile falls back to an exact-name lookup even though the implementation explicitly allows duplicate names. If token A is deleted out of band and token B with the same name exists, the provider adopts B but keeps output.clientSecret from A (unless a version bump happens); state then pairs B's clientId with A's secret and may also rename/rotate the unrelated token. This corrupts credentials and can mutate the wrong service token. [ Exceeded comment limit ]
  • line 242: findTokenByName is wrapped in a catch-all that converts any list failure into undefined. In reconcile, a transient API/rate-limit/permission failure at line 242 is consequently interpreted as absence and followed by createAccessServiceTokenForAccount, which can create a duplicate same-named token (the API does not enforce name uniqueness), leaving the original token orphaned and changing which credential the stack uses. [ Exceeded comment limit ]
  • line 257: The Effect.catch around createAccessServiceTokenForAccount treats every failure as a same-name create race. A transient/network, permission, or validation failure therefore falls through to findTokenByName; if any unrelated token has that name, the provider adopts it and returns success with no newly captured secret (output?.clientSecret is usually undefined). This silently binds the stack to the wrong credential instead of surfacing the create failure. [ Exceeded comment limit ]
  • line 295: Even when the generic plan detects a changed previousClientSecretExpiresAt and invokes reconcile, the sync condition only checks name and duration. With those unchanged and no version bump, no PUT/rotate request is made, so the previous secret's expiration is never extended or revoked for a change to this property alone. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Addressing/AddressMap.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 155: AddressMapProvider marks accountId stable but never supplies a diff that compares it with the active CloudflareEnvironment account. If the same stack/state is deployed after switching profiles, the plan sees unchanged props and treats the resource as a no-op, leaving the persisted map bound to the old account; later reads/deletes use that old account id and fail (and no map is created in the new account). Account-scoped providers need to return replace when output.accountId !== accountId. [ Exceeded comment limit ]
  • line 334: list() returns immutable Cloudflare-managed maps (canDelete: false), but this provider is not marked nuke: { skip: true }; delete() then immediately succeeds at line 334 without deleting them. Consequently alchemy unsafe nuke includes these maps in its targets and reports them as deleted even though they remain in the account, so repeated/account-wide teardown cannot accurately clean up or report the result. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Containers/LocalContainerProvider.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 125: The string dockerfile is documented as relative to context, but this branch resolves it directly with fs.realPath(news.dockerfile), which resolves relative to the process working directory. For the documented { context: "./app", dockerfile: "Dockerfile" } form, it looks for ./Dockerfile instead of ./app/Dockerfile and either fails the local deployment or produces an out-of-context path. Resolve the Dockerfile against the resolved context (while preserving absolute paths). [ Exceeded comment limit ]
  • line 139: Wrapping image preparation in Artifacts.cached can permanently hang later lifecycle calls after a failed first build. Artifacts.cached stores Deferred.await(deferred) before running the effect and only completes it on success; any bundling, filesystem, or Dockerfile error leaves the deferred unresolved. Because the local RPC provider process persists across retries, the next prepareImage call reads that cached effect and waits forever, making subsequent alchemy dev attempts hang until the provider is restarted. [ Exceeded comment limit ]
  • line 139: prepareImage is memoized only by container-image:${id} at line 139, but local providers run in the long-lived RPC server whose ArtifactStore persists across dev deploys. After the first build, editing the main bundle or any file in an external Docker context causes diff to receive the old hash from Artifacts.cached, so it returns no update and the running container continues executing stale code until the RPC server is restarted. The cache key must include current source content or be scoped to a single deploy rather than the persistent provider process. [ Exceeded comment limit ]
  • line 177: precreate receives raw props containing unresolved Outputs, but makeAttributes passes those props to makeContainerEnv before the inputs are evaluated. makeContainerEnv skips unresolved values under props.env but does not skip environmentVariables[].value, so a valid value such as { name: "BUCKET", value: bucket.bucketName } is copied as an Output into dev.env. The worker consumes the precreate dev object to start the local container before the final reconcile, causing an unresolved object to be handed to the Docker/runtime environment instead of the resolved string. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/CustomCertificate/CustomCertificate.ts — 0 comments posted, 5 evaluated, 5 filtered
  • line 261: certificateId and uploadedOn are declared stable, but Cloudflare's Custom Certificates API explicitly returns a new certificate ID and deletes the old ID when a sni_custom certificate is PATCHed. During a rotation, downstream resources therefore receive the old deleted ID as their stable reference and can keep configuring or observing a certificate that no longer exists. These fields must not be treated as stable for that update path (or the provider must model the replacement). [ Exceeded comment limit ]
  • line 376: The same ID-only observation is unsafe after an interrupted update: Cloudflare deletes the old sni_custom ID and returns a new one from a successful PATCH. If persistence fails after that PATCH, the next run still has the old ID; getCertificate returns undefined, and this branch uploads another certificate instead of recovering the already-created replacement. The patched certificate is then orphaned and future runs can accumulate duplicates. [ Exceeded comment limit ]
  • line 376: The reconcile path only observes by output.certificateId; when the creating/created state commit fails after createCustomCertificate succeeds, a retry has no output ID and falls through to another upload. Custom certificates have no uniqueness constraint, so this creates an orphaned duplicate certificate (and the original is not recoverable through state). The provider must recover by a deterministic lookup or otherwise make the greenfield retry idempotent. [ Failed validation ]
  • line 406: deploy is accepted as a mutable PATCH field, but it is neither included in desiredHash nor compared in the dirty checks. If a deployed certificate changes from staging to production (or vice versa) while the PEM/key are unchanged, certDirty and all other dirty flags remain false, so no PATCH is sent and the certificate stays in the old deployment environment. [ Exceeded comment limit ]
  • line 408: bundleMethod is documented with a default of "ubiquitous", but this dirty check only runs when news.bundleMethod is explicitly defined. After a certificate was deployed with "force" or "optimal", removing the property leaves the non-default bundle in Cloudflare forever because the provider neither detects the change nor patches it back to "ubiquitous". [ Failed validation ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/CustomHostname/CustomHostname.ts — 0 comments posted, 5 evaluated, 5 filtered
  • line 320: This list returns every custom hostname in every zone as ordinary Attributes, with no ownership filtering or Unowned marker. Provider list() is the input to alchemy unsafe nuke, which passes each returned attribute directly to delete; therefore running nuke with this provider enabled deletes unrelated customers' custom hostnames, not just resources created by the current stack. [ Exceeded comment limit ]
  • line 374: The create recovery uses Effect.catch, which catches every typed create failure rather than only an already-exists/conflict error. If creation fails for a validation, quota, permission, or transient server error and findByHostname happens to see a matching existing hostname, the provider treats that failed create as success, may PATCH that existing resource, and returns successful attributes instead of surfacing the original failure. [ Exceeded comment limit ]
  • line 380: The create-race fallback can bypass the custom hostname adoption guard: if read sees no hostname, another actor creates that hostname before createCustomHostname, and the create failure is followed by a successful findByHostname, this code adopts the found object as observed and proceeds to PATCH it without an Unowned/adopt check. Because custom hostnames intentionally have no ownership marker, an out-of-band customer hostname can be modified merely by racing deployment. [ Exceeded comment limit ]
  • line 385: justCreated is set to true after the entire create-and-recovery expression, including the branch where createCustomHostname failed and the handler returned an already-existing hostname from findByHostname. In that race/recovery case the resource was not created with desiredSsl, but !justCreated suppresses the SSL PATCH, leaving the recovered hostname permanently unconverged until a later run. [ Exceeded comment limit ]
  • line 401: The reconcile path claims to synchronize news.ssl, but the comparison at sslEqualsObserved ignores cloudflareBranding, customCertificate, customKey, and customCsrId (and narrowHostname never retains those observed fields). After any of these supported Ssl inputs changes on an existing hostname, this condition remains equal and no PATCH is sent, so the Cloudflare certificate configuration silently stays stale. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Devices/CustomProfile.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 277: The cold recovery lookup matches only name and does not exclude match.default === true. If the requested name equals the account's default profile name, read returns that singleton as an Unowned custom profile; after adoption, reconcile uses its policyId and PATCHes/deletes it through the custom-profile provider, allowing a custom resource to mutate the account-wide default WARP policy. [ Failed validation ]
  • line 511: list enumerates every item from listDevicePolicyCustoms but never filters out the account's default profile (p.default). Cloudflare's collection includes that singleton, so list() exposes it as a DeviceCustomProfile; account-wide teardown/import can then treat the non-deletable default policy as a custom resource and invoke this provider's delete path against it, risking a failed or destructive default-profile operation. [ Exceeded comment limit ]
  • line 514: In list, a profile is only converted to undefined when the main observeProfile GET sees DevicePolicyNotFound. If the profile is deleted after that GET but before buildAttrs fetches its three list endpoints, observeLists propagates DevicePolicyNotFound and aborts the entire account enumeration instead of dropping that vanished item as the comment promises. [ Out of scope (post-validation triage) ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Dns/Record.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 246: The cold-scan/reconcile path passes the user-supplied news.name directly to findByNameType, whose exact client-side comparison does not normalize names. Cloudflare returns DNS names in Punycode even when created with Unicode input; therefore a valid Unicode record is not recognized on a cold read or after state loss, and reconcile can create a second record (or miss the adoption safety check). Normalize both requested and observed names before matching. [ Exceeded comment limit ]
  • line 282: The DnsRecordAlreadyExists recovery treats any record found after the initial scan as safe to adopt and then updates it. If another stack or an operator creates the (name, type) record between read/findByNameType and createRecord, this branch runs even when adopt is false and overwrites the existing record's content and mutable fields. The race recovery must preserve the ownership gate instead of mutating a record merely because the API reported a conflict. [ Exceeded comment limit ]
  • line 368: delete converts every failure from dns.deleteRecord into success via Effect.catch(() => Effect.void), not just an already-missing record. A permission error, outage, or other failed deletion therefore causes the engine to remove the resource from state while the DNS record remains live, leaving teardown falsely successful and the record unmanaged. Catch only the documented not-found cases and propagate other failures. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Gateway/Rule.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 302: RuleProvider.diff only checks action and never compares the persisted output.accountId with the current CloudflareEnvironment account. Because provider stables do not themselves trigger a diff, redeploying the same stack/state against a different Cloudflare account can be planned as a noop, leaving the old account's rule attributes and ID in state instead of replacing or reconciling the rule in the new account. [ Exceeded comment limit ]
  • line 322: When no persisted ruleId exists, read returns undefined, but reconcile then unconditionally calls findRuleByName and treats any match as its own rule. Thus a first deploy with a colliding existing Gateway rule bypasses the planner's adoption/ownership check and updates that foreign rule even when adoption is disabled, potentially changing enforcement behavior. [ Exceeded comment limit ]
  • line 342: The createRule fallback catches all errors and treats a same-named list result as the successfully created rule. A non-conflict failure such as invalid request data, authorization failure, or a transient server error can therefore be hidden and converted into an update of an existing rule, rather than re-failing the original error or only recovering a duplicate-create race. [ Exceeded comment limit ]
  • line 409: Rule.delete swallows every deleteGatewayRule failure with Effect.catch(() => Effect.void). If deletion fails for authorization, transport, or service-availability reasons, the engine can mark the destroy successful and discard state while the Gateway rule remains active, leaving enforcement in Cloudflare and an orphaned resource. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Intel/IndicatorFeed.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 272: When createIndicatorFeed returns success without an id (the Cloudflare API schema makes both result and result.id optional), the fallback uses findByName(accountId, name), which intentionally selects the oldest matching feed. Because indicator-feed names are not unique, if a prior feed already has that name this selects the wrong feed rather than the one just created; subsequent updates/snapshot upload mutate the pre-existing feed and the newly created undeletable feed is orphaned. A missing-id create response should be treated as unrecoverable or correlated more safely. [ Exceeded comment limit ]
  • line 341: The provider enumerates feeds via list, but its delete implementation is only a warning/no-op because feeds have no delete API. Without declaring nuke: { skip: true }, alchemy unsafe nuke includes every indicator feed, invokes this no-op, and reports it as deleted even though it remains on the account (and will be discovered again on the next nuke). This makes account-wide cleanup silently incomplete and misleading. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/KV/ReadNamespace.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 31: ReadNamespaceClient makes raw an unconditional public operation, but the HTTP and local implementations construct this client with raw: Effect.die(...) because they have no native KVNamespace. Any caller using the advertised client.raw with ReadNamespaceHttp or ReadNamespaceLocal gets a runtime defect instead of a typed failure; make this capability optional or expose it only on the native-binding client type. [ Exceeded comment limit ]
  • line 175: The bulk getWithMetadata overloads advertise Map<string, KVNamespaceGetWithMetadataResult<...>>, but the non-native implementations fulfill array requests by calling the value-only bulk endpoint and wrapping every entry with metadata: null. Thus ReadNamespaceHttp/ReadNamespaceLocal silently discard stored metadata for getWithMetadata([...]), unlike the declared client contract and native KV behavior. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/MagicCloudNetworking/OnRamp.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 283: For an adopted on-ramp (output present but olds undefined), the replacement checks for region, cloudAsn, adoptedHubId, and hubProviderId never run because they are inside if (old !== undefined). Those properties are immutable according to this provider, yet reconcile then observes the adopted resource and only patches mutable fields, silently retaining an existing on-ramp with a different region/ASN/hub identity while reporting success. [ Exceeded comment limit ]
  • line 324: The on-ramp provider never treats its account scope as immutable. OnRampAttributes persists accountId, and all API endpoints are account-scoped, but diff has no check that output.accountId matches the current CloudflareEnvironment account. If a stack is redeployed under another account, reconcile first reads the old on-ramp using output.accountId, then patches it with the new accountId (or returns it as if it belonged to the new account on a no-op), rather than replacing it; this can fail reconciliation or associate the old-account resource with the new stack. [ Exceeded comment limit ]
  • line 374: The PATCH dirty checks ignore transitions from a configured optional value to an omitted value: description, vpc, attachment arrays, and management flags are only compared when their corresponding news field is defined. After a prior deployment sets (for example) description or attachedVpcs, removing that property from the desired OnRamp leaves the old cloud value in place and returns it as converged because no patch is issued. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/MagicNetworkMonitoring/Rule.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 214: The account replacement check only runs when news.accountId is a string. If a rule was deployed with an explicit account ID and the prop is later removed while the ambient profile now points at another account, the effective account changes but this diff returns no replacement. reconcile then looks in the new account and may create/adopt a second rule while leaving the original rule orphaned in the old account. [ Exceeded comment limit ]
  • line 241: A failed cached-ID lookup falls back to a name-only match, but the result is returned as owned whenever output exists. If the managed rule is deleted out of band and another actor creates a new rule with the same unique name, read returns that replacement as plain attributes, bypassing Unowned; the next reconcile can patch it (or destroy it) without the adopt policy. The name-only path cannot prove ownership, just as the cold-read comment acknowledges. [ Exceeded comment limit ]
  • line 313: dirty only compares optional rule fields when the new value is defined. If a deployed threshold rule has duration: "5m" (or a threshold/zscore setting) and the property is later removed from the desired Rule, every guarded comparison is false, so reconciliation returns the observed rule without calling patchRule; the old Cloudflare setting remains indefinitely. This also violates the documented duration default of "1m" when it is removed. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/MagicTransit/SiteLan.ts — 0 comments posted, 7 evaluated, 7 filtered
  • line 198: The diff exits whenever olds is undefined, but the provider contract explicitly uses that state for an adopted resource that already has output. As a result, an adopted LAN whose output.siteId differs from news.siteId, or whose output.haLink differs from the desired haLink, is treated as an in-place update. siteId cannot be changed in place and haLink is create-only, so reconciliation either targets/creates under the wrong site or leaves the adopted LAN's HA setting unchanged while recording it as converged. [ Failed validation ]
  • line 208: Even when prior props exist, the create-only haLink comparison uses olds.haLink rather than the observed output.haLink. If the LAN is changed out of band from false to true, a desired haLink: false still compares as equal, and updateSiteLan has no haLink field to repair it. The provider then persists the observed true value as if reconciliation succeeded and continues to miss the drift on later plans. [ Exceeded comment limit ]
  • line 236: The LAN provider also omits the account-scope replacement check. With persisted output from account A and the environment now targeting account B, reconcile looks up output.lanId only in B, fails to find it, then creates or adopts a same-named LAN in B while the LAN in A remains unmanaged. Since accountId is included in stable attributes and the endpoint is keyed by account, this should be a replacement rather than a silent new resource/orphan. [ Exceeded comment limit ]
  • line 267: After createSiteLan, the API returns the entire site's LAN array and LAN names are explicitly not unique, but this code selects the first LAN matching name (or even the first LAN overall). If two creates for the same name race, both responses can contain both LANs and this selects the lexicographically/response-first pre-existing LAN rather than the LAN just created. The resource then persists the wrong lanId, so later updates/deletes can mutate or destroy another LAN and leave the newly-created one orphaned. [ Exceeded comment limit ]
  • line 285: The PUT dirty check treats omitted optional fields as no-op instead of detecting that previously configured values must be cleared. For example, changing vlanTag from 30 to omitted (documented default 0), removing routedSubnets, or omitting staticAddressing to return to DHCP leaves the existing LAN configuration untouched because every such comparison is guarded by news.<field> !== undefined; the provider then reports the stale LAN as reconciled. [ Exceeded comment limit ]
  • line 291: The LAN dirty calculation does not compare the full mutable nested configuration. It only checks nat.staticPrefix, routed-subnet prefix/nextHop (via sameRoutedSubnets), and staticAddressing.address; changes to routed-subnet NAT, secondaryAddress, virtualAddress, DHCP relay/server fields, or reservations therefore skip updateSiteLan and are reported as converged even though the desired LAN configuration changed. [ Exceeded comment limit ]
  • line 351: list() only catches MagicWanUnauthorized for both the account site enumeration and each site's LAN enumeration. The same Magic WAN calls can return the typed Forbidden error for a token with partial/insufficient scope (the adjacent SiteWan provider handles both tags, and the LAN tests document this case). In that situation provider listing fails instead of returning the intended empty/partial result, breaking operations such as account-wide enumeration on otherwise valid credentials. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/R2/BucketEventNotification.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 209: diff never compares the persisted output.accountId with the current CloudflareEnvironment account. After deploying this account-scoped configuration and switching the profile to another account, the provider reports no replacement; reconcile continues using output.accountId, so the new account is not managed and the old account's notification remains attached. [ Exceeded comment limit ]
  • line 269: Because a notification configuration has no ownership marker, output does not prove that the configuration currently returned by getConfiguration is the one previously managed. If the old config is deleted out of band and another actor recreates the same (bucketName, queueId, jurisdiction) pair, this branch returns it as owned (output ? attrs : Unowned(attrs)), allowing the next reconcile to delete and replace that actor's rules without adoption approval. [ Exceeded comment limit ]
  • line 297: When rules drift, reconciliation deletes the live queue configuration at line 297 before attempting the replacement PUT. Any PUT failure leaves the notification configuration absent with no rollback; for example, the same file documents that overlapping prefix/suffix rules are rejected by Cloudflare, so changing a valid deployment to such a rule set permanently drops the previously working notifications while the deployment fails. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/ResourceSharing/Share.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 237: The cold-read path adopts the oldest sent share with a matching name as a normal owned resource, even though the code itself notes that share names are not unique. After state loss, an unrelated share with that name can therefore be selected, and the subsequent reconcile may rename it, remove its recipients/resources, or delete it during destroy. This lookup must not silently claim ownership without a positive identity check (or must be gated as unowned). [ Exceeded comment limit ]
  • line 265: When output is absent, reconcile always calls createResourceSharing without first looking up the deterministic name. If the create succeeds but the subsequent state commit fails or the process is interrupted, the retry enters this branch again and creates another share. The code explicitly notes that names are not unique, so Cloudflare cannot deduplicate this retry; repeated deploys can leave orphaned duplicate shares. [ Exceeded comment limit ]
  • line 298: Organization recipients cannot be reconciled correctly here: desired.organizationId is compared to r.accountId, but Cloudflare's recipient list/get response exposes account_id and does not expose organization_id. For a share declared with { organizationId: ... }, the live association therefore does not match, so each update tries to create it again; the removal loop also treats the existing association as unwanted and deletes it. This causes repeated recipient churn (or duplicate/create failures) for organization-targeted shares. [ Failed validation ]
  • line 336: Inline resource reconciliation keys entries only by resourceType and resourceId and, when a match is found, updates only meta. It never compares the desired resourceAccountId with the live entry. Because diff does not replace on resources changes, changing resourceAccountId (including from the default current account to an explicit owner) runs this path but leaves the old owner in Cloudflare, so the share silently does not converge to the declared resource. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Ruleset/CustomRuleset.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 248: The output === undefined branch unconditionally calls createRulesetForAccount and does not re-find an existing ruleset by the deterministic (name, phase, kind) identity. A successful Cloudflare create followed by a failed/interrupted state commit is retried with no output, creating another ruleset instead of converging. This violates the provider's required idempotent reconcile behavior and can orphan duplicate account rulesets. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Rum/Rule.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 182: listRules failures tagged Forbidden are converted to an empty result for that ruleset, with no retry or indication that enumeration was incomplete. Since provider.list() feeds account-wide alchemy unsafe nuke, a transient or permanent 403 causes every rule in that ruleset to be omitted and therefore never passed to delete; the nuke run can report completion while leaving those rules behind. Retry transient 403s and surface an unresolved listing (or otherwise make the omission explicit) instead of treating it as []. [ Exceeded comment limit ]
  • line 248: When the persisted output points at a ruleset that was deleted out of band (for example, its RUM Site was deleted), listRules returns undefined and this branch treats that as a missing rule. It then calls createRule with the deleted rulesetId, but the Cloudflare create endpoint requires an existing ruleset, so every update/recovery deploy fails instead of recreating or otherwise recovering the parent/ruleset. A missing ruleset must not be conflated with an absent rule. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/SecretsStore/Secret.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 178: The SecretNameAlreadyExists recovery blindly adopts and patches the name-matching secret after the initial read missed it. A concurrent stack or out-of-band creator can win between read and createStoreSecret; with adoption disabled, this path still sends news.value to that foreign secret, irreversibly overwriting its write-only value. Re-check ownership (or fail unless adoption is explicitly enabled) before patching the raced secret. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Tags/ZoneResourceTags.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 302: For tags: {}, desired is empty and recordsEqual returns true when getZoneTag reports the normal empty set, so reconcile returns a successful ZoneResourceTagsAttributes without issuing a PUT. However read treats that same empty set as undefined at line 260. Thus an empty-tag configuration produces state for a resource that does not exist and is lost on the next refresh/import; either reject empty tags or consistently model the empty set as present. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Tunnel/Route.ts — 0 comments posted, 5 evaluated, 5 filtered
  • line 174: reconcile calls observe by account/network before checking news.adopt, and observe deliberately scans routes attached to any tunnel. Thus, when a pre-existing route owns the requested CIDR, a resource with adopt: false still treats that foreign route as its own and the later sync PATCH can move it to news.tunnelId and overwrite its comment. The adopt flag is therefore bypassed for the normal pre-existing-route path, allowing one stack to take over another stack's route. [ Exceeded comment limit ]
  • line 174: The reconcile observation path looks up only CIDR and optional VNET, not the route type. Cloudflare's network-route collection contains non-cfd_tunnel routes (for example WARP connector and Magic routes), so a same-CIDR route of another type can be returned as observed; the subsequent PATCH sends the requested tunnelId and the provider persists it as a Tunnel Route. This can hijack or corrupt unrelated connectivity during an ordinary reconcile, independently of the adopt flag. [ Exceeded comment limit ]
  • line 177: When news.virtualNetworkId is omitted, reconcile passes undefined to the account-wide lookup. The lookup then accepts a matching CIDR from any virtual network, even though Cloudflare uses the account's default virtual network when the create request omits virtual_network_id and permits the same CIDR in separate virtual networks. If that CIDR exists only in a non-default VNET, the provider adopts/patches that route instead of creating the desired default-VNET route, moving traffic and recording the wrong virtualNetworkId in state. [ Exceeded comment limit ]
  • line 274: The delete handler catches every failure from deleteNetworkRoute, not just an already-missing route. Permission errors, rate limits, and transient Cloudflare/5xx failures are therefore reported as successful destroys while the route remains deployed, leaving state and cloud connectivity inconsistent with no retry or operator-visible failure. [ Exceeded comment limit ]
  • line 284: list enumerates every account-wide network route but does not filter tunTypes to cfd_tunnel. Cloudflare's route collection also returns warp_connector, warp, magic, ip_sec, gre, and cni routes; the provider maps all of them into this resource's attributes. Since provider listings feed account-wide teardown and each mapped item is delete-ready, alchemy unsafe nuke can pass non-Cloudflare-Tunnel routes to deleteNetworkRoute and remove unrelated connectivity. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Cloudflare/Tunnel/Tunnel.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 165: Adding list makes TunnelProvider resources eligible for alchemy unsafe nuke, but the provider's delete still wraps the API call in Effect.catch(() => Effect.void). Any real deletion failure (permission, rate limit, dependency conflict, or transient outage) is therefore reported to nuke as success, so it emits deleted while the tunnel remains in Cloudflare and is not retried or surfaced to the operator. Only an idempotent not-found result should be swallowed. [ Exceeded comment limit ]
  • line 275: After createTunnelCloudflared succeeds, the result is assigned to observed without checking that it has an id. The Cloudflare response schema allows result.id to be absent, and the subsequent observed.id! calls pass undefined to getTunnelCloudflaredToken (and possibly writeConfiguration) while returning an undefined tunnelId; the deploy then fails or persists unusable state, leaving the newly created tunnel orphaned. Re-read by name or fail explicitly when creation returns no id. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/GitHub/Environment.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 322: The custom policy sync does not deduplicate news.deploymentBranchPolicy.customBranchPolicies. If a config contains the same pattern twice, observedNames is never updated after the first create, so the loop POSTs the duplicate and GitHub rejects it (the API documents a 303 for an existing pattern), causing the whole reconciliation to fail instead of converging. [ Out of scope (post-validation triage) ]
  • line 431: list emits only environment attributes (environmentId, nodeId, name, etc.) and omits the required owner and repository location, but account-wide nuke passes each listed item as olds to delete. Consequently deleteAnEnvironment receives owner: undefined and repo: undefined for enumerated environments, so nuke cannot delete GitHub environments discovered by this provider. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Planetscale/MySQL/MySQLPassword.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 315: The branch-enumeration effect at listBranches.pages(...) does not handle PlanetScale's documented 403 Forbidden response. Because it is inside the outer Effect.forEach, one database that the token cannot read causes the whole provider list() to fail; the nuke scanner then converts that provider failure to an empty inventory, so passwords in all other accessible databases are skipped as well. Catch the per-database permission error (or otherwise retain partial results) rather than failing the full scan. [ Exceeded comment limit ]
  • line 350: listPasswords is only wrapped with Effect.catchTag("NotFound", ...), but PlanetScale documents a 403 Forbidden response when the credential lacks password-management/connect access. During list() (used by account-wide enumeration/nuke), one inaccessible branch therefore fails the entire provider instead of being skipped, so nuke/list cannot operate in organizations where the token can enumerate databases/branches but not every branch's passwords. Handle Forbidden (as the Postgres role provider does) or otherwise preserve partial results. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Planetscale/Postgres/PostgresDefaultRole.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 258: The branch-enumeration effect at listBranches.pages(...) does not handle PlanetScale's documented 403 Forbidden response. Since it runs inside the outer Stream.flatMap, a database inaccessible to the token terminates the entire default-role inventory stream; nuke then sees no default roles at all for this provider and leaves them undeleted. Handle a forbidden branch/database as an empty branch set (while preserving other databases) instead of failing the full scan. [ Exceeded comment limit ]
.repos/alchemy-effect/packages/alchemy/src/Sync.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 263: When provider.read returns an Unowned value, this code removes the ownership marker and treats the result as ordinary live state. It then calls provider.reconcile on drift without consulting AdoptPolicy. Providers use Unowned specifically when they found an existing cloud object but cannot prove this stack owns it, so a sync of a previously persisted resource can overwrite a foreign object's configuration (for example after a resource is deleted/recreated or its ownership tags change), bypassing the adoption safety gate. [ Exceeded comment limit ]
.repos/alchemy-effect/scripts/aws-leak-sweep.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 341: The SSM branch cannot preserve a valid single-component hierarchical parameter name. AWS allows /foo and foo as distinct parameter names, but ARN parsing can yield the same parameter/foo resource suffix; when raw is foo, this code stores foo and destroyLeak later calls DeleteParameter with Name: "foo", not /foo. A tagged /foo leak is therefore left behind, and if both names are present the cleanup can target the wrong parameter. Keep the original parameter name from the resource metadata or handle the leading-slash ARN representation explicitly. [ Exceeded comment limit ]
  • line 1054: The main sweep unconditionally adds every schedule returned by discoverSchedules to the deletion set. That discovery only checks whether the name contains -${stage}-; it does not verify any Alchemy tag or stack ownership. Running this tool with --delete can therefore delete unrelated user-created EventBridge schedules whose names happen to contain -test- (or the selected stage), causing service outages and permanent schedule loss. [ Exceeded comment limit ]
apps/desktop/scripts/probe-turnstile.cjs — 0 comments posted, 3 evaluated, 3 filtered
  • line 50: The probe creates the BrowserWindow with show: false and never focuses it, but clickPoint relies on webContents.sendInputEvent. Electron documents that sendInputEvent() only works when the containing BrowserWindow is focused, so both the email-field click and submit click are discarded in this probe. The later JS fallbacks can still submit the form, but no trusted mouse interaction is exercised, making the Turnstile/input-behavior result misleading. [ Out of scope (post-validation triage) ]
  • line 92: The chrome and chrome-runtime modes attach the DevTools debugger twice: the generic branch at lines 75–78 already attaches for both modes, then the Chrome-specific branch calls debugger.attach("1.3") again at line 92. Electron rejects a second attach, so either mode exits through the catch before Network.setUserAgentOverride (and before its intended probe) can run. [ Out of scope (post-validation triage) ]
  • line 225: runMode catches every navigation, DOM, debugger, and preload failure, writes it to the JSONL file, and then returns normally. The caller subsequently executes app.exit(0) regardless of those errors, so a failed probe (for example a missing preload or timed-out Auth0 navigation) still has a successful process exit status and cannot fail CI or a shell-based compatibility check. [ Out of scope (post-validation triage) ]
apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorModule.kt — 0 comments posted, 1 evaluated, 1 filtered
  • line 52: The Android module never declares or implements the onComposerSubmit event. The shared ComposerEditor API passes onSubmit for hardware Command-Return, but this Events list omits it and SelectionAwareEditText has no key/editor-action handler, so Command-Return on Android is consumed as ordinary multiline input (or ignored) and cannot trigger send. [ Cross-file consolidated ]
apps/mobile/modules/t3-composer-editor/android/src/main/java/expo/modules/t3composereditor/T3ComposerEditorView.kt — 0 comments posted, 3 evaluated, 3 filtered
  • line 216: setScrollEnabled(false) only toggles isVerticalScrollBarEnabled, which controls whether the scrollbar is drawn, not whether the EditText can scroll. The public scrollEnabled prop therefore has no effect on actual scrolling; a multiline editor with scrolling disabled can still scroll its content (and the collapsed composer can move its text out of view). [ Exceeded comment limit ]
  • line 266: updateInputFlags() conflates the independent autoCorrect and spellCheck props: unless both are true it adds TYPE_TEXT_FLAG_NO_SUGGESTIONS. Android documents that this flag suppresses dictionary candidates and overrides TYPE_TEXT_FLAG_AUTO_CORRECT, so autoCorrect=false, spellCheck=true disables spell checking, while autoCorrect=true, spellCheck=false also disables autocorrection instead of preserving it. [ Exceeded comment limit ]
  • line 377: ComposerChipSpan is only a ReplacementSpan, so it changes measurement/drawing but does not make the underlying token range atomic. Because the EditText remains freely editable and no selection/deletion override handles this span, placing the caret in a chip and typing or pressing backspace edits one character of the serialized @.../$... source; the token then becomes partially corrupted and disappears on the next token reconciliation, unlike the atomic attachment behavior expected by the composer. [ Exceeded comment limit ]
apps/mobile/modules/t3-composer-editor/ios/T3ComposerEditorView.swift — 0 comments posted, 1 evaluated, 1 filtered
  • line 539: droppedImageProviders only claims a drop when every provider is an image. A normal mixed drop (for example, an image dragged alongside text or another file) therefore returns drop.suggestedProposal, allowing UITextView's built-in image handling to insert an NSTextAttachment. serializedText only understands ComposerTextAttachment; unknown attachments are serialized as the attachment replacement character, so the image is not sent through onComposerPasteImages and the controlled draft receives a spurious \u{FFFC} instead. [ Exceeded comment limit ]
apps/mobile/modules/t3-native-controls/ios/T3NativeControlsModule.swift — 0 comments posted, 1 evaluated, 1 filtered
  • line 95: prepareShowcaseCapture deletes every matching generic-password and internet-password item because the SecItemDelete queries contain only kSecClass and no app-specific service, account, or other qualifier. When a showcase build runs this at app startup, it removes unrelated credentials/tokens stored by the app, not just showcase state, with no recovery path. [ Exceeded comment limit ]
apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt — 0 comments posted, 14 evaluated, 10 filtered
  • line 68: The visible-range callback always calls emitVisibleFile(first), and the Android implementation resolves index 0 to the first file even when the scroll offset is exactly zero. ReviewSheet uses scrollToTop() for the “All files” selection, so tapping that destination either leaves the previous file selected (if already at offset 0) or immediately selects the first file after scrolling to the top; the user cannot select the all-files state on Android. Handle the zero-offset case as a null file selection before resolving a file header. [ Exceeded comment limit ]
  • line 80: setTokensResetKey clears canvasView.tokensByRowId but does not invalidate an in-flight setTokensJson decode. If a large setTokensJson request is decoding when the reset key changes, its completion still has the current tokensDecodeGeneration and posts the old token map after the clear, so the new content can be rendered with stale syntax highlighting until another token update arrives. [ Exceeded comment limit ]
  • line 87: Changing contentResetKey clears tokens and offsets but leaves rows and visibleRows populated with the previous file/section. Until the asynchronous setRowsJson decode finishes, the canvas continues drawing and hit-testing the old diff; a tap in that window can emit actions for the prior content, and the visible-file callback can also report an old file after the reset. Reset the displayed rows (and invalidate the old row decode) when changing content so old data cannot remain interactive. [ Exceeded comment limit ]
  • line 91: setContentResetKey calls resetHorizontalOffsets() without stopping horizontalScroller. A horizontal code fling that is in progress when the diff section changes will continue in computeScroll() and reapply the old section's X position to the new rows, so the new section can immediately jump away from the requested horizontal reset. [ Exceeded comment limit ]
  • line 92: setContentResetKey marks the initial scroll pending, but immediately calls applyPendingInitialScroll() while the previous content is still in visibleRows. That consumes the pending flag and posts an offset based on the old rows; when the new setRowsJson result later replaces those rows, rebuildVisibleRows() has no pending scroll left to apply. Switching an already-populated source file (or another content set) therefore leaves the view at the old row position instead of honoring the new initialRowIndex. Clear the old rows or preserve the pending flag until the new rows are installed. [ Exceeded comment limit ]
  • line 151: setRowsJson resets lastVisibleFileId, but DiffCanvasView.lastVisibleRange is not reset and rebuildVisibleRows() does not force a visible-range notification. If a refresh replaces the diff with a different file at the same visible row indices/count, emitVisibleRange() sees the unchanged (first,last) pair and never invokes emitVisibleFile; the inspector therefore keeps the old file selection until the user scrolls. Reset the canvas visible-range cache or explicitly emit the current visible file after installing new rows. [ Exceeded comment limit ]
  • line 697: Updating rows rebuilds offsets but never invalidates lastVisibleRange. If a content/section change produces the same visible index range as the previous data (especially when already scrolled to the top), onDraw suppresses the next onVisibleRowsChanged callback as a duplicate. Since the wrapper resets lastVisibleFileId on every rows update, the new section's visible file is then never sent to onVisibleFileChange until the user scrolls far enough to change the range, leaving the file navigator stale. [ Out of scope (post-validation triage) ]
  • line 905: emitVisibleRange reports indices from rows, but rows is the collapsed visibleRows list supplied by rebuildVisibleRows, while the JS highlighter consumes those indices against the original uncollapsed data.rows. After collapsing a file, every later row index is shifted; for a collapsed file larger than the highlighter overscan, scrolling into a later file causes highlighting requests to target earlier rows, leaving the actually visible code unhighlighted (and potentially highlighting the wrong rows). [ Exceeded comment limit ]
  • line 1426: floatDp treats a present but nonnumeric/null JSON value as a valid metric: Android JSONObject.optDouble returns NaN on coercion failure, and this code stores that as NaN instead of using fallbackPx. For example, {"rowHeight":null} makes rowHeightPx become NaN; the row-height conversion then collapses rows to the minimum 1 pixel (and invalid widths/paddings can similarly corrupt drawing) rather than safely retaining the fallback style. [ Out of scope (post-validation triage) ]
  • line 1429: floatSp converts every JSON font size with density, which ignores Android's user font-scale factor (scaledDensity/SP conversion). On a device with a non-default system font size, the canvas text remains at the default physical size instead of scaling with the rest of the app, so the new style path breaks the platform's text-size accessibility behavior. [ Exceeded comment limit ]
apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/T3TerminalView.kt — 0 comments posted, 7 evaluated, 5 filtered
  • line 66: The themeConfig setter reparses each new config, but the parser only replaces paletteColors when the new config contains at least one palette entry and never resets cursorColorValue when cursor-color is removed. Changing from a themed config to an empty/smaller config therefore sends stale palette/cursor values through applyTheme(), so the terminal keeps colors from the previous theme instead of reflecting the current props. [ Exceeded comment limit ]
  • line 81: The autoFocus setter has no oldValue/field == value guard, so every native prop update with autoFocus = true calls requestKeyboardFocus(). This view receives frequent updates as initialBuffer changes; after a user dismisses the keyboard, the next terminal-output update can refocus the hidden EditText and reopen/retain the IME, making it impossible to keep the terminal keyboard closed while output is arriving. [ Exceeded comment limit ]
  • line 83: autoFocus calls requestKeyboardFocus() synchronously from the prop setter. Expo/RN creates the view and applies its initial props before inserting it into the window, so inputView has no window token when showSoftInput() runs; unlike the removed constructor post { ... } path, this class has no attach-time retry. As a result, the default autoFocus terminal can mount focused but with the soft keyboard not shown until the user taps it. [ Exceeded comment limit ]
  • line 288: The early return only compares cols and rows, even though fontSize changes terminalCanvas.cellWidthPx and cellHeightPx. If a font-size change leaves the computed grid dimensions unchanged (for example while a dimension is clamped at 2/400/200, or for a small size change), nativeResize is skipped and Ghostty retains the old pixel cell dimensions. Terminal pixel-size queries then report stale cols * cell_width/rows * cell_height values until a later grid-count change. [ Out of scope (post-validation triage) ]
  • line 423: parseThemeConfig never resets paletteColors when the new config contains no palette entries. After a theme with a palette is applied, setting themeConfig to an empty or palette-free config still passes the old palette to nativeSetTheme, so the terminal keeps colors that are absent from the current configuration instead of reverting to defaults. [ Out of scope (post-validation triage) ]
apps/mobile/modules/t3-terminal/android/src/main/java/expo/modules/t3terminal/TerminalCanvasView.kt — 0 comments posted, 3 evaluated, 3 filtered
  • line 278: A long-press is always forwarded to startWordSelection, even when a selection is already active. If the press lands on a blank cell, nativeSelectWordAt returns false (ghostty_terminal_select_word documents no selectable word), and startWordSelection only resets selectionActive without calling clearSelection; the native selection therefore remains highlighted while the canvas believes no selection is active. Subsequent taps no longer clear it, leaving stale selection state (and potentially a stale action toolbar). [ Exceeded comment limit ]
  • line 395: extendSelectionTo can shrink the word selected by the initial long-press. For a word spanning columns 0..4, pressing column 2 then moving to column 3 takes the else branch and sends (anchor=0, end=3) to the native delegate, dropping column 4; moving toward column 1 similarly drops column 0. The stored wordEnd*/wordStart* values are only used as the anchor, so the claimed word-granular selection is not preserved while dragging within the original word. [ Exceeded comment limit ]
  • line 447: The handle hit position is below its cell (handleCenterY adds handleRadius()), but handle drag coordinates are later converted with rowAt, which treats that below-cell position as the next terminal row. As a result, after grabbing a handle on any non-bottom-row selection, the first move at or near the visible handle is interpreted as row + 1 and changes the selection even before the finger has moved to another row; vertical handle dragging is consistently offset downward. [ Exceeded comment limit ]
apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 279: The readiness condition at line 279 can be satisfied before appearance preferences have loaded. themeApplied is computed from themeIds, which are initially resolved from storedPreferences ?? {} and therefore equal the default palette while the preference load is still pending; if the requested showcase theme is that default, it becomes true even when the persisted light/dark themes will later differ. Because this final effect does not also require appearancePreferencesReady, a fast fixture hydration can write the native ready marker and let the harness capture the persisted theme instead of the requested one. [ Exceeded comment limit ]
apps/mobile/src/features/threads/NewTaskContextPickerScreens.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 251: The checkout decision trusts branch.current from the refs list, even though the flow explicitly notes that this cached flag can lag an out-of-band checkout. If another process switches the project after the list is loaded, the stale row can still say current: true; selecting it skips switchRef, then flow.selectBranch records that branch while the repository remains on a different branch. Use the live checkout state (or compare against currentCheckoutBranchName) when deciding whether a local selection needs checkout. [ Exceeded comment limit ]
apps/mobile/src/features/threads/NewTaskRouteScreen.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 119: When an incoming share has a reservation whose destination project is not currently in projects, the rows remain enabled and selectProject awaits releaseShareReservation before navigating. A double-tap (or two taps during a slow release) starts two calls; both releases can succeed/idempotently no-op, and each then dispatches StackActions.push("NewTaskDraft", ...), leaving duplicate draft routes for the same share and potentially running the import against the shared draft twice. Add a synchronous selection/in-flight guard or disable the rows while the release is pending. [ Exceeded comment limit ]
apps/mobile/src/features/threads/threadListV2.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 431: The settled-tail comparator only considers latestUserMessageAt and then updatedAt, but settled recency is based on settledAt for explicit settles and the latest timestamp across latestTurn activity when auto-settled. Consequently, a thread explicitly settled now (or one whose agent turn completed after its last user message) can be ordered behind much older settled threads, so the recency tail shows the wrong history order. [ Exceeded comment limit ]
apps/mobile/src/native/T3ComposerEditor.native.tsx — 0 comments posted, 2 evaluated, 2 filtered
  • line 245: The Android path now passes the configured family (for example DMSans-Regular) to the native view, but the corresponding Android setter maps every non-Mono name to Typeface.DEFAULT instead of loading/using that family. Consequently both Android composer screens render in the system default font after this implementation replaces the React Native TextInput path. [ Out of scope (post-validation triage) ]
  • line 299: ComposerEditorProps exposes onSubmit, and ThreadComposer passes handleSend to it, but this Android implementation never forwards that callback to NativeView (and the Android native view has no submit event prop). As a result, Command-Return/hardware-keyboard submission silently does nothing on Android even though the same component supports it on iOS. [ Exceeded comment limit ]
apps/server/src/mcp/PreviewMidsceneService.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 368: The operation's AbortController is wired only to the MCP/Effect interruption signal; broker.invoke has no host-disconnect signal or callback wired into this service. If the desktop host disconnects while Midscene is awaiting a model response (between broker calls), run(agent, controller.signal) continues instead of terminating. When it later requests another browser operation, the broker may have failed over or the operation can keep running against a stale tab, violating the required host-disconnect cancellation boundary. [ Exceeded comment limit ]
  • line 433: The query callback always supplies a usage object via usageFromCall(queryResult.usage). When Midscene returns no AIUsageInfo (allowed by PreviewMidsceneAgentHandle.query), this produces an all-zero usage value, so execute never uses its agent.metrics() fallback. Queries therefore report zero tokens/calls even though the agent performed a model request, while act and assert correctly fall back to metrics. [ Exceeded comment limit ]
apps/server/src/provider/Layers/CodexAdapter.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 676: parentThreadId is a provider thread identifier, but this is emitted as parentAgentId, which the client treats as a task/agent ID. Every child with a parentThreadId is therefore classified as a workflow_agent and grouped under that ID; because no task with the parent provider-thread ID is created, the child becomes an orphan/direct entry instead of a normal Codex child with the intended hierarchy and kind. [ Failed validation ]
  • line 733: When a child turn finishes with turn.status === "failed", the mapper emits only task.updated with status: "failed" and drops turn.error (the Codex schema documents this field as populated on failed turns). The persisted task then shows a generic failure with no error detail in the Agents panel, losing the provider's actual failure reason. [ Exceeded comment limit ]
apps/server/src/pullRequest/BitbucketPullRequestProvider.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 125: listChangeRequests drops input.involvement and input.viewer when calling Bitbucket's API, and the service does not apply involvement locally (it only applies the separate row filters). Consequently the Bitbucket "authored" and "reviewing" list modes return all pull requests for the selected state/query instead of only requests involving the signed-in user. [ Failed validation ]
apps/server/src/resourceTelemetry/Model.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 472: Process identity is inconsistent across native and synthetic Electron samples. Native samples keep their own startTimeMs at identityKey, while a missing native process is synthesized with metric.creationTimeMs; these timestamps are explicitly treated as the same process within a 2-second tolerance at lines 420-423, but processIdentityKey uses exact values here. If the native timestamp is 1000 and the desktop timestamp is 1500, a temporary native omission changes the key from 300:1000 to 300:1500, causing a false exit/start and resetting CPU/I/O deltas; when native data returns it flips back again. [ Exceeded comment limit ]
  • line 514: preservePreviousRates is applied to synthetic Electron processes during every desktop-only merge. When a process is absent from the native snapshot, process is synthesized from the current desktop metric, but if it existed in input.previous, this branch returns the old previous.process.cpuPercent instead of the metric's current CPU percentage. Thus a desktop update changing an Electron process from (for example) 10% to 80% still publishes 10% until the next native update, defeating the desktop telemetry refresh for native-invisible Electron processes. [ Exceeded comment limit ]
  • line 536: When a desktop Electron metric has no cumulativeCpuSeconds, the synthetic process created at lines 409-416 can carry a fractional CPU time because the fallback calculation does not round its elapsed CPU increment. This value is copied unchanged into the returned process at cpuTimeMs here, although the contracts require ResourceTelemetryProcess.cpuTimeMs to be a non-negative integer. For example, 12.34% over 1_000 ms produces 123.4, yielding an invalid telemetry snapshot when it is validated or encoded. [ Exceeded comment limit ]
  • line 575: The new merge path invokes orderProcessTree on every snapshot at this line, and that helper recursively visits each child. A sufficiently deep process chain can exceed Node's call-stack limit and throw RangeError: Maximum call stack size exceeded, causing telemetry refreshes to fail. The iterative depth calculation does not make this ordering traversal stack-safe. [ Out of scope (post-validation triage) ]
apps/server/src/textGeneration/PiTextGeneration.ts — 0 comments posted, 4 evaluated, 4 filtered
  • line 161: runPiJson treats the first agent_end event as generation completion, but the Pi event handling code distinguishes retrying agent_end events (willRetry: true) from terminal completion and waits for agent_settled/a terminal event. When a provider transiently fails and Pi starts an automatic retry, this code immediately reads get_last_assistant_text from the incomplete attempt, commonly yielding empty/invalid output or stale text and failing the generation instead of allowing the retry to finish. [ Exceeded comment limit ]
  • line 163: The completion listener is forked with plain Effect.forkScoped, so its Stream.fromPubSub subscription is established asynchronously and the PubSub has no replay buffer. A very fast Pi/OMP process can publish agent_end after the prompt is accepted but before this child fiber subscribes; the event is then lost, Fiber.join(agentEnd) never completes, and the request waits until timeout instead of returning the generated result. [ Exceeded comment limit ]
  • line 165: For generateBranchName and generateThreadTitle, the input attachments are reduced to metadata by the prompt builder, and the prompt command at line 165 sends only message—it never reads or supplies the attachment bytes in Pi RPC's images field. Thus image-only/UI requests are presented to Pi as filenames and sizes despite the prompt telling the model to use attached images, producing titles/branches without the user's screenshot context. [ Exceeded comment limit ]
  • line 165: The only PI_TIMEOUT_MS guard wraps Fiber.join(agentEnd) after the set_model, optional set_thinking_level, and prompt RPC requests have already completed. Those client.request(...) calls have no timeout, so a Pi/OMP process that stays alive but stops replying (including during startup/model selection or prompt acknowledgement) leaves text generation and its request fiber blocked indefinitely instead of producing the advertised timeout error. [ Exceeded comment limit ]
apps/web/src/components/ContextPanel.tsx — 0 comments posted, 2 evaluated, 2 filtered
  • line 146: The panel unconditionally starts threadContext for every mounted server thread. The server implementation of this RPC rejects threads without an active provider session (session: null or a stopped/expired session), but the panel has no guard for that state and therefore shows a request error instead of context for historical/completed supported-provider threads. Since the panel is offered based on provider kind rather than session activity, opening Context on an old thread reliably hits this path. [ Exceeded comment limit ]
  • line 181: contextQuery.data.provider is the driver slug returned by the context RPC, so an OMP session supplies "omp" (and the built-in Pi driver supplies "piAgent"). Because this value is preferred over thread.session.providerName at line 181 and formatProviderDisplayName has no mappings for either slug, the Provider cell renders Omp/PiAgent instead of the configured user-facing names such as Oh My Pi/Pi whenever the context request succeeds. [ Out of scope (post-validation triage) ]
apps/web/src/components/search/ProjectContentSearchDialog.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 274: The per-file badge uses group.matches.length, but groups is built from visibleMatches (only the first 100 results). A file with more than 100 matches is therefore shown as having only 100 matches until the user scrolls far enough to load more, even though the search result set already contains the full count. Compute the badge from all matches for that path, or label it as a visible-count. [ Out of scope (post-validation triage) ]
apps/web/src/components/settings/SettingsPanels.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 1655: When the fallback input is focused, discoverInstalledFonts() can resolve while the user is still typing. This branch then unmounts the input and mounts FontFamilyPicker with selectedFamily={trimmed}, which is derived from the committed value, not the local draft; any draft text (and its pending timer) is discarded without being committed. A user who types during the permission/enumeration request can therefore lose the font name they entered. [ Failed validation ]
apps/web/src/components/settings/settingsSearch.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 82: environment-identification is included in the search catalog for every environment, even though the corresponding settings row is conditionally omitted when resolveEnvironmentIdentificationPillLabel returns null (for example, the existing tests show Latest and Alpha do so). Searching for it in those environments returns a result whose appearance target contains no such setting. [ Out of scope (post-validation triage) ]
  • line 109: font-smoothing is always returned by searchSettings, but the matching FontSmoothingRow returns null unless navigator.platform is macOS. On Windows/Linux, searching for this title produces a result with no font-smoothing anchor, so selecting it cannot reach the requested setting. [ Out of scope (post-validation triage) ]
  • line 167: The start-from-origin result is advertised unconditionally, but its actual SettingsRow is rendered only when settings.defaultThreadEnvMode === "worktree" (otherwise the row does not exist). Clicking this result only targets the parent new-threads anchor, so in the default/local mode the search result lands on a section without the requested setting and gives no way to reach it. [ Exceeded comment limit ]
apps/web/src/themePalette.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 1244: canvas is parsed through parseThemeRgbColor, which returns only the source RGB and discards its alpha. Alpha-bearing canvas values are supported by the editor, so a canvas such as rgb(0 0 0 / 50%) is actually rendered composited over the app background, but all derived foregrounds, surfaces, and canvasIsDark decisions here treat it as opaque black. Editing any advanced family after saving such a palette can therefore produce incorrect contrast choices and unreadable/mismatched derived colors; the canvas must be composited against the real backdrop (or alpha handled consistently) before deriving colors. [ Exceeded comment limit ]
native/resource-monitor/src/main.rs — 0 comments posted, 3 evaluated, 3 filtered
  • line 352: The refresh at self.system.refresh_processes_specifics(ProcessesToUpdate::All, ...) requests command lines for every process, while the later MAX_PROCESS_COMMAND_BYTES truncation only applies when constructing selected output rows. sysinfo therefore retains the full command-line arguments for all processes in System; a local workload with many processes/large arguments can make the sidecar allocate far beyond its advertised 64 MiB history/16 KiB command bounds on every sample, causing severe memory pressure or monitor termination. [ Exceeded comment limit ]
  • line 431: processes.sort_by_key emits the snapshot in numeric PID order rather than the documented depth-first process-tree order. With a tree such as root 100 -> child 200 -> grandchild 300 and another child 150, the emitted list is 100, 150, 200, 300, so a consumer of the native snapshot cannot treat contiguous rows as complete subtrees and renderer collapse/expansion can attach rows to the wrong tree structure. Preserve the traversal order (or emit explicit depth information) instead of sorting by PID. [ Out of scope (post-validation triage) ]
  • line 482: matches_external_identity floors the Electron timestamp by comparing it to the whole-second native value, but the Electron publisher rounds creation times to milliseconds. If the real process start is in the final 0.5 ms of a second (for example native 10_000 ms and Electron 10_000.0? specifically native 10_000 ms with rounded Electron 11_000 ms), the rounded value falls into the next bucket and this rejects the still-matching PID. That external root and all of its descendants are then omitted for the process lifetime. [ Out of scope (post-validation triage) ]
packages/shared/src/usageMerge.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 294: The merge treats bucket.costSource === "providerReported" as proof that every record in the bucket was provider-reported. UsageAggregator can put provider-reported and model-priced (or unpriced) records for the same provider/model/day into one bucket and marks such a mixed bucket modelPriced, while retaining unpricedRecords. Consequently providerReportedShare is understated (often zero) and modelPricedShare is overstated, so the UI reports incorrect cost-quality provenance for mixed buckets. [ Exceeded comment limit ]

- 新增完整的 TailwindCSS 配置与主题变量定义
- 实现窗口控件相关的样式与自定义变体支持
- 添加暗黑模式及桌面运行环境专属样式
- 优化滚动条、聊天 Markdown 渲染及代码块样式
- 主入口 main.tsx 结构化,兼容 Electron 与浏览器环境
- 集成 Clerk 认证支持,自动选择 Electron 或 Web 版本
- 配置字体资源与全局样式导入,确保 UI 统一风格
- 引入 midscene/core 依赖,支持内置“midscene-preview”技能
- 新增 bundledSkills 模块,提供技能路径解析和 Codex 服务初始化封装
- 在 CodexProvider 和 CodexSessionRuntime 中替换初始化逻辑,调用封装函数初始化带技能支持的 Codex App Server
- 在服务器构建脚本中复制 bundled-skills 目录至发布目录,确保技能资源包含在构建产物中
- 修改桌面应用命名,将“Chinese”标签更改为“Browser”
- 更新预览管理器,增加基于视口坐标自动滚动的功能
- 新增 PreviewMidsceneToolkit 及其运行时和处理器,实现对内置浏览器的控制和验证能力
- 扩展 contracts 包,添加 previewMidscene 相关接口定义
- 更新 .gitignore,忽略 openspec 目录
- 维护测试覆盖,新增对技能加载及预览 midscene 工具包的单元测试
- 添加 Midscene 模型配置解析和环境变量覆盖逻辑,支持从服务器设置动态加载配置
- 允许在代理中揭示隐藏目标标签页,确保 Midscene 代理创建前标签页可见
- 在 PreviewMidscene 运行时中注入模型配置参数,支持多模型动态切换
- 服务器设置支持安全存储和隐匿 Midscene API Key,避免密钥泄露到客户端
- 新增并完善 Midscene 相关单元测试,覆盖配置应用及错误处理逻辑
- 在前端添加 Midscene 配置界面国际化文案(中英文)
- 优化预览自动化的错误重试逻辑,增强 UnknownVizError 捕获与重试机制
- 修改 ChatView 组件,增加线程错误显示及隐藏功能,实现错误优先级和错误免打扰管理
- 修复 auth 模块中瞬态引导错误检测,支持因果链递归判定和传输错误判别
- 优化 Electron 预加载脚本导入,使用 contracts 新增的 preview-automation-features 模块导入方式
- PreviewAutomationHosts 中改进预览 Webview 可见性判定和等待逻辑,确保 DOM 渲染稳定后执行操作
nolaurence and others added 20 commits July 22, 2026 00:10
为推理型 provider(Codex reasoning / Claude thinking / Pi thinking_delta /
OpenCode reasoning)增加端到端的思维链展示能力。推理内容与 assistant 文本
共享同一消息 ID,以可折叠的「Thinking」区块在 UI 中渲染。

- contracts: OrchestrationMessage.reasoningText(可选)、ThreadMessageSentPayload.reasoning、
  thread.message.assistant.reasoning.delta 命令
- decider/projector: 新增 reasoning delta 命令分支,流式合并 reasoningText
  (streaming 追加 / completion 替换)
- ingestion: 提取 reasoning_text / reasoning_summary_text 增量,复用
  createBufferedTextStore 缓冲逻辑,遵循 enableAssistantStreaming 投递模式
- 持久化: ProjectionThreadMessage 增加 reasoningText,SQLite upsert/SELECT
  补 reasoning_text 列,新增 migration 033
- ProjectionPipeline / ProjectionSnapshotQuery: 投影与读取模型双向贯通
- UI: AssistantReasoningBlock 可折叠组件,流式时自动展开、完成后自动收起
- i18n: 中英文 chat.reasoning.* 键

验证: vp run typecheck / vp check 通过;ingestion 42/42、orchestration+
persistence 179/179、contracts+client-runtime+web 452/452 测试通过

Co-authored-by: traeagent <traeagent@users.noreply.github.com>
feat: 支持显示思维链功能
- 在右侧面板新增“上下文”选项卡支持上下文内容查看
- ChatComposer 组件支持打开上下文面板的回调属性传递
- ContextWindowMeter 组件添加打开上下文面板按钮
- 支持根据提供商 (codex、piAgent、opencode) 控制上下文面板可用性
- 服务端 ProviderAdapter 增加 readThreadContext 方法用于获取线程上下文原始消息
- CodexAdapter、OpenCodeAdapter、PiAdapter 分别实现原生上下文读取接口
- ProviderService 新增对应的线程上下文读取RPC接口及验证逻辑
- 升级 RPC 合约,增加 getThreadContext 方法支持上下文获取
- 消息时间线过滤函数改用 workEntryShouldBeVisible 以优化可见日志条目
- 添加上下文面板相关国际化文案,包括中文和英文
- 添加上下文面板相关单元测试覆盖子代理任务工作状态渲染
- rightPanelStore 增加 context 类型支持及对应状态存储版本升级
- RightPanelTabs 组件集成上下文面板入口及可用性控制
- server.test.ts 中 ProviderService 服务模拟添加以支持上下文相关测试环境
- 在简介中添加 Pi 供应商支持说明
- 更新安装说明,新增 Pi 供应商安装和认证步骤
- 修正并补充供应商指南链接,包括 Pi 的文档链接
- 新增桌面应用打包指南,涵盖 macOS、Linux 和 Windows 平台
- 详细列出各平台打包命令及对应架构说明
- Surface subagent lifecycle details and activity in the right panel
- Preserve runtime event data and add coverage for status mapping

� Conflicts:
�	apps/server/src/orchestration/projector.test.ts
- Retry transient electron-builder network failures up to two times
- Add coverage for retry classification and limits
Squashes 840 upstream commits after f61fa94.
Upstream tip: b60a2c0
- Add OMP provider support across server, web, contracts, and documentation
- Bundle provider runtime tests and settings integration
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ce6bbbe1-9e9f-42b1-9e64-77bfdd2f9d30

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nolaurence nolaurence closed this Aug 25, 2026
@github-actions github-actions Bot added the vouch:unvouched PR author is not yet trusted in the VOUCHED list. label Aug 25, 2026
@nolaurence
nolaurence deleted the codex/bundle-midscene-preview branch August 25, 2026 09:11

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Effect service conventions

This branch adds several new Effect service modules (apps/server/src/mcp/PreviewMidsceneService.ts, apps/server/src/mcp/preview-midscene/PreviewMidsceneRuntime.ts, apps/server/src/provider/pi/*, apps/server/src/bundledSkills.ts, apps/server/src/provider/Drivers/OmpDriver.ts). PreviewMidsceneRuntime.ts and OmpDriver.ts follow the conventions; four issues below are worth fixing in the new service code.

Note: the PR is 13.8k files against main because the fork branch carries a squashed upstream sync, so only the fork-introduced service modules were reviewed.

Posted via Macroscope — Effect Service Conventions

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

UI consistency review

Three concrete issues in the changed web UI scope (apps/web/src). The rest of the web diff is mostly i18n string threading, which looks consistent with the shared primitives.

  1. apps/web/src/index.css — the composer/context-strip glass composition was deleted and replaced by an opaque footer background.
  2. apps/web/src/components/ChatView.tsx — the bottom spacer lost its 1rem / 1.25rem breathing room, so the composer footer sits flush against the viewport bottom wherever env(safe-area-inset-bottom) is 0.
  3. apps/web/src/components/ContextPanel.tsx — the new refresh control is a raw <button> that recreates Button, dropping the primitive's focus ring, pointer cursor, and disabled treatment.

Since these are visible, theme-sensitive layout changes, please pair the fix with light/dark evidence from the real composer (the PR's "UI Changes" section is still the template).

Posted via Macroscope — UI Consistency

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3495ffa. Configure here.

resolve,
upsertCommentForCurrentHead,
validateResult,
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Orphaned transfer report scripts

Low Severity

thread-transfer-report.cjs and its test are added, but no workflow under .github/workflows invokes them after CI was reduced to fork release only. The reporting helpers are dead code that will never publish PR comments or enforce ceilings in this tree.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3495ffa. Configure here.

expected,
),
toThrow: (expected) => {
const { threw, error } = resolveThrown(actual);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium src/Expect.ts:619

expect(123).toThrow() and expect(new Error()).toThrow() pass even though no function was invoked, masking tests that failed to wrap the operation. resolveThrown treats every non-function as an already-thrown error; toThrow should require a function and fail for non-function values.

🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/packages/alchemy-test/src/Expect.ts around line 619:

`expect(123).toThrow()` and `expect(new Error()).toThrow()` pass even though no function was invoked, masking tests that failed to wrap the operation. `resolveThrown` treats every non-function as an already-thrown error; `toThrow` should require a function and fail for non-function values.

Evidence trail:
Reviewed commit 3495ffae. `.repos/alchemy-effect/packages/alchemy-test/src/Expect.ts:419-429` treats all non-functions as thrown errors; `:618-629` passes when `threw` is true and `expected` is undefined. `https://v3.vitest.dev/api/expect.html` — `toThrowError` documentation states that the code must be wrapped in a function.

Comment on lines +65 to +70
const client = yield* HttpClient.HttpClient;
const res = yield* client.get(`https://${endpoint}/`, {
headers: AWS.Lambda.microvmAuthHeaders(authToken),
});
return yield* res.text;
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium src/microvm-worker.ts:65

rawReachable records a MicroVM as ready for any HTTP response, including unauthorized, 404, or 500 responses, so the raw baseline can report a false readyMs. HttpClient does not fail on non-2xx responses; check res.status and the expected ok body before allowing the readiness retry to succeed.

         const client = yield* HttpClient.HttpClient;
-        const res = yield* client.get(`https://${endpoint}/`, {
+        const res = yield* client.get(`https://${endpoint}/`, {
           headers: AWS.Lambda.microvmAuthHeaders(authToken),
         });
-        return yield* res.text;
+        const body = yield* res.text;
+        if (res.status !== 200 || body !== "ok") {
+          return yield* Effect.fail(
+            new Error(`readiness probe failed: ${res.status}: ${body.slice(0, 120)}`),
+          );
+        }
+        return body;
Also found in 1 other location(s)

.repos/alchemy-effect/benchmark/container/src/orchestrator.ts:73

rawReachable treats every HTTP response as a successful readiness probe: after client.get(...), it only reads res.text and never checks res.status or the expected body. HttpClient preserves non-2xx responses rather than failing them, so an unauthorized/404/500 response from the MicroVM proxy or server ends the retry and records a false readyMs, making the raw baseline results invalid.

🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/benchmark/container/src/microvm-worker.ts around lines 65-70:

`rawReachable` records a MicroVM as ready for any HTTP response, including unauthorized, 404, or 500 responses, so the raw baseline can report a false `readyMs`. `HttpClient` does not fail on non-2xx responses; check `res.status` and the expected `ok` body before allowing the readiness retry to succeed.

Evidence trail:
Reviewed commit 3495ffae. `.repos/alchemy-effect/benchmark/container/src/microvm-worker.ts:57-70, 102-119, 199-210`; `.repos/effect-smol/packages/effect/src/unstable/http/HttpClient.ts:56-61, 83-90, 564-570`. Git commands: `git show 3495ffae -- .repos/alchemy-effect/benchmark/container/src/microvm-worker.ts`; `git show 3495ffae:.repos/effect-smol/packages/effect/src/unstable/http/HttpClient.ts`.

Also found in 1 other location(s):
- .repos/alchemy-effect/benchmark/container/src/orchestrator.ts:73 -- `rawReachable` treats every HTTP response as a successful readiness probe: after `client.get(...)`, it only reads `res.text` and never checks `res.status` or the expected body. `HttpClient` preserves non-2xx responses rather than failing them, so an unauthorized/404/500 response from the MicroVM proxy or server ends the retry and records a false `readyMs`, making the raw baseline results invalid.

plt.close(fig)


all_max = max(max(by_key[k]) for _, k, _ in series)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium scripts/plot-blog.py:94

When a valid CSV has no successful samples for a key in series, all_max raises ValueError: max() arg is an empty sequence, so the script produces no plots despite other series having data. Filter unavailable base series before calculating the maximum and handle the case where none remain.

+series = [s for s in series if by_key[s[1]]]
-all_max = max(max(by_key[k]) for _, k, _ in series)
+all_max = max((max(by_key[k]) for _, k, _ in series), default=0)
🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/benchmark/container/scripts/plot-blog.py around line 94:

When a valid CSV has no successful samples for a key in `series`, `all_max` raises `ValueError: max() arg is an empty sequence`, so the script produces no plots despite other series having data. Filter unavailable base series before calculating the maximum and handle the case where none remain.

Evidence trail:
.repos/alchemy-effect/benchmark/container/scripts/plot-blog.py:18-25, 61-67, 94-95 (commit 3495ffa)

// validates it and fails the cluster with "missing one or more required
// dependencies" otherwise. Rendered locally and applied to the EKS
// cluster before the HyperPod cluster attaches.
const chart = yield* FetchHyperPodChart({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium src/eks-infra.ts:137

A clean first deployment fails at FetchHyperPodChart before the chart is rendered because git clone targets .alchemy/cache/hyperpod-cli without a pre-existing .alchemy/cache parent. Create the cache directory before cloning (or otherwise ensure the parent exists).

🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/examples/aws-hyperpod/src/eks-infra.ts around line 137:

A clean first deployment fails at `FetchHyperPodChart` before the chart is rendered because `git clone` targets `.alchemy/cache/hyperpod-cli` without a pre-existing `.alchemy/cache` parent. Create the cache directory before cloning (or otherwise ensure the parent exists).

Evidence trail:
.repos/alchemy-effect/examples/aws-hyperpod/src/hyperpod-chart.ts:52-67 (commit 3495ffa)
.repos/alchemy-effect/examples/aws-hyperpod/src/eks-infra.ts:137-143 (commit 3495ffa)
.repos/alchemy-effect/packages/alchemy/src/AlchemyContext.ts:33-45 (commit 3495ffa)
Git clone documentation: https://git-scm.com/docs/git-clone

) => Effect.Effect<unknown, any, any>;
};

export default Cloudflare.Worker(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Critical src/microvm-worker.ts:53

The public Cloudflare.Worker lets any caller invoke /boot without authentication or rate limiting, and each request runs AWS.Lambda.RunMicrovm for a fresh VM. An internet caller can therefore exhaust the account's MicroVM quota and incur AWS charges; require access control and/or rate limiting before launching VMs.

Also found in 1 other location(s)

.repos/alchemy-effect/benchmark/container/src/orchestrator.ts:63

url: true creates a public Lambda Function URL, but this handler has no authentication or rate limit before boot launches a fresh MicroVM. Anyone who discovers the URL can repeatedly call /boot (and choose among all six images), consuming the account's MicroVM quota and incurring AWS compute/build-related usage; this benchmark endpoint needs an access control mechanism rather than exposing the launch operation anonymously.

🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/benchmark/container/src/microvm-worker.ts around line 53:

The public `Cloudflare.Worker` lets any caller invoke `/boot` without authentication or rate limiting, and each request runs `AWS.Lambda.RunMicrovm` for a fresh VM. An internet caller can therefore exhaust the account's MicroVM quota and incur AWS charges; require access control and/or rate limiting before launching VMs.

Evidence trail:
.repos/alchemy-effect/benchmark/container/src/microvm-worker.ts:53-56, 167-186, 212-215, 220-227, 237-245 (commit 3495ffa); .repos/alchemy-effect/benchmark/container/alchemy.run.ts:18-22, 52-58 (commit 3495ffa); .repos/alchemy-effect/packages/alchemy/src/Cloudflare/Workers/Worker.ts:353-356 (commit 3495ffa); .repos/alchemy-effect/packages/alchemy/src/AWS/Lambda/MicrovmImage.ts:474-478, 519-523 (commit 3495ffa); .repos/alchemy-effect/benchmark/container/test/bench.test.ts:31-34, 53-59 (commit 3495ffa).

Also found in 1 other location(s):
- .repos/alchemy-effect/benchmark/container/src/orchestrator.ts:63 -- `url: true` creates a public Lambda Function URL, but this handler has no authentication or rate limit before `boot` launches a fresh MicroVM. Anyone who discovers the URL can repeatedly call `/boot` (and choose among all six images), consuming the account's MicroVM quota and incurring AWS compute/build-related usage; this benchmark endpoint needs an access control mechanism rather than exposing the launch operation anonymously.

Comment on lines +177 to +184
fsModule.write = ((fd: unknown, data: unknown, ...rest: Array<unknown>) => {
if (fd === 1 || fd === 2) {
sink(`stray fd${fd}`, data);
const callback = rest.findLast((arg) => typeof arg === "function") as
| ((err: Error | null, written: number, data: unknown) => void)
| undefined;
callback?.(null, 0, data);
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 High src/StrayOutput.ts:177

Intercepted asynchronous fs.write calls report written = 0 and invoke the callback synchronously, so callers that advance by the reported byte count can retry indefinitely or treat the write as incomplete. Report the accepted chunk's byte count and invoke the callback asynchronously.

    fsModule.write = ((fd: unknown, data: unknown, ...rest: Array<unknown>) => {
       if (fd === 1 || fd === 2) {
         sink(`stray fd${fd}`, data);
+        const written =
+          typeof data === "string"
+            ? Buffer.byteLength(data)
+            : ((data as Uint8Array).byteLength ?? 0);
         const callback = rest.findLast((arg) => typeof arg === "function") as
           | ((err: Error | null, written: number, data: unknown) => void)
           | undefined;
-        callback?.(null, 0, data);
+        queueMicrotask(() => callback?.(null, written, data));
         return;
       }
🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/packages/alchemy-test/src/StrayOutput.ts around lines 177-184:

Intercepted asynchronous `fs.write` calls report `written = 0` and invoke the callback synchronously, so callers that advance by the reported byte count can retry indefinitely or treat the write as incomplete. Report the accepted chunk's byte count and invoke the callback asynchronously.

Evidence trail:
.repos/alchemy-effect/packages/alchemy-test/src/StrayOutput.ts:153-191, commit 0b0d174; .repos/alchemy-effect/packages/alchemy-test/src/cli.ts:188-205, commit 0b0d174; Node.js fs.write documentation: https://nodejs.org/api/fs.html#fswritefd-buffer-offset-length-position-callback

if (options !== undefined) {
const stdio = options.stdio;
if (stdio === "inherit") {
options.stdio = ["inherit", "pipe", "pipe"];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 High src/StrayOutput.ts:217

Reusing an options object causes later childProcess.spawn and Bun.spawn calls to lose their capture pumps: the first call changes caller-owned stdio, stdout, and stderr values to "pipe", so subsequent children inherit undrained pipes and can block once the pipe fills. Clone the options and stdio configuration before rewriting these fields.

🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/packages/alchemy-test/src/StrayOutput.ts around line 217:

Reusing an `options` object causes later `childProcess.spawn` and `Bun.spawn` calls to lose their capture pumps: the first call changes caller-owned `stdio`, `stdout`, and `stderr` values to `"pipe"`, so subsequent children inherit undrained pipes and can block once the pipe fills. Clone the options and stdio configuration before rewriting these fields.

Evidence trail:
.repos/alchemy-effect/packages/alchemy-test/src/StrayOutput.ts:214-238 and 252-284 @ 3495ffa
https://nodejs.org/api/child_process.html#optionsstdio
https://nodejs.org/api/child_process.html#subprocessstdout
https://bun.sh/reference/bun/spawn
https://bun.sh/docs/guides/process/spawn-stdout

fig, ax = plt.subplots(figsize=(9.5, 4.8))
for label, key, color in series:
xs = sorted(by_key[key])
ys = [(i + 1) / len(xs) * 100 for i in range(len(xs))]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium scripts/plot-blog.py:141

The plots overstate completeness by showing only successful boots: failed rows are filtered out at line 20, so a 24/25-success run renders 24 strip points and a CDF that reaches 100% instead of 96%. Preserve the total boot count per key before filtering, use it as the CDF denominator, and indicate failed boots in the strip plot.

🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/benchmark/container/scripts/plot-blog.py around line 141:

The plots overstate completeness by showing only successful boots: failed rows are filtered out at line 20, so a 24/25-success run renders 24 strip points and a CDF that reaches 100% instead of 96%. Preserve the total boot count per key before filtering, use it as the CDF denominator, and indicate failed boots in the strip plot.

Evidence trail:
Reviewed commit 3495ffae7efc: .repos/alchemy-effect/benchmark/container/scripts/plot-blog.py:17-25, 72-81, 137-145; .repos/alchemy-effect/benchmark/container/test/bench.test.ts:50-61, 94-105, 132-183. Verify with `git show 3495ffae7efc:.repos/alchemy-effect/benchmark/container/scripts/plot-blog.py` and `git show 3495ffae7efc:.repos/alchemy-effect/benchmark/container/test/bench.test.ts`.

// 2. global console — bun's console writes natively, NOT via stdout.write
// -------------------------------------------------------------------------
try {
const methods = [

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium src/StrayOutput.ts:115

console.assert, console.count, console.timeEnd, console.timeLog, and console.clear still write directly to the terminal during capture, so those calls can interleave reporter output or corrupt the TUI alternate screen. Add these output-producing methods to the patch list (or proxy the complete Console API).

🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/packages/alchemy-test/src/StrayOutput.ts around line 115:

`console.assert`, `console.count`, `console.timeEnd`, `console.timeLog`, and `console.clear` still write directly to the terminal during capture, so those calls can interleave reporter output or corrupt the TUI alternate screen. Add these output-producing methods to the patch list (or proxy the complete `Console` API).

Evidence trail:
Commit 3495ffae: .repos/alchemy-effect/packages/alchemy-test/src/StrayOutput.ts:112-151; .repos/alchemy-effect/packages/alchemy-test/src/cli.ts:188-205; .repos/alchemy-effect/packages/alchemy-test/src/Tui.ts:443-459. Bun documentation: https://bun.com/reference/globals/Console, https://bun.com/reference/globals/Console/clear, https://bun.com/docs/runtime/console

Comment on lines +120 to +122
density = gaussian_kde(xs, bw_method=0.25)(np.log10(grid))
ax.plot(grid, density, color=color, linewidth=2.2, label=label)
ax.fill_between(grid, density, color=color, alpha=0.15)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium scripts/plot-blog.py:120

The script raises instead of producing the remaining plots when an oc_series key has fewer than two successful samples or identical timings. gaussian_kde cannot estimate a density for those inputs because the sample is insufficient or has singular covariance; skip the KDE curve for degenerate sample sets.

-    density = gaussian_kde(xs, bw_method=0.25)(np.log10(grid))
-    ax.plot(grid, density, color=color, linewidth=2.2, label=label)
-    ax.fill_between(grid, density, color=color, alpha=0.15)
+    if len(xs) >= 2 and np.ptp(xs) > 0:
+        density = gaussian_kde(xs, bw_method=0.25)(np.log10(grid))
+        ax.plot(grid, density, color=color, linewidth=2.2, label=label)
+        ax.fill_between(grid, density, color=color, alpha=0.15)
🤖 Copy this AI Prompt to have your agent fix this:
In file @.repos/alchemy-effect/benchmark/container/scripts/plot-blog.py around lines 120-122:

The script raises instead of producing the remaining plots when an `oc_series` key has fewer than two successful samples or identical timings. `gaussian_kde` cannot estimate a density for those inputs because the sample is insufficient or has singular covariance; skip the KDE curve for degenerate sample sets.

Evidence trail:
Commit 0b0d174. `.repos/alchemy-effect/benchmark/container/scripts/plot-blog.py:17-25` filters and groups only successful samples; `:101-108` defines `oc_series`; `:116-135` unconditionally constructs/evaluates `gaussian_kde` before the CDF at `:137-156`. SciPy documentation: https://docs.scipy.org/doc/scipy/reference/generated/scipy.stats.gaussian_kde.html (does not support data lying in a lower-dimensional subspace). Verification command: `git show 0b0d174:.repos/alchemy-effect/benchmark/container/scripts/plot-blog.py`.

@macroscopeapp

macroscopeapp Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Skipped

Macroscope did not run approvability analysis for this PR. This PR modifies Macroscope's approvability configuration (.macroscope/approvability.md). Changes to the rules that govern approval are never approved automatically. This cannot be overridden.

Not approved because:

  • 15 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant