mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-09-20 18:13:49 +00:00
* feat(cli): backfill project_id from every user-invoked project command `crewai run` has always backfilled: a project declaring [tool.crewai] without a project_id gets one minted the first time it runs. No other command did, so a project driven entirely through `crewai test`, `crewai deploy` or `crewai traces enable` never acquired an id and every one of its runs stayed unattributable - which is the denominator problem, not a cosmetic gap. Adds the same call to train, replay, test, login, deploy create, deploy push, flow add-crew, enterprise configure and traces enable. Every one is an action the user explicitly invoked, which is the condition run_crew already relies on, so this is the existing principle applied evenly rather than a new policy. It is still never called from the SDK during kickoff, and get_or_create_project_id still refuses to create the [tool.crewai] table, so an unrelated directory is never rewritten. `crewai flow kickoff` is deliberately untouched: it delegates to run_crew and already inherits the backfill. A test pins that so the delegation is not accidentally duplicated. There is no `crewai evaluate` command - `crewai test` is that path. The call is the first statement in each command so a command that later fails still leaves the project with an id. The tests patch the backfill to raise, which proves the call happened and guarantees nothing after it runs, so no test touches user settings, spawns a subprocess or reaches the network. Verified they fail against the unpatched module: 9 command tests fail, the 2 guard tests still pass. Tests live under lib/crewai/tests/cli/ because that is the path the required CI job runs; nothing runs lib/cli/tests/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN * test(cli): assert the backfill at runtime instead of reading source text Addresses CodeRabbit and github-code-quality on #7057. The two guard tests grepped module source for a call string, which asserts on formatting rather than behavior: a reformat would break them and a real regression could slip past. They now invoke the commands in an isolated project and assert on observed calls. The flow-kickoff test patches the two distinct import sites separately and asserts run_crew's is called exactly once while cli's is not called at all, which is what makes 'delegates' and 'duplicates' distinguishable at runtime rather than by reading the file. Verified both catch what they claim: injecting a duplicate call into flow_run fails the delegation test, and removing run_crew's own call fails the run test. This also drops the module-level 'import crewai_cli.cli as cli_module' that mixed import styles with the existing 'from crewai_cli.cli import crewai', which is the code-quality finding - the rewrite removes the need for it entirely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN * test(cli): make the exact-once assertion observable Addresses CodeRabbit on #7057, and the finding was correct: with side_effect=_BackfillReached the mock raised on first use, so call_count == 1 was guaranteed by the mock rather than by the code. A second backfill call inside the same run_crew execution could never have been observed. Both backfill mocks now return normally and execution is stopped at the first call AFTER the backfill (configured_project_json_crew), so the recorded count is real. Verified the difference this makes: injecting a duplicate get_or_create_project_id() INSIDE run_crew now fails both tests, which the previous version could not detect at all. The flow_run duplicate case is still caught. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN * test(cli): assert flow kickoff reaches the post-backfill boundary Addresses CodeRabbit on #7057, and the finding was right: the flow-kickoff test discarded the runner.invoke() result, so if the path returned or raised after one backfill call but before configured_project_json_crew, both call-count assertions would still have passed - for the wrong reason. test_run_still_backfills already asserted the boundary; this makes the pair consistent. Verified it earns its place: injecting an early return after the backfill and before the boundary now fails both tests, and previously would have failed neither. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN * test(cli): pin that the backfill precedes command-specific work Addresses CodeRabbit on #7057. The finding is valid: the parametrized test proves the backfill is reached, not that nothing ran before it, so its assertion message claimed more than the test established. Fixed in two parts rather than as proposed. The message now states what the test actually proves, and a new test pins the ordering on login: , whose first action goes through a module-level name that can be patched without reaching into the command. Deliberately not parameterized across all nine commands, which is what the finding suggested: that would mean naming each command's current first action, and those change as commands evolve, so the suite would end up tracking their internals rather than this ordering property. One representative command establishes it, and placement is visible in the diff for the rest. Verified it catches the regression: swapping login's first two statements so its own work runs before the backfill fails the new test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfV2uMqWRcdfufMvtdCVoN --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
182 lines
7.3 KiB
Python
182 lines
7.3 KiB
Python
"""Every user-invoked CLI command that touches a project backfills its `project_id`.
|
|
|
|
`crewai run` has always backfilled: a project that declares `[tool.crewai]` but no
|
|
`project_id` gets one minted the first time the user runs it. No other command did,
|
|
so a project driven entirely through `crewai test`, `crewai deploy` or
|
|
`crewai traces enable` never acquired an id and every one of its runs stayed
|
|
unattributable.
|
|
|
|
These commands are all actions the user explicitly invoked, which is the same
|
|
condition `run_crew` already relies on, so extending the backfill needs no new
|
|
policy. It is still never called from the SDK during kickoff, and
|
|
`get_or_create_project_id` still refuses to create the `[tool.crewai]` table, so an
|
|
unrelated directory is never rewritten.
|
|
|
|
The patched backfill RAISES here rather than returning. That proves the call happened
|
|
and simultaneously guarantees nothing after it runs, so no test touches real user
|
|
settings, spawns a subprocess, or reaches the network.
|
|
"""
|
|
|
|
from pathlib import Path
|
|
from unittest import mock
|
|
|
|
from click.testing import CliRunner
|
|
from crewai_cli.cli import crewai
|
|
import pytest
|
|
|
|
|
|
class _BackfillReached(Exception):
|
|
"""Raised by the patched backfill so the command stops at that point."""
|
|
|
|
|
|
class _StopAfterBackfill(Exception):
|
|
"""Raised at the first call AFTER the backfill, to stop without masking it.
|
|
|
|
Used where a call *count* is asserted. If the backfill itself raised, the count
|
|
would be capped at one by the mock rather than by the code under test, so a
|
|
duplicate call could never be observed.
|
|
"""
|
|
|
|
|
|
# (test id, argv). Args are the minimum click accepts; the command body is never
|
|
# reached beyond the backfill call, so nothing here needs to be a valid target.
|
|
COMMANDS = [
|
|
("train", ["train", "-n", "1"]),
|
|
("replay", ["replay", "-t", "task-1"]),
|
|
("test", ["test"]),
|
|
("login", ["login"]),
|
|
("deploy_create", ["deploy", "create"]),
|
|
("deploy_push", ["deploy", "push"]),
|
|
("flow_add_crew", ["flow", "add-crew", "some_crew"]),
|
|
("enterprise_configure", ["enterprise", "configure", "https://example.test"]),
|
|
("traces_enable", ["traces", "enable"]),
|
|
]
|
|
|
|
|
|
@pytest.fixture
|
|
def runner():
|
|
return CliRunner()
|
|
|
|
|
|
@pytest.mark.parametrize(("name", "argv"), COMMANDS, ids=[c[0] for c in COMMANDS])
|
|
def test_command_backfills_project_id(runner, name, argv):
|
|
with mock.patch(
|
|
"crewai_cli.cli.get_or_create_project_id",
|
|
side_effect=_BackfillReached,
|
|
) as backfill:
|
|
result = runner.invoke(crewai, argv)
|
|
|
|
assert backfill.called, (
|
|
f"`crewai {' '.join(argv)}` did not backfill project_id, so a project driven "
|
|
f"only through this command never becomes attributable"
|
|
)
|
|
assert isinstance(result.exception, _BackfillReached), (
|
|
"the backfill must be reached, not merely importable. This does NOT by itself "
|
|
"prove nothing ran before it -- see "
|
|
"test_backfill_precedes_command_specific_work for that, which pins the "
|
|
"ordering on one representative command"
|
|
)
|
|
|
|
|
|
def test_backfill_precedes_command_specific_work(runner):
|
|
"""The backfill runs before the command does anything of its own.
|
|
|
|
Placement is the point: a command that fails partway must still leave the project
|
|
with an id. The parametrized test above proves the backfill is *reached*, which is
|
|
not the same claim -- work could in principle happen first and still satisfy it.
|
|
|
|
Pinned on `login` because its first action is a call through a module-level name
|
|
(`Settings`) that can be patched without reaching into the command. One
|
|
representative command is deliberate: asserting this for all nine would mean
|
|
naming each command's current first action, which changes as commands evolve and
|
|
would make the suite track their internals rather than this ordering property.
|
|
"""
|
|
with mock.patch(
|
|
"crewai_cli.cli.get_or_create_project_id", side_effect=_BackfillReached
|
|
) as backfill, mock.patch("crewai_cli.cli.Settings") as settings:
|
|
result = runner.invoke(crewai, ["login"])
|
|
|
|
assert backfill.called
|
|
assert isinstance(result.exception, _BackfillReached)
|
|
assert not settings.called, (
|
|
"login touched its own first action before backfilling; a failure after that "
|
|
"point would leave the project without an id"
|
|
)
|
|
|
|
|
|
MINIMAL_PYPROJECT = """\
|
|
[project]
|
|
name = "demo"
|
|
version = "0.1.0"
|
|
|
|
[tool.crewai]
|
|
"""
|
|
|
|
|
|
def _in_a_project(runner):
|
|
"""A cwd that `crewai run` accepts, so execution reaches the backfill call."""
|
|
return runner.isolated_filesystem()
|
|
|
|
|
|
def test_run_still_backfills(runner):
|
|
"""`crewai run` was already correct and must stay that way.
|
|
|
|
The backfill returns normally and execution is stopped at the next call in
|
|
run_crew instead, so the recorded call count is real rather than an artifact of
|
|
the mock raising on first use.
|
|
"""
|
|
with _in_a_project(runner):
|
|
Path("pyproject.toml").write_text(MINIMAL_PYPROJECT, encoding="utf-8")
|
|
with (
|
|
mock.patch("crewai_cli.run_crew.get_or_create_project_id") as backfill,
|
|
mock.patch(
|
|
"crewai_cli.run_crew.configured_project_json_crew",
|
|
side_effect=_StopAfterBackfill,
|
|
),
|
|
):
|
|
result = runner.invoke(crewai, ["run"])
|
|
|
|
assert backfill.call_count == 1, "`crewai run` stopped backfilling project_id"
|
|
assert isinstance(result.exception, _StopAfterBackfill), (
|
|
"execution must have reached the boundary after the backfill, otherwise the "
|
|
"call count above proves nothing about ordering"
|
|
)
|
|
|
|
|
|
def test_flow_kickoff_delegates_the_backfill_and_does_not_duplicate_it(runner):
|
|
"""`crewai flow kickoff` must inherit the backfill from run_crew, not repeat it.
|
|
|
|
A second call would mint under a lock run_crew is about to take -- wasted work
|
|
rather than a correctness bug, but it would also hide the delegation from anyone
|
|
reading the command.
|
|
|
|
Both backfill mocks return normally and execution is stopped at the first call
|
|
*after* the backfill in run_crew. That matters: if the backfill itself raised,
|
|
`call_count == 1` would be guaranteed by the mock rather than by the code, and a
|
|
duplicate call inside run_crew could never be observed at all.
|
|
"""
|
|
with _in_a_project(runner):
|
|
Path("pyproject.toml").write_text(MINIMAL_PYPROJECT, encoding="utf-8")
|
|
with (
|
|
mock.patch("crewai_cli.cli.get_or_create_project_id") as in_cli,
|
|
mock.patch("crewai_cli.run_crew.get_or_create_project_id") as in_run_crew,
|
|
mock.patch(
|
|
"crewai_cli.run_crew.configured_project_json_crew",
|
|
side_effect=_StopAfterBackfill,
|
|
),
|
|
):
|
|
result = runner.invoke(crewai, ["flow", "kickoff"])
|
|
|
|
assert in_run_crew.call_count == 1, (
|
|
"flow kickoff must reach run_crew's backfill exactly once -- more than one "
|
|
"means it was duplicated somewhere on this path"
|
|
)
|
|
assert not in_cli.called, (
|
|
"flow kickoff must not add its own backfill call; it delegates to run_crew"
|
|
)
|
|
assert isinstance(result.exception, _StopAfterBackfill), (
|
|
"execution must have reached the boundary after run_crew's backfill. Without "
|
|
"this, both counts above would also pass if the path returned or raised "
|
|
"between the backfill and that boundary -- i.e. for the wrong reason"
|
|
)
|