Skip to content

improvement(logs): cut recurring production log noise and fix the bugs behind it - #8871

Merged
waleedlatif1 merged 26 commits into
stagingfrom
improvement/log-noise-cleanup
Oct 10, 2026
Merged

waleedlatif1 merged 26 commits into
stagingfrom
improvement/log-noise-cleanup

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Cuts recurring production log noise, fixing the root cause wherever the noise came from a real bug. Each item lists what the logs showed and what to check in CloudWatch after deploy.

Root-cause fixes

  • Serializer: trigger-mode blocks — every trigger-mode tool block (e.g. a Slack trigger) logged "Tool selection failed during serialization" on each serialization. The reason: tool-mode sub-blocks such as operation aren't serialized in trigger mode, but the tool was still selected. Trigger-mode blocks now skip tool selection. Behavior change: their config.tool is now '' instead of an arbitrary fallback tool. TriggerBlockHandler runs these blocks and never reads it. Watch: the Serializer "Tool selection failed" line disappears.
  • Executor: response-format re-parse — collectBlockData re-derived every block's output schema on each condition/function/agent/reference evaluation. An invalid agent response format was therefore re-parsed and logged with a full stack every time. Schemas are now memoized per (immutable) serialized block. The parse failure logs at debug without the stack (the editor lint already reports an invalid response format). Watch: ResponseFormatUtils warnings disappear.
  • Webhook credentials — "Failed to resolve credential account" fired on every delivery through a service-account or managed-OAuth credential (e.g. a custom Slack bot). These credentials have no account row, so the lookup always missed. They're now skipped; the warning stays for a credential or account that's genuinely missing. Watch: the line only appears for real misses.
  • Flint generate_pages — items was typed json, so the LLM schema advertised it as an object and dropped the item schema. A warning fired on every schema build. It's now an array of 1–10 { targetPageSlug, context }; block-provided JSON strings still parse. Regenerated tool metadata and docs. Watch: the ToolsParams "items property ignored" line disappears.
  • Tokenizer — encodingForModel only knows exact OpenAI names. Unknown models warned, and each built its own copy of a BPE rank table. Encodings now resolve by model family and are cached once per encoding; the model → encoding lookup is memoized. Behavior change: newer and provider-prefixed OpenAI ids (gpt-5.x, gpt-6.x, azure/gpt-4o) now count with o200k_base instead of cl100k_base. Watch: TokenizationAccurate warnings disappear.
  • Admin mothership proxy — the route had no logger. It passed upstream 5xx through as its own 500, failed on non-JSON or empty bodies, and returned a silent 500 when the admin key was missing. It now logs method, environment, endpoint, status and a truncated upstream body. Behavior change: an upstream 5xx or a non-JSON body now returns 502 carrying the upstream error/message; an empty 2xx (e.g. 204) passes through. Watch: the remaining failures carry a cause.
  • Catalog tool reads — listing, reading and executing catalog tools loaded and sanitized every custom block's deployed workflow on each request, just to derive input fields that tool scope never reads. Tool reads and block listing now use the lightweight custom-block rows; only block detail reads, which render inputs, derive them.

Log-level and dedupe changes (no behavior change)

  • Subblock type drift — immutable deployed snapshots re-warned "Repairing malformed subBlock metadata" on every load when a field's declared type had changed (e.g. dropdown → combobox). Drift is now repaired at debug; missing or unknown types still warn.
  • Idempotency — "No unique identifier found" now logs at debug only for providers that declare deliveryIdOptional. Only the generic webhook does, since its deliveries carry no id unless an idempotency field is configured. Header-based providers (GitHub, GitLab, Shopify, Linear, Svix…) still warn when their delivery header is missing.
  • Slack reactions.get missing_scope — a bot without reactions:read failed this call on every reaction event. It's now logged once per webhook per hour at info, and the call still runs, so a reinstalled bot gets text back on the next event. Other errors still warn.
  • JSON-typed block inputs — the generic handler no longer tries JSON.parse on plain strings (file ids, URLs, references) that can't be JSON. Every valid JSON text still parses.
  • Sandbox provenance — catalog routes (blocks, tools, connector types) no longer warn "no recorded secret provenance". The route set comes from the catalog contracts and is tested against the generated v2 route table. Data-bearing routes without a producer warn once per route per hour. Recording and admission are unchanged.
  • Proxy scanners — "Blocked suspicious request" (empty/tool user agents probing paths like /admin.php) is now debug. The 403 is unchanged. WAF logs (all requests) and ALB access logs still record this traffic.
  • Telemetry forwarding — collector timeouts and errors are now warn instead of error. Forwarding is best effort and the route still returns success.
  • Per-call success chatter → debug — PII batch masking, function execution request/success, sandbox mount resolution, table row queries and table limits, usage-limit statistics, and DAG builds.

Deliberately not changed

  • RouteHandler OK stays at info for every successful request. A CloudWatch metric filter on module = "RouteHandler" feeds the API p90 latency alarm, and dashboards break down per-route latency and errors from these lines. Debug lines never reach OTel.
  • "Tool call routing decision" stays at info. It's the diagnostic for stale-catalog and in-band double-dispatch races.
  • I grepped the infra repo for every demoted module and message; there are no other metric-filter or alarm consumers. The generic "Recent App Errors" dashboard widgets match on error/failed text, so the demoted lines simply stop cluttering them.

Type of Change

  • Bug fix
  • Improvement

Testing

  • New: serializer/index.edge-case-blocks.test.ts (trigger-mode tool id) and lib/tokenization/accurate.test.ts. Both were shown red on the pre-fix code.
  • New: lib/mothership/tools/sandbox-catalog-routes.test.ts. Shown red when the catalog patterns drift from the route table.
  • Full gate locally before review rounds (lint, type-check, check:audits, docs-manifest:check, docs:check, tool-metadata:check, affected Vitest); CI green on the final HEAD.

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)

Trigger-mode blocks run through TriggerBlockHandler and never read a tool id,
while their tool-mode sub-blocks (e.g. operation) are not serialized, so
selecting a tool always threw and logged a fallback warning.
collectBlockData re-derived every block's output schema on each condition,
function, agent, and reference evaluation, re-parsing an invalid agent
response format and logging its full stack every time. Memoize the schema per
immutable serialized block, attribute parse failures to the real block id, and
log them at debug without the stack.
Service-account and managed-OAuth credentials (e.g. a custom Slack bot) have no
OAuth account row, so the account-owner lookup always missed and warned on
every webhook run. Skip the lookup for them and warn only when the credential
or its account is genuinely missing.
… webhook

A Slack bot without reactions:read fails reactions.get identically for every
reaction event until it is reinstalled. Record that configuration state once
per webhook per hour at info and keep the warning for other failures.
Deployed snapshots are immutable and re-sanitized on every load, so a field
whose declared type later changed (e.g. dropdown to combobox) warned on every
materialization forever. Log repairs at debug when the stored type is one some
registered block declares; keep warning for missing or unknown types.
Listing, reading, and executing a catalog tool resolved the catalog gate with
every custom block's input fields, which loads and sanitizes each custom
block's deployed workflow per request. Tool scope only needs the block types
(every custom block exposes workflow_executor), so tool reads use the
lightweight overlay rows; block reads still derive the inputs.
…s without one

A generic webhook without an idempotency header or configured field has no
delivery identifier by design, so warning on every delivery was noise. Warn
only when the provider normally extracts an id from the body.
items was typed json, so the LLM schema advertised it as an object and
dropped its item schema with a warning on every schema build. Declare it as an
array of 1-10 page objects; block-provided JSON strings still parse through
parseItems. Regenerate tool metadata and docs.
…n strings

json-typed block inputs also accept plain strings (a file id, URL, or
reference) that are kept as-is, so parsing them could only fail and warn.
Parse only text that can be JSON; every valid JSON text still parses.
…rovenance warnings

Catalog responses (blocks, tools, connector types) carry no workspace data, so
no producer records provenance for them and the per-request warning was
noise. Skip them statically, and report a data-bearing route without a
producer once per route per hour instead of on every request. Admission and
recording behavior is unchanged.
encodingForModel only knows exact OpenAI names, so newer or provider-prefixed
OpenAI ids fell back to cl100k_base (miscounting o200k models) and every
unknown model built and cached its own copy of the rank table. Resolve exact
names through js-tiktoken, newer OpenAI ids by family, everything else to
cl100k_base, and cache one instance per encoding.
Every blocked request is an empty or tool user agent probing paths like
/admin.php; the 403 is the whole response, so the per-request warning was
pure ingest cost.
Forwarding to the external collector is best effort and the route still
returns success, so a collector timeout or error is not an application error.
…to 502

The proxy logged nothing, passed an upstream 5xx straight through as its own
500, threw on a non-JSON upstream body, and returned a silent 500 when the
admin key was missing. Log the method, environment, endpoint, status, and a
truncated upstream body; answer upstream 5xx and unparseable bodies with 502;
and log the missing-key misconfiguration.
PII batch masking, function execution request/success, sandbox mount
resolution, table row queries and limits, usage-limit statistics, DAG builds,
and mothership tool routing logged an info line on every call. They are
diagnostic detail, not events, so log them at debug.
It is the diagnostic for stale-catalog 'No handler for tool' dispatches and
in-band double-dispatch races, so it stays visible in production.
…hip proxy

A gateway 502 now carries the upstream's error or message field, which the
admin UI shows, and an empty 2xx body (e.g. 204) passes through as an empty
response instead of failing JSON serialization into a misleading 502.
A non-OpenAI model threw and caught inside getEncodingNameForModel on every
count. Cache the resolved encoding name per model id in a bounded LRU.
…oute table

Derive each catalog pattern by matching its contract path against the
generated v2 route table, so a renamed path parameter cannot drift from the
pattern matchV2Route reports and silently restore the warning.
…clares them optional

Header-based providers (GitHub, GitLab, Shopify, Linear, Svix) have no body
extractor, so keying the warning on the extractor silenced exactly the
anomalous case of their delivery header going missing. Providers now declare
deliveryIdOptional; only the generic webhook does, since its deliveries carry
no identifier unless an idempotency field is configured.
- Skip reactions.get entirely while a webhook's bot is known to lack the
  scope, instead of repeating a call known to fail.
- Treat any intact sub-block whose stored type differs from the registry as
  drift, dropping the registry-wide type scan.
- Move the JSON-candidate check next to isJSONString and trim once.
- Make the catalog gate's cheap overlay rows the default; only block reads
  derive custom block inputs.
- Collapse the admin mothership proxy's three copied handlers into one.
…the route table

Extract the catalog route set into isCatalogRoute, built lazily from the catalog
contracts resolved through the generated v2 route table, and test it against
real request paths so a route or parameter rename cannot silently restore the
missing-provenance warning.
@vercel

vercel Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Oct 10, 2026 1:49am UTC

Request Review

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 38 files

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

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/lib/webhooks/providers/slack.ts Outdated
Comment thread apps/sim/lib/catalog/application/list-blocks.ts Outdated
@greptile-apps

greptile-apps Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge, with the earlier Slack reconnect problem fixed.

Summary

Cuts repeated logs and avoids unnecessary schema, tokenizer, and catalog work.

  • Trigger-mode blocks no longer select a tool during serialization.
  • Workflow evaluations reuse each block’s output schema.
  • JSON-typed block inputs keep ordinary strings intact.
  • Webhook credential checks distinguish accountless credentials from missing accounts.
  • Webhook id warnings now depend on whether a provider expects a delivery ID.
  • Slack reaction permission failures are logged once per webhook per hour.
  • Token counts resolve newer OpenAI model IDs to their model-family encoding.
  • The admin mothership proxy reports upstream causes and handles empty or invalid responses.
  • Catalog tool reads avoid loading deployed workflows for custom-block inputs.
  • Sandbox catalog reads no longer trigger missing-provenance warnings.
  • Routine messages move to quieter log levels without changing behavior.
  • Flint page generation takes a bounded array of pages with required fields.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Catalog request] --> B{Needs input details?}
  B -->|Yes| C[Load deployed input fields]
  B -->|No| D[Read custom block rows]
  C --> E[Apply visibility checks]
  D --> E
  E --> F[Return catalog]
Loading

Reviews (3) · Last reviewed commit: "chore(logs): tighten log-noise changes a..." · Reviewed by Greptile

Comment thread apps/sim/lib/webhooks/providers/slack.ts Outdated
Skipping the call for the cooldown window left a reinstalled Slack bot with
empty reaction text for up to an hour. Keep the once-per-window log but always
make the call. Block listing also uses the lightweight custom-block rows, since
block summaries never read custom block inputs.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@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 cubic-dev-ai 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.

No issues found across 37 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

- Drop the blockId option threaded through getEffectiveBlockOutputs; it only
  labeled a debug log.
- Name the log-dedupe windows and cache ceilings.
- Build the sandbox catalog route set by converting contract paths to the
  route table's pattern syntax; the route-table test guards drift.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@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 cubic-dev-ai 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.

No issues found across 36 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

@waleedlatif1
waleedlatif1 merged commit ba485c6 into staging Oct 10, 2026
49 checks passed
@waleedlatif1
waleedlatif1 deleted the improvement/log-noise-cleanup branch October 10, 2026 02:38

This branch was previously deployed

1 inactive deployment
Preview — 39adc465 Deployed Oct 10, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant