Repository navigation
do-not-merge/testing-nuts-failure-logs - #476
Open
nico-pappagianis wants to merge 16 commits into
Open
nico-pappagianis wants to merge 16 commits into
nico-pappagianis wants to merge 16 commits into
Conversation
nico-pappagianis
force-pushed
the
do-not-merge/testing-nuts-failure-logs
branch
from
October 19, 2026 17:23
3ff1523 to
cdba4c3
Compare
Contributor
|
This PR lacks any commits of the 'fix' or 'feat' type, and therefore will not trigger a release. To silence all further warnings, react to this warning comment (or any other) with the 👀 emoji. NOTE: If your repo uses squash commits, make sure to add the appropriate conventional commit prefix (e.g., 'fix:', 'feat:', 'chore:', etc) in the squash commit message. |
…urn [do-not-merge] The run-eval NUTs issued the identical JSON-payload command 4x and the identical YAML-spec command 2x, each a fresh CLI process doing a fresh OAuth token refresh against the single shared scratch-org admin user. That burst trips Salesforce's per-user login-rate limit (login rate exceeded), the dominant NUT failure. Run each distinct command once in a before() hook and share the parsed result across the tests that assert against it. All assertions preserved; genuinely-distinct commands untouched.
…merge] Give the agent test run-eval NUT its own scratch-org user (System Administrator profile + EinsteinGPTPromptTemplateManager, mirroring the shared admin) so its OAuth grant/session churn no longer concentrates on the shared admin and trips the per-user login-rate cap.
… [do-not-merge] Dedicated-user experiment (c2bb5f9) did not clear login-rate: run-eval's first grant on a brand-new user still tripped the cap, proving it is not per-user (org- or IP-wide). Revert that change and instead run the run-eval JSON hook under SF_LOG_LEVEL=trace, dumping the token-redacted OAuth-exchange lines to CI to capture the rate-limit response's HTTP status / endpoint / Retry-After and settle per-org vs per-IP.
…) [do-not-merge] Replace Array.from(...) as string[] with a type-guard filter so @typescript-eslint/no-unnecessary-type-assertion passes. (Local hooks had missed it — stale node_modules was missing @eslint-react; reinstalled.)
…t-merge] Replace the do-not-merge SF_LOG_LEVEL=trace diagnostic with a login-rate retry helper (execCmdWithLoginRateBackoff) and apply it to the run-eval JSON and YAML before-hooks. The shared scratch org's per-org/per-runner login-rate budget is spent by setup + other NUTs before run-eval runs; retrying with backoff (up to 4 attempts, ~6 min total wait) lets the window recover during the serial waits. Hook timeouts raised to fit the backoff plus the ~18-min eval run.
…[do-not-merge] The backoff passed a testkit-re-exported Duration to @salesforce/kit's sleep(), whose `instanceof Duration` check missed (different class copy), producing a NaN timeout that resolved instantly — so all 4 attempts fired within ~6s and the window never had time to clear. Pass plain millisecond numbers to sleep() instead.
…e) [do-not-merge] Revert the login-rate backoff (CI run 39992637604 proved it: the Duration identity fix made sleep actually wait, but 6 min of backoff never cleared the window, and it pushed the suite into the 60-min job timeout). Instead, isolate run-eval on a DEDICATED TestSession/scratch org: - extract org provisioning into reusable provisionEvalOrg(session) - add getRunEvalTestSession()/getRunEvalUsername(); restore AGENT_USER_USERNAME to the shared org's agent user afterward so z2 publish isn't contaminated - rewire agent.test.run-eval.nut.ts to the dedicated org Discriminates per-ORG vs per-IP: if run-eval's first grant succeeds on a fresh org, the cap is per-org; if it still trips login rate exceeded, it's per-IP.
…ssion [do-not-merge] Prior attempt (2bc5f4c) failed fast: a second TestSession.create() throws 'Attempted to wrap cwd which is already wrapped' and corrupts the first session (testkit wraps process.cwd() globally — one TestSession per process). Keep the per-org isolation experiment but change the mechanism: declare a second scratch org (alias 'runeval') in the SAME shared TestSession, provision it lazily, and point the run-eval NUTs at it. provisionEvalOrg is now parameterized by alias. Still discriminates per-ORG vs per-IP for the login-rate cap.
…merge] CI run 40037029393 proved the dedicated-org experiment does not help: run-eval tripped 'login rate exceeded' on its first token grant against a fresh scratch org on a different pod/orgId than the shared org. A brand-new org with its own login budget failing means the cap is not per-org. The only constant across the shared org, the dedicated org, and all the different pods is the CI runner IP -> the budget is spent IP-wide by the rest of the suite before run-eval runs. Both isolation levers are now refuted in CI (dedicated user c2bb5f9, dedicated org 2bc5f4c/2d7d421). Restore shared-setup.ts and run-eval.nut.ts to the 8b6f429 shared-session baseline (de-dup + NUT-CONTEXT/NUT-FAILURE correlation logging kept).
…t-merge] The 'login rate exceeded' OAuth cap is per-IP (proven: dedicated user AND dedicated org both failed in CI). Every other salesforcecli plugin with a heavy NUT suite (plugin-data 11 cmds, plugin-deploy-retrieve 15) splits its NUTs across a command matrix so each group runs on its own GitHub runner IP with its own grant budget. plugin-agent ran the entire suite serially in one job per OS, so a single IP absorbed every grant and the budget was spent before run-eval's first grant. Split the 19 NUT files into 5 groups via test:nuts:* scripts + a command matrix (5 x 2 OS = 10 jobs). run-eval is isolated on its own job for maximum headroom; z0-z5 stay in one job to preserve their shared-org create->validate->publish->preview->trace ordering. adl/mcp already skip in CI (no TARGET_ORG). The shared nut.yml accepts the 'command' input.
…-merge] Un-poison the this.retries(2) on the first publish test: without --force-overwrite the generate step dies with JsonAabExistsError on every retry over the bundle dir left by a failed first attempt, so the real first-attempt publish failure never surfaces in CI. With it, retries genuinely re-publish and the true error is captured.
…[do-not-merge] The publish NUTs asserted old behavior removed by forcedotcom/agents#310: - verifyGenAiPlannerBundleExistsOrNot checked for genAiPlannerBundles/<name>_vN on disk, which publish no longer produces (moved to AiAgentDefinition/Bot retrieve). Replaced with verifyAgentMetadataRetrieved, which checks the current output dirs (aiAgents / aiAgentDefinitionVersions / bots). - verifyPublishedAgent was a dead no-op: a malformed SELECT SELECT query that always threw, caught by a void Promise.reject that swallowed every failure, and it checked BotVersion.DeveloperName (unrelated string) not the version. Fixed to query BotVersions.VersionNumber and assert the version count, so it actually verifies the publish for the first time.
…-merge] CV5 asserted the server rejects a no-visibility Internal context variable (VerifiedCustomerId) on preview start. That is purely server behavior: neither the plugin (flags.ts validates only name/type/value-type) nor the SDK (scriptAgent passes `variables` straight through) enforces visibility. The test passed when written (PR #483, 2026-09-08) because the server rejected it then. The server has since changed to accept/ignore it, confirmed e2e against a live org: the command now exits 0 and returns a valid session with no 'Internal' error. The test pinned a server contract the plugin doesn't own, so it's removed. CV1-CV4/CV6 still cover the plugin's own context-variable handling.
…t-merge]
CV1/CV2/CV3/CV6 passed the JSON payload wrapped in single quotes
(--context-variables-json '[{...}]'). execCmd runs the command through
shelljs, which uses bash on Unix (strips the single quotes -> clean JSON)
but cmd.exe on Windows (single quotes are not special -> the literal
'[...]' reaches the flag and JSON.parse rejects it). That is a test-only
portability bug: the plugin parser is correct, and quote-stripping is the
shell's job.
Add a jsonFlag() helper that quotes per-platform (single quotes on Unix,
double-quoted with escaped inner quotes on Windows) and build the payloads
as JS values. Validated both forms deliver byte-identical JSON. No product
change; Unix behavior unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Do not merge. Throwaway PR to trigger CI and verify the new NUT failure-logging output in the z-series.
Only change is a comment appended to README.md — the point is to run plugin-agent's NUTs and confirm:
[NUT-CONTEXT]appears (every run) with orgId + pod + UTC timestamp.[NUT-FAILURE]appears for the deterministic z3 failure (history never created). Note: z4 fails in abefore allhook, which the rootafterEachdoesn't catch — only[NUT-CONTEXT]covers that run.Grep the job logs for
[NUT-CONTEXT]/[NUT-FAILURE]. Close without merging when done.Companion do-not-merge PR in agents: forcedotcom/agents#343.