fix(logs): close the log-once coverage gaps from the cross-PR review by waleedlatif1 · Pull Request #8881 · simstudioai/sim

waleedlatif1 · 2026-10-10T04:04:55Z

Summary

Closes the gaps a cross-PR review found after #8872, where author-caused failures were still logged at ERROR or logged more than once.

Execution boundaries: execute-workflow (streaming/chat API, copilot, workflow tests), the scheduled-workflow catch, the table group-cell catch, and both resume paths now go through logFailureOnce with their execution id, like the other surfaces.
- A revoked credential or another author-caused failure logs at INFO.
- A failure an inner boundary already logged for that execution is not repeated.
- Unattributed faults still log at ERROR.
Non-revoked credential failures went from four ERROR lines to two, joined by request id:
- The token resolver keeps the only line that has the cause. It now carries credentialId, providerId, and a redacted cause. It stays at ERROR because it is the only server line on the browser token route.
- The tool boundary line carries toolId, workflowId, executionId, blockId, and credentialId.
- Removed the two re-logs in between (credential-token and the token catch in tools/index.ts).
Child workflow failure line now carries the projected root error and the child execution id. Before, it had neither, so it was the parent's only line and gave nothing to act on.
Legacy workflow job: a usage-limit or suspended-account refusal throws a UserFailure and is no longer logged twice at ERROR.
v2 /workflows/{id}/execute and MCP serve no longer return the internal admission codes USAGE_LIMIT_EXCEEDED and ACCOUNT_SUSPENDED in details.code / error data.
- This restores the response shape from before fix(webhooks): stop admission-refusal retry loops and per-retry log writes #8870.
- ACCOUNT_SUSPENDED is not in FORBIDDEN_DETAIL_CODES, the closed set the v2 contract allows on a 403.
- A 402 already says USAGE_LIMIT_EXCEEDED.
Agent provider failures: the block executor's failure line names the provider and model again, projected alongside the error.

Kept at ERROR: unattributed faults at every boundary, and the token resolver's refresh failure (it holds the cause).

What to watch in CloudWatch after deploy

"Workflow execution failed", "Error executing scheduled workflow", "Workflow group cell execution failed", and "Resume execution failed" should appear at INFO/WARN with a failureKind field for author-caused failures, and mostly disappear when an inner boundary already logged the failure.
"Credential token resolution failed" and "Error fetching access token for …" should be gone. "Failed to refresh access token" should carry credentialId and providerId.
"Preprocessing failed" lines from legacy jobs should be gone.
v2 402/403 refusals should no longer carry details.code for these admission codes.

Type of Change

Bug fix

Testing

New tests, each shown failing on the pre-fix code:
- execute-workflow: revoked credential and internal fault are both logged once for the execution (probed through logFailureOnce).
- Legacy job: usage limit and suspended account are attributed to the author; an unclassified refusal stays internal as a control.
- v2 execute: 402/403 refusals carry no internal details.code.
- Block executor: the failure line carries provider and model, and fails closed on an incomplete registry.
Touched test files run locally one at a time. bunx biome check on changed files. Full lint, type-check, check:audits, and suites run in CI.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing (new tests pass the test-audit authoring gate)
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Routes the remaining execution boundaries (execute-workflow, schedule, table group cell, resume) through logFailureOnce with their execution id, so a revoked credential logs at INFO and a failure an inner boundary logged is not repeated. Collapses a non-revoked credential failure to one ERROR line at the tool boundary with the credential id, keeping the cause at WARN in the token resolver under the same request id. The child failure line carries the projected root error and the child execution id. A legacy job's usage-limit or suspended-account refusal is the author's and is no longer logged twice. The v2 execute and MCP serve responses no longer carry the internal admission codes, restoring their prior shape, and the Agent failure line names its provider and model again.

Keeps the token resolver's refresh failure at ERROR (it is the only server line on the browser token route) with the credential, provider, and redacted cause; throws a UserFailure for a legacy job's admission refusal; narrows the Agent failure details to provider and model; and passes the child execution id through unchanged.

… case

vercel · 2026-10-10T04:05:00Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
docs	Skipped		Oct 10, 2026 4:45am UTC

cubic-dev-ai

All reported issues were addressed across 19 files

_{Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic}

greptile-apps · 2026-10-10T04:10:29Z

Confidence Score: 3/5

[Medium impact] Adds deduplication and diagnostic details to error logging.

Do not merge until the tool failure log stops exposing resolved environment secrets.

Findings

Secrets reach server logs ▶
Server failures look user-caused ▶

Summary

This PR adds logFailureOnce coverage, removes repeated credential error logs, and restores useful failure details.

Workflow and resume boundaries log each run’s failure once.
Credential failure logs identify the account and keep the cause.
Child workflow warnings show the child run and its root error.
Legacy workflow jobs attribute known admission refusals to the author.
V2 workflow refusals leave internal admission codes out of public details.
Agent block failure logs include the provider and model.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Direct tool call] --> B[Map credential selector]
  B --> C[Resolve environment reference]
  C --> D[Capture selectedCredentialId]
  D --> E[Credential lookup fails]
  E --> F[Tool failure log]
  F --> G[Raw credentialId field]
  F --> H[Projected error details]

_{Reviews (2) · Last reviewed commit: "fix(logs): keep trusted ids beside proje..." · Reviewed by Greptile}

…esumes on the parent run The resume and child failure lines keep their execution, block, and resume ids outside the secret projection, which drops everything when it fails closed. Resumed runs execute under the parent's execution id, so the resume boundaries dedupe on it; a 4xx resume admission refusal is the caller's. The tool failure line names the credential selected after normalization.

waleedlatif1 · 2026-10-10T04:45:46Z

@greptile

waleedlatif1 · 2026-10-10T04:45:49Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-10T04:45:55Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

No issues found across 21 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Turn on auto-fix | Re-trigger cubic}

greptile-apps · 2026-10-10T04:51:51Z

        workflowId: executionContext?.workflowId ?? undefined,
        executionId: executionContext?.executionId,
        blockId: typeof toolContext.blockId === 'string' ? toolContext.blockId : undefined,
+        credentialId: selectedCredentialId,


Secrets reach server logs

selectedCredentialId can contain a decrypted secret, not just an account ID. A direct snowflake_execute_sql call can pass credentialId: '{{TOKEN}}'. The tool's oauthCredential field is user-only, so resolveToolEnvReferences replaces the reference before this value is captured.

If credential lookup rejects that value, this catch writes the secret as credentialId, outside projectToolLogMetadata. Anyone with access to the server logs could read it. Project this field too, or log only an ID confirmed by credential lookup.

How this was verified: The public call accepts the reference, the tool resolves it from the caller's environment, and the failure log writes the resolved value without secret projection.

greptile-apps · 2026-10-10T04:51:51Z

+      if (error instanceof ResumeAdmissionError && error.statusCode < 500) {
+        markFailureKind(error, 'user')


Server failures look user-caused

Not every ResumeAdmissionError below 500 is the caller's fault. requireResumeDeploymentVersion throws it with status 409 when a saved run is missing its execution mode or deployment version, or when those values disagree. runResumeExecution checks these values from the saved snapshot and claimed log row.

The new blanket mark turns those server-owned state failures into INFO lines attributed to the user. Operators can miss broken saved state because it looks like an ordinary user mistake. Keep these failures internal and mark only refusals the caller can cause.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

waleedlatif1 added 4 commits October 9, 2026 21:04

test(logs): assert only the boundary behavior in the execute-workflow…

664c119

… case

chore(audits): shrink the explicit-any baseline for tools/index.ts

3d4a54d

cubic-dev-ai Bot reviewed Oct 10, 2026

View reviewed changes

greptile-apps Bot reviewed Oct 10, 2026

View reviewed changes

Comment thread apps/sim/tools/index.ts Outdated

Comment thread apps/sim/executor/handlers/workflow/workflow-handler.ts Outdated

vercel Bot deployed to Preview October 10, 2026 04:10 View deployment

vercel Bot temporarily deployed to Preview October 10, 2026 04:45 Inactive

cubic-dev-ai Bot reviewed Oct 10, 2026

View reviewed changes

greptile-apps Bot reviewed Oct 10, 2026

View reviewed changes

		if (error instanceof ResumeAdmissionError && error.statusCode < 500) {
		markFailureKind(error, 'user')

Conversation

waleedlatif1 commented Oct 10, 2026

Summary

What to watch in CloudWatch after deploy

Type of Change

Testing

Checklist

Uh oh!

vercel Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 3/5

Findings

Summary

Diagram

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Oct 10, 2026

Uh oh!

waleedlatif1 commented Oct 10, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 10, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Oct 10, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

greptile-apps Bot commented Oct 10, 2026 •

edited

Loading