Repository navigation
ENG-3685: name the functional suite's backend asset ids, once - #1140
Closed
elsheikhams99 wants to merge 4 commits into
Closed
elsheikhams99 wants to merge 4 commits into
elsheikhams99 wants to merge 4 commits into
Conversation
Every v2 functional test that needed a platform asset carried the raw ObjectId inline: 24 hex characters, no name, no owner, no override. The default LLM appeared in four files, the Slack integration in seven, and Firecrawl/Tavily/Google Search were copy-pasted byte-for-byte between the agent and tool suites. Those literals are not portable. CI resolves them against test-platform-api, a push to main resolves them against production, and a laptop with no BACKEND_URL resolves them against dev-platform-api: three separate id spaces. A retired asset therefore failed one leg at a time, and each failure read as an unrelated backend error from somewhere deep inside a test body. tests/functional/_assets.py is now the single source: one `AssetIds` dataclass per environment (dev/test/prod, selected by BACKEND_URL), a named field per asset, and an AIXPLAIN_TEST_<NAME> override for a single run -- which is also how a backend this module has never heard of gets a working id without a commit. Retiring an id is one line. A session fixture in tests/functional/v2/conftest.py resolves every id before any test runs and reports all the gaps at once, naming the asset, the id, the call that failed and the override that unblocks the run. The v2 conftest now imports its default backend from the same module rather than repeating the literal, because the id space selected and the backend the client talks to drifting apart is the failure this ticket is about. test_arabic_agent.py parametrises on the asset *name* rather than the id, so collection stays backend-independent. Its `gpt-4o` label was the correct half of the pair the ticket flags: the id was onboarded 2024-05-16 per its ObjectId timestamp, four days after GPT-4o shipped and five weeks before Claude 3.5 Sonnet did, and skills/aixplain-builder gives it as the id form of openai/gpt-4o. The "Anthropic Claude 3.5 Sonnet v1" comment the v1 suite attached to it was the stale one; the dating is recorded next to the id. Guards: tests/unit/test_functional_hygiene.py fails on any bare 24-hex literal under tests/functional/ -- in a comment as readily as in code, since one of the removed ids had been documenting a tool that no longer existed -- plus two checks that stop it passing vacuously. The module's own logic (host matching, overrides, unknown backends) is covered in tests/unit/test_functional_assets.py, following the cleanup_guards/test_functional_cleanup split. Scope is the v2 suite. The v1 legs still carry their own literals and are deliberately untouched: PROD-2918 deletes all eight of those directories, so rewriting them here would buy a delete/modify conflict and nothing else. The exemption is a named tuple in the guard rather than a detector quietly pointed at tests/functional/v2, and it matches nothing once PROD-2918 lands.
…d-functional-asset-ids
…nest Addresses the review findings on this branch. The ENG-3544 blanket-skip guard had gone blind to the v2 suite. Its detector treated any function whose AST dump contained "environ" as guarded, and the autouse verify_assets_resolve fixture contained that text only by accident (its docstring, an `environment` variable, `environment_for`). An unconditional pytest.skip there still passed, and with the API-key read moved behind _api_key(), the real-conftest check only matched slack_token. Now: - the `client` and `assets` fixtures read TEAM_API_KEY/AIXPLAIN_API_KEY with an inline os.getenv, where the detector can see it; - the detector needs a real getenv/environ attribute or name node, not the substring; - test_ci_matrix_coverage.py has a regression case: an autouse fixture that skips unconditionally while naming "the environment" is flagged; - the real-conftest check requires every skip in the v2 conftest to be guarded and one guard to read the API key. A new test deletes the env reads from the real conftest and expects every skip to be flagged. Verified by mutation in a scratch copy: an unconditional skip in client or assets, an autouse blanket skip, or the guard behind _api_key() each fail the suite. The session-wide, autouse verify_assets_resolve is replaced by per-name resolution. The `assets` fixture returns LazyAssets: the first read of `assets.X` resolves X through SPECS (client.<resource>.get(id)) and caches the outcome, success or failure, so @flaky reruns don't hit the backend again. A missing asset fails only the tests that read it, with the same message as before (asset, id, backend, call, override). A leg that reads no asset makes no resolution call and builds no client. The fixture checks credentials before it computes the id space, so no key plus an unrecognised BACKEND_URL skips instead of erroring. Host matching in tests/functional/_assets.py is now an exact lookup of urlparse(url).hostname, lower-cased. Before, a substring test read staging-platform-api as prod and a query string naming test-platform-api as test. AIXPLAIN_TEST_ENV is applied only to an unrecognised host. If it contradicts a recognised one, that raises ConflictingEnvironmentError. Override and pin values are stripped, and a whitespace-only value counts as unset. Unit tests cover the new host cases, the pin rules, whitespace overrides, and LazyAssets with a fake client. A static check makes sure no autouse fixture in the v2 conftest comes back. The module docstring and AGENTS.md now describe lazy resolution and no longer treat PROD-2918 as future work. This also corrects counts in 073af9d's message: Slack was inlined in six files, not seven; two ids (Firecrawl, Tavily) were shared by the agent and tool suites, not three; and the gpt-4o dating and "eight directories" no longer apply.
Addresses the review findings on test_functional_hygiene.py: - OBJECT_ID_PATTERN is case-insensitive, keeping its lookarounds, so an upper-case ObjectId is caught too. - The scan asserts it found at least one file, so pointing it at the wrong directory can't pass vacuously. - Relative paths are compared in POSIX form, so the _assets.py exemption also holds on Windows. - The V1_LEGS/_is_v1_leg exemption is removed. v1 is gone (PROD-2918), and the generic directory names it listed would have exempted a new tests/functional/model/ file with a bare id. - A new test checks that every asset name in test_arabic_agent.py's MODELS is a field of AssetIds. It parses the module rather than importing it, so a typo fails here instead of at run time on a backend. - Docstrings are added to the test functions this PR introduced.
Collaborator
Author
|
Superseded by #1163, which combines this PR with the other ENG-3683–3692 PRs into one branch based on development (one commit per original PR, conflicts resolved), plus fixes from the first live functional CI run. |
aix-ahmet
pushed a commit
that referenced
this pull request
Oct 9, 2026
Combined from PR #1140, including its review fixes and the conflict resolutions against the PRs that come before it in this branch.
aix-ahmet
added a commit
that referenced
this pull request
Oct 9, 2026
…l (combined) (#1163) * ENG-3686: remove committed API keys and scan for secrets on every push Combined from PR #1139, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3685: name the functional suite's backend asset ids, once Combined from PR #1140, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3684: hold each CI leg to an execution ratio, not just one executed test Combined from PR #1138, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3683: gate PRs and releases on the functional matrix Combined from PR #1137, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3691: ticket-key every permanent skip, enforce registered markers, drop dead fixtures Combined from PR #1158, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3692: align v2 test hygiene, markers and lint scope Combined from PR #1157, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3687: add functional coverage for untested v2 resources Combined from PR #1154, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3689: add v2 team-agent functional tests Combined from PR #1155, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3688: port agent lifecycle functional tests to v2 Combined from PR #1156, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3690: cover public agent run-time features functionally Combined from PR #1162, including its review fixes and the conflict resolutions against the PRs that come before it in this branch. * ENG-3688: send expected output as a string in run payloads build_run_payload turned a Pydantic class into model_json_schema() and an instance into model_dump(), both dicts, so executionParams.expectedOutput reached the backend as an object. The backend rejects that with "400 executionParams.expectedOutput must be a string", which is how test_agent_lifecycle.py::test_agent_expected_output_run_returns_json failed in the first live CI run. Save already sent json.dumps(schema). Run payloads now JSON-encode a class (the same schema string save persists) and an instance, as they already did for a dict. Unit tests cover the class passed through execution_params, the agent's own class, and an instance. * ENG-3688: assert the backend's in-use error code test_delete_agent_in_use_fails expected "cannot be deleted" in the error, but the backend refuses the delete with the code err.agent_is_in_use ("Failed to delete resource: err.agent_is_in_use" in the first live CI run). Assert on that code instead of prose the backend never sends. * ENG-3688: stop asserting max_inspectors survives a fetch test_agent_round_trips_execution_and_inspector_fields failed in the first live CI run with "assert None == 2" on fetched.max_inspectors. The SDK encodes and decodes the field under the same key (maxInspectors, checked offline: build_save_payload sends it, from_dict reads it back), so the None means the backend does not persist it. The test still saves with max_inspectors=2, so the backend must accept it, and keeps asserting the fields that do round-trip. * ENG-3690: reword the prompt and variables run tests neutrally Both tests asked the model to repeat a "secret", and in the first live CI run GPT-5.4 treated that as a request to leak hidden instructions: test_prompt_override_reaches_model got "I can't share secrets or hidden instructions", and test_variables_are_substituted_into_instructions got the literal "word" back instead of the substituted value. Each now asks for a random token ("Reply with exactly this code and nothing else: <token>") and asserts the token appears. The variables test keeps the placeholder in the instructions, where the SDK rewrites {{x}} to {x} on save and the backend substitutes the query keys at run time (the same contract v1's process_variables used), so the SDK path is unchanged. * ENG-3690: verify only acceptance of run-time inspectors test_run_time_inspector_runs expected a step for the run-time inspector, but in the first live CI run the response's steps held only the agent's own LLM step, so the response exposes no evidence the inspector ran. Renamed to test_run_time_inspector_accepted per the file's *_accepted convention: it asserts the inspectors list reaches the payload as given and that the backend accepts the run. Drops the now-unused _step_agent_label helper. * ENG-3687: page through a sorted search when checking pagination test_second_page_does_not_repeat_the_first compared pages 0 and 1 of an unsorted page_size=1 model search, and in the first live CI run both returned the same model (6ab3e2f8...). Without sort_by the SDK sends sort: [{}], so the backend's order is not guaranteed across two calls, and whether pageNumber counts from 0 or 1 is not something the test should depend on. The test now sorts by creation time and compares pages 1 and 2 (distinct under either convention) with three records each, asserting the two id sets are disjoint. The SDK sends pageNumber/pageSize exactly as v1 did, so no SDK change. * ENG-3687: give every functional test skill a unique name The skill_dir fixture named each skill func-test-skill-<epoch seconds>. Two skills created within the same second (consecutive tests, or a rerun) share that name, and in the first live CI run the backend refused the second with err.exist_name, erroring the setup of TestAgentReflectsSkillUpdate::test_agent_run_reflects_updated_skill_content. Append a uuid fragment, as the agent names in the same file already do. * ENG-3684: fall back to any multi-action tool In the first live CI run every caller of resolve_multi_action_tool failed ("Searched 100 tool(s) ... Slack-backed tools with actions available: none"), and both TestInputActionFieldsRoundTrip tests failed on "Pinned integration 686432941223092cb4294d3f reports no actions available". The cause is actions_available, not the tenant: the backend leaves it unset (None) on search results and integrations. The same run shows the Slack integration with actions_available=None while test_tool.py::test_tool_get_parameters passed on that integration's non-empty list_actions(). Filtering on the field ruled out every tool. resolve_multi_action_tool now skips only an explicit False and probes candidates with list_actions(): Slack-backed tools first, then tools of any other integration, then the rest, bounded to 5 pages and 25 probes. The AIXPLAIN_TEST_MULTI_ACTION_TOOL_ID override is unchanged, and finding nothing still fails, naming every probed tool and its action count. The snake-case round-trip tests use the new resolve_action_source: the pinned Slack integration when it lists actions, else the resolved multi-action tool (same list_actions/list_inputs surface), failing loudly when neither lists any. * ENG-3683: cap functional matrix parallelism The first live run of the functional matrix lost ~40 tests to "429 err.agent_concurrency_limit_reached" (limit 20 for the test team): every leg drives agents on the same tenant, several at -n 2, and all legs started at once. Those failures say nothing about the SDK. Add max-parallel: 4 to the functional job's strategy, with the reason beside it, and a unit test in test_ci_matrix_coverage.py that fails if the cap is dropped or raised above four. required-checks.md notes the cap next to the concurrency group that bounds runs per PR. * ENG-3688: send expected output as a string from session runs too ExecutionConfig.to_api_dict passed execution_params['expected_output'] through unchanged, so a session run with a Pydantic class, instance or dict sent a non-string executionParams.expectedOutput, which the backend rejects with "400 executionParams.expectedOutput must be a string". This is the same bug the previous commit fixes on the direct run path. The session path now encodes it the same way: a class as its JSON schema, an instance as its JSON, and a dict or list with json.dumps. * ENG-3684: xfail the sync-only model when the asset lacks connection_type metadata ENG-3684 turned the old skip in test_sync_model_connection_type into an assert, but the pinned sync-only model exposes no connection_type metadata on the backend, so the model leg went red on a fact no SDK change can fix. Both connection-type tests now call pytest.xfail when the metadata is missing, naming the pinned asset and the AIXPLAIN_TEST_SYNC_ONLY_MODEL / AIXPLAIN_TEST_ASYNC_ONLY_MODEL override. When the metadata is present the real assertions run unchanged, so the tests start passing by themselves once the asset carries it or is re-pinned. * ENG-3688: use one expectedOutput encoder for save, run and session expectedOutput was encoded three times, differently. build_run_payload JSON-encoded a Pydantic class, instance or dict but sent a list raw; ExecutionConfig.to_api_dict also encoded lists; build_save_payload turned a class into its schema string but stored a Pydantic instance (and a dict) as an object, which the agent engine reads back as a Python repr when a run falls back to the stored value. All three now call session._expected_output_to_wire (imported lazily in agent.py, as resolve_attachments already is, to avoid an import cycle): class -> json.dumps(model_json_schema()), instance -> model_dump_json(), dict/list -> json.dumps, str/None unchanged. The run path keeps its rule that an empty string under text/markdown is sent as None. The save-side change is deliberate. The only test pinning a dict at save, test_basemodel_instance_is_still_dumped_to_dict, preserved the original v2 behaviour rather than a backend contract: the save endpoint already stores the schema string for a class. It now asserts a JSON string, and new tests cover a dict at save, save/run agreement, a list on the run path and the text-format empty-string rule. CHANGELOG notes the wire-format fix and that a fetched agent now carries the string. * ENG-3688: assert inspector fields on the save payload, not after fetch test_agent_round_trips_execution_and_inspector_fields failed in CI with "assert [] == ['output']": the platform's v2 create/update path does not persist inspectorTargets or maxInspectors, and the read side defaults the targets to []. That is a backend gap (ticket to be filed), not an SDK bug. The test now asserts both fields on agent.build_save_payload() -- what the SDK actually sends -- before saving, with a comment saying why and when to move them back below the fetch. The output_format and expected_output round-trip assertions are unchanged. * ENG-3690: make the run-params leg report backend gaps as expected failures Five tests in test_agent_run_params.py were red in CI for reasons that are not SDK bugs, and one for a fixture bug: - variables and prompt: the SDK sends the v1-compatible contract (query {input, <vars>}; top-level prompt/criteria/identifier), but the agent engine reads only query.input. Each test is split into a payload test that must pass and an output test under xfail(strict=True, raises=AssertionError). A refused run still raises APIError and a non-success status calls pytest.fail, so only the output check can be the expected failure. - budget maxDurationSeconds=0: the payload carries the cap, but the run hangs IN_PROGRESS on the backend. The payload is asserted first, the run gets timeout=60 instead of the 300 s default, and the test is a strict xfail on the SDK TimeoutError only. - attachments: RED_PNG_BYTES was one pixel at 50% alpha, so "coral" was a correct answer. It is now a 16x16 8-bit RGB PNG with no alpha channel, generated with zlib/struct and decoded to confirm every pixel is (255, 0, 0). - test_debug_returns_analysis duplicated test_debugger.py, so one missing meta-agent turned two legs red. Deleted; the module docstring now says where debug() is covered and how the xfails are structured. * ENG-3689: xfail the team expected-output run until the engine resolves $defs test_run_team_agent_with_expected_output failed in CI with a pydantic ValidationError: _People nests _Person, so its JSON schema reaches _Person through $ref/$defs, which the agent engine does not resolve. The nested type degrades to a list of anything and the model answered {"result": ["Bruno: 34 ..."]}. The test is now xfail(strict=True, raises=pydantic.ValidationError) with a backend reason (ticket to be filed). The persistence checks, a failed run and a missing delegation raise AssertionError, which the xfail does not accept, so only the structured-output validation can be the expected failure. The flaky mark is dropped: rerunning a known backend gap only spends three billed team runs to reach the same xfail. * ENG-3689: drop the run_response_generation team test test_run_response_generation_accepted spent a billed, rerun-able team run to show only that the backend accepts run_response_generation, while test_agent_run_params.py documents that the agent runtime ignores the flag (and its JSON-output test already passes it). Two files disagreeing about the same parameter is worse than one saying it once. Delete the test and replace its line in the module docstring with a note pointing at test_agent_run_params.py. * ENG-3687: never file issues against production tests/functional/v2/test_issue.py files real issues, and there is no API to delete one. The issue leg ran on every push to main, against the production backend, so each merge added two undeletable test reports to the production issue system. The issue leg now carries `prod_safe: false` in the matrix. "Run Tests" receives the leg's path and that key through env: (no more `${{ }}` in the script body) and, when IS_PROD is true and the key is false, prints a ::notice:: explaining the skip and exits 0 without running pytest. The leg still runs on PRs, pushes to test and the nightly, against the test backend. On production runs `functional (issue)` therefore reports success without having run, and release.yaml's gate counts it green. That is deliberate (no production run can exercise it without the side effect) and is stated in the workflow comment and docs/ci/required-checks.md. New unit tests run the real step with a stub python: the issue leg runs pytest when IS_PROD is false and skips with a notice when it is true, every other leg still runs its exact pytest command on production, and the step body holds no `${{ }}`. The expression evaluator learned `matrix.*` lookups for this. * ENG-3683: hand SLACK_TOKEN only to the legs that read it, and drop HF_TOKEN SLACK_TOKEN and HF_TOKEN were exported to every functional leg, including same-repository PR runs, which pip install whatever dependency changes the PR carries. Only three legs read SLACK_TOKEN (agent-lifecycle, model and tool, through the slack_token fixture), and nothing under tests/functional reads HF_TOKEN at all; aixplain/utils/config.py defines it but no SDK or test path consumes it. Those three legs now carry `slack: true` in the matrix, and the env step expands the secret as `matrix.slack && secrets.SLACK_TOKEN || ''`, the same pattern that gates the _PROD secrets, writing SLACK_TOKEN to GITHUB_ENV only when it is non-empty. HF_TOKEN is no longer exported. Unit tests: the `slack` set must equal the legs whose test file uses the slack_token fixture, and every other leg renders SLACK_TOKEN as empty; the real env step exports SLACK_TOKEN only when it has a value; nothing under tests/functional reads HF_TOKEN and main.yaml does not export it. The secrets table in docs/ci/required-checks.md says the same. * ENG-3683: count only runs that prove they used production in the release gate release.yaml's functional gate recognised a manual dispatch on the release tag with `.event == "workflow_dispatch" and .head_branch == $tag`. A run's head_branch is a bare name, so a dispatch on a *branch* named exactly like the tag (which IS_PROD treats as a test-backend run) passed the filter, and its green legs could satisfy a production release. main.yaml gains a trivial `production-backend` job whose `if:` is the functional job's IS_PROD expression (repeated, since a job-level `if:` cannot read another job's env). The gate now reads each candidate run's jobs through a jq `jobs_filter` that yields the run's successful job names only when `production-backend` is among them; a run without it is logged as a warning and not counted. Checking the real ref inside the run is robust where a "does refs/heads/$TAG exist now" lookup is not: a branch deleted after its dispatch would slip past that. Unit tests: the job's condition matches IS_PROD for every event/ref case, including a dispatch on a branch named `v0.3.1`; the jobs filter, run on paginated fixture JSON, drops a run whose production-backend job was skipped and keeps one where it succeeded on a later page; the gate script still wires the filter in; the required-checks doc names the new job, which is listed as not to be required (it is skipped on every PR). * ENG-3683: let the leg timeout fire before the functional job timeout Six legs have `timeout: 45`, which equalled the functional job's `timeout-minutes: 45`. When a leg hung, the job-level kill landed first, so the leg showed as cancelled and its teardown never ran -- the per-leg timeout could never do its job. Raise the job timeout to 55, above the longest leg timeout, with the reason beside it, and add a unit test in test_ci_matrix_coverage.py that fails if the job timeout is not above every leg's. * ENG-3683: correct stale CI docs and comments - AGENTS.md pointed at tests/mock_responses/, which ENG-3691 deleted, and said every leg has a 30-minute timeout; legs declare 30 or 45 minutes. - main.yaml's concurrency comment (and the matching unit-test docstring) said the matrix is 14 legs; it is one leg per functional test file, so say that instead of a count that rots. - docs/ci/required-checks.md left `benchmark` out of the deleted v1 legs. - release.yaml (and a unit-test message) justified `actions: read` by the repository being private; it is public. The scope is needed because the workflow's deny-all default grants nothing. * ENG-3686: keep leaked-key details out of the public doc docs/ci/secret-scanning.md is published in a public repository, and it listed each removed credential's file and line, the backend it worked against, a partial fingerprint of each key and the path of a third token, alongside the statement that none had been rotated. That is a map for anyone looking for live credentials in history. The doc now says only what a reader of this repository needs: committed credentials were removed in ENG-3686, they remain in history and must be rotated, rotation is urgent and independent of any PR, and the specifics are tracked privately in the ENG-3686 ticket. The scanner design, the repo rules, the gitleaks-vs-detect-secrets rationale and the allowlist policy are unchanged; the open items are kept, without identifiers. The same details are removed from the test_secret_scanning.py module docstring, the .gitleaks.toml header comments, the test_eval.py docstring (which tied a deleted script to its key and backend) and a test_agent_lifecycle.py docstring that named the provider of a leaked MCP URL. * ENG-3686: drop the dead login_response.json allowlist entry .gitleaks.toml exempted, by exact value, the recorded ECR password in tests/mock_responses/login_response.json. ENG-3691 deleted that file, so the entry matches nothing, and the allowlist policy is to remove a dead exemption: it can only hide something later. gitleaks dir . with the pinned config still reports no leaks; with the value allowlists removed it reports only the documentation placeholders and the hand-written fake keys in unit tests, which is what the updated docs/ci/secret-scanning.md describes in place of the old "8 findings". * ENG-3684: let a failing integration action listing fail the test resolve_action_source caught every exception from the pinned integration's list_actions() and treated it as "no actions", then fell back to a multi-action tool. A broken listing endpoint therefore went unnoticed: the snake-case round-trip tests passed on the fallback tool while the integration path they are meant to cover was down. Only an empty listing falls back now; an exception propagates and fails the test with the real error. The docstring says why. * ENG-3687: check that pages 0 and 1 of a sorted search are disjoint test_second_page_does_not_repeat_the_first compared pages 1 and 2 only, so it could no longer see the bug it was written around: in the first live CI run pages 0 and 1 returned the same model. The SDK documents page_number as 0-based, and a backend that counts from 1 and clamps 0 up to 1 would alias exactly those two pages. Under the stable CREATED_AT sort the test now also fetches page 0 and asserts it is full and shares nothing with page 1, with a message naming the 1-indexing suspicion. The 1-vs-2 check stays first, so a failure tells the two problems apart. * ENG-3684: create the recurring test triggers disabled test_trigger.py created an enabled trigger every two hours (plus daily, weekly and monthly ones) and relied on teardown to delete them. PR runs use cancel-in-progress, so a newer push can cancel a leg before teardown runs, leaving an active recurring trigger that keeps running its agent on the shared test backend. The recurring triggers are now created with enabled=False. What those tests check -- the schedule_type mapping and notifications -- does not depend on the trigger being active, and enable/disable is still covered by test_enable_disable_via_save on a far-future one-off trigger. The two-hourly test also asserts the fetched trigger is stored disabled, so a backend that ignored the flag on create would show up rather than quietly leak an active schedule. * ENG-3687: encode every non-string expected_output in one shared module Tuples, numbers and bools went out unencoded and still drew the 400; dicts were ASCII-escaped while Pydantic instances kept UTF-8. The encoder now lives in aixplain/v2/_expected_output.py so Agent no longer imports it lazily from session.py, and the read-back change is listed under Breaking. * ENG-3685: cache only permanent asset-resolution failures One 503, timeout or 429 on the first read of an asset was cached for the whole session, so every later test in the worker, @flaky reruns included, replayed it. Successes and definite failures (a 4xx other than 408/409/425/429, an explicit retryable=False, an undeserializable response, a client that cannot be built) are still cached; anything else is asked again on the next read. * ENG-3683: tighten functional-scope paths, label cancellation and the Slack leg - The path filter now covers every module directly under tests/ and pytest.ini, so a PR touching only ci_guards.py, cleanup_guards.py or the marker registry no longer skips the matrix and passes functional-result green. - An unrelated label no longer cancels a running matrix: its run queues in the PR's group instead. A push or the functional-tests label still cancels. - agent-lifecycle no longer holds SLACK_TOKEN: its only Slack test is skipped (BUG-1098). The guard now ignores unconditionally skipped tests, so it fails when that skip is removed without restoring slack: true. * ENG-3686: catch platform keys after per-environment credential names The separator between the credential name and the key could not contain a-f, so TEAM_API_KEY_PROD="<key>", AIXPLAIN_API_KEY_DEV=<key>, SECRET_KEY = "<key>", api_key_test: <key> and {"x-aixplain-key": "<key>"} all went unreported. The rule now needs only a non-hex character right before the key, and aixplain-key is a credential name. Checked with gitleaks v8.28.0: all five shapes are caught, a bare sha256 digest is not, and the tree scan stays clean. * ENG-3684: resolve the multi-action tool once per module and stop masking listing errors multi_action_tool re-ran the bounded search (up to 5 Tool.search pages and 25 list_actions probes) for each of its consumers; the id is now resolved once per module and each test gets a fresh Tool.get. _action_count no longer turns every exception into 0: a pinned tool's listing error propagates, and a search candidate is passed over only when the backend refuses that tool with a 4xx. * ENG-3688: keep two run tests off backend gaps they do not test - test_deprecated_files_alias_uploads_and_reads reads a .txt file, so it runs on the default LLM instead of VISION_LLM, whose outages turned the leg red. - test_agent_expected_output_run_returns_json uses a flat schema: the engine does not resolve $ref/$defs (the strict xfail in test_team_agent.py), so the nested People schema could fail the leg on all three flaky attempts. * ENG-3684: share unique_name and agent_ids from tests/functional/_helpers.py _unique, _name, _unique_name and _agent_ids were redefined in five test files. They are now unique_name (uuid-suffixed, safe under xdist and reruns) and agent_ids (id strings, dicts or Agent objects) in the shared helpers module. * ENG-3684: add the Apache 2.0 header to the new functional modules AGENTS.md requires it on new source files; _assets.py, _helpers.py and the ten test modules this PR adds had none. Appended to each module docstring, as in aixplain/v2/file.py. * ENG-3684: find the event-trigger integration and connection without secrets TEST_COMPOSIO_INTEGRATION_ID / TEST_CONNECTION_ID (and their _PROD variants) were never created, so the trigger leg was red on every run and release.yaml could not release. Neither is a credential: - the integration is fetched by its marketplace path, composio/gmail, which resolves on every backend; - the connection comes from resolve_trigger_connection, a bounded search for a connected tool that lists event-trigger types (Gmail-backed tools first), pinnable with AIXPLAIN_TEST_TRIGGER_CONNECTION_ID. The workflow no longer reads the four secrets, and a unit guard keeps them out. --------- Co-authored-by: aix-ahmet <ahmet.gunduz@aixplain.com>
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
1 participant
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.
Every v2 functional test that needed a platform asset carried the raw ObjectId inline: 24 hex characters, no name, no owner, no override. The default LLM appeared in four files, the Slack integration in seven, and Firecrawl/Tavily/Google Search were copy-pasted byte-for-byte between the agent and tool suites.
Those literals are not portable. CI resolves them against test-platform-api, a push to main resolves them against production, and a laptop with no BACKEND_URL resolves them against dev-platform-api: three separate id spaces. A retired asset therefore failed one leg at a time, and each failure read as an unrelated backend error from somewhere deep inside a test body.
tests/functional/assets.py is now the single source: one
AssetIdsdataclass per environment (dev/test/prod, selected by BACKEND_URL), a named field per asset, and an AIXPLAIN_TEST override for a single run -- which is also how a backend this module has never heard of gets a working id without a commit. Retiring an id is one line.A session fixture in tests/functional/v2/conftest.py resolves every id before any test runs and reports all the gaps at once, naming the asset, the id, the call that failed and the override that unblocks the run. The v2 conftest now imports its default backend from the same module rather than repeating the literal, because the id space selected and the backend the client talks to drifting apart is the failure this ticket is about.
test_arabic_agent.py parametrises on the asset name rather than the id, so collection stays backend-independent. Its
gpt-4olabel was the correct half of the pair the ticket flags: the id was onboarded 2024-05-16 per its ObjectId timestamp, four days after GPT-4o shipped and five weeks before Claude 3.5 Sonnet did, and skills/aixplain-builder gives it as the id form of openai/gpt-4o. The "Anthropic Claude 3.5 Sonnet v1" comment the v1 suite attached to it was the stale one; the dating is recorded next to the id.Guards: tests/unit/test_functional_hygiene.py fails on any bare 24-hex literal under tests/functional/ -- in a comment as readily as in code, since one of the removed ids had been documenting a tool that no longer existed -- plus two checks that stop it passing vacuously. The module's own logic (host matching, overrides, unknown backends) is covered in tests/unit/test_functional_assets.py, following the cleanup_guards/test_functional_cleanup split.
Scope is the v2 suite. The v1 legs still carry their own literals and are deliberately untouched: PROD-2918 deletes all eight of those directories, so rewriting them here would buy a delete/modify conflict and nothing else. The exemption is a named tuple in the guard rather than a detector quietly pointed at tests/functional/v2, and it matches nothing once PROD-2918 lands.