test(resource): add failing tests for built-in fs-directory type bootstrap (#537) #567
Merged
brent.edwards
merged 2 commits from 2026-03-07 02:31:45 +00:00
feature/m3-test-resource-bootstrap-fs into master
Dismiss Review
Are you sure you want to dismiss this review?
Labels
Clear labels
auto/needs-reevaluation
controller-managed
overdue
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Needs Feedback
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
Controller deferred this PR; awaiting Phase 6+ scope-evaluator or operator re-enablement.
Auto-agents controller manages this PR/issue (see tools/controller/deploy/RUNBOOK.md). Remove this label to abandon controller management.
PR blocked by an open issue dependency. Operator must close the dep (or remove the dependency link) before the merge driver can act. Auto-cleared by merge_drive when no open deps remain.
Most recent merge cycle hit CI timeout. Driver excludes this PR while last merge_cycle row is < 30 min old; label persists thereafter as visible history.
Currently being processed by an implementer worker.
Currently being processed by the merge driver.
Currently being processed by a reviewer worker.
Merge driver heartbeat stale; pipeline halted. Closed automatically on next clean tick.
Detected master commit violating the strict merge invariant. Tracked as an issue (not a PR label); kept here for label completeness.
In-cycle escalation: most recent attempt ran at the Tier 0 slot (`tier-0`). Slot's model defined in .opencode/models/tiers.yaml.
In-cycle escalation: most recent attempt ran at the Tier 1 slot (`tier-1`). Slot's model defined in .opencode/models/tiers.yaml.
In-cycle escalation: most recent attempt ran at the Tier 2 slot (`tier-2`). Slot's model defined in .opencode/models/tiers.yaml. Gated behind IMPLEMENTER_ESCALATION_TIER2_ENABLED.
In-cycle escalation: most recent attempt ran at the Tier -1 slot (`tier-min`). Slot's model defined in .opencode/models/tiers.yaml. Suffix is ``-min`` (not ``--1``) so the Forgejo UI reads naturally.
Tracking issues used by the AI Automation system for agents to communicate and report.
Rebase conflict needs LLM conflict-resolver.
Failing CI needs implementer attention.
Documenting a driver incident or rollback.
Reviewer has APPROVED this PR and no later REQUEST_CHANGES is outstanding. The merge driver requires this label to even consider a PR for merging. Set by the reviewer worker on APPROVE; cleared on REQUEST_CHANGES.
Train repeatedly lost master-tempo races. Driver excludes via merge_cycle until cooldown elapses; label persists as visible history.
Revert PR backing out an invariant violation. Fast-tracked through the merge driver.
Sentinel PR duplicated from upstream into a personal fork by tools/duplicate_prs_to_fork.py for pipeline testing. Lives only in the fork; the canonical pipeline never sees it.
No implementer activity for N days. Flagged for human review. Auto-cleared on next push to head branch.
Repeatedly fails on current master (>= 3 ci-fail-on-rebased-sha releases in 12 h). Excluded from driver until human triage.
A ticket in a blocked state and unable to complete until some other task is completed first.
Bounty
$100
A bounty of $100 for any open-source contributor who provides a MR that solves this issue
Bounty
$1000
A bounty of $1000 for any open-source contributor who provides a MR that solves this issue
Bounty
$10000
A bounty of $10000 for any open-source contributor who provides a MR that solves this issue
Bounty
$20
A bounty of $20 for any open-source contributor who provides a MR that solves this issue
Bounty
$2000
A bounty of $2000 for any open-source contributor who provides a MR that solves this issue
Bounty
$250
A bounty of $250 for any open-source contributor who provides a MR that solves this issue
Bounty
$50
A bounty of $50 for any open-source contributor who provides a MR that solves this issue
Bounty
$500
A bounty of $500 for any open-source contributor who provides a MR that solves this issue
Bounty
$5000
A bounty of $5000 for any open-source contributor who provides a MR that solves this issue
Bounty
$750
A bounty of $750 for any open-source contributor who provides a MR that solves this issue
MoSCoW
Could have
Could have feature in order to satisfy the epic/legendary.
MoSCoW
Must have
Must have feature in order to satisfy the epic/legendary.
MoSCoW
Should have
Should have feature in order to satisfy the epic/legendary.
There are questions in the ticket that can not be completed until the project owner provides clarity.
Points
1
1 man-hours worth of work for an expert with no learning curve.
Points
13
13 man-hours worth of work for an expert with no learning curve.
Points
2
2 man-hours worth of work for an expert with no learning curve.
Points
21
21 man-hours worth of work for an expert with no learning curve.
Points
3
3 man-hours worth of work for an expert with no learning curve.
Points
34
34 man-hours worth of work for an expert with no learning curve.
Points
5
5 man-hours worth of work for an expert with no learning curve.
Points
55
55 man-hours worth of work for an expert with no learning curve.
Points
8
8 man-hours worth of work for an expert with no learning curve.
Points
88
88 man-hours worth of work for an expert with no learning curve.
Priority
Backlog
This ticket has backlogged priority and is not to be worked on yet
Priority
CI Blocker
Critical priority issue that blocks CI/CD pipeline and prevents PR merges
Priority
Critical
The priority is critical
Priority
High
The priority is high
Priority
Low
The priority is low
Priority
Medium
The priority is medium
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.
A ticket for learning a tool or technology that is needed to be able to do future planning and design.
State
Completed
The ticket has been fully implemented, completed, and merged with the source code. This label should only be applied once a ticket is closed.
State
Duplicate
A ticket that represents the same content as an existing ticket.
State
In Progress
A ticket that is actively being developed.
State
In Review
A ticket that has had some code completed to implement but is waiting to pass peer review and is not yet merged in.
State
Paused
This ticket's work started but wasn't finished. It's on hold (likely in a feature branch) and will be resumed later, either due to a blocker or a delay.
State
Unverified
All new tickets start in this state. A developer may set it to show the ticket is unverified. This means we haven't agreed to work on it. It will either move to a verified state or be closed as wontdo.
State
Verified
The issue has been verified by a developer as legitimate. It will be worked on and verified tickets are now considered part of the backlog.
State
Wont Do
This ticket has been decided it wont be done. This may mean the bug has been determined to not be real (cant verify) or the feature is one we have decided we dont want to adopt.
Type
Automation
Any edits or discussion about the AI automated coding system.
Type
Bug
Something that doesnt work as intended.
Type
Discussion
Anytime a ticket represents a discussion about a subject and doesnt fall into one of the other categories.
Type
Documentation
An error or improvement needed in the documentation.
Type
Epic
Any first tier epic. That is, an epic which contains only issues as children and will not have sub-epics.
Type
Feature
Some new functionality not present.
Type
Legendary
A type of Epic which will contain other Epics.
Type
Refactor
A code change that restructures existing code without changing its external behavior.
Type
Support
Someone needs help using the project.
Type
Task
A generic task that doesnt fit into the other type categories.
Type
Testing
Work exclusively focusing on fixing or expanding testing.
Projects
Clear projects
No project
Assignees
aditya (Aditya Chhabra)
aleenaumair (Aleena Umair)
brent.edwards (Brent Edwards)
CoreRasurae (Luis Mendes)
drew (Drew Morris)
eugen.thaci (Eugen Thaci)
freemo (Jeffrey Phillips Freeman)
HAL9000 (HAL 9000)
HAL9001 (HAL9001)
hamza.khyari (Hamza Khyari)
hurui200320 (Rui Hu)
justin.morris
khird (Kyle Hird)
org.cleveragents
Clear assignees
No Assignees
CoreRasurae
Notifications
Due Date
No due date set.
Blocks
#523 `agents resource add fs-directory` should not cause an error.
cleveragents/cleveragents-core
#537 test(resource): add failing tests for built-in fs-directory type bootstrap (#523)
cleveragents/cleveragents-core
Reference: cleveragents/cleveragents-core#567
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "feature/m3-test-resource-bootstrap-fs"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Add TDD-style Behave BDD tests for the built-in
fs-directoryresource type bootstrap (bug #523). Three Gherkin scenarios: one failing TDD test reproducing the bug (no bootstrap called during init, tagged@wip), and two regression tests verifyingbootstrap_builtin_types()seeds correct data andagents resource add fs-directorysucceeds after bootstrap. Includes Robot Framework regression tests.Files Changed
features/resource_type_bootstrap_fs.feature— 3 Gherkin scenarios (1@wipTDD, 2 regression)features/steps/resource_type_bootstrap_fs_steps.py— Step definitions with in-memory SQLite +ResourceRegistryServicerobot/resource_type_bootstrap_fs.robot— 2 Robot Framework regression testsCHANGELOG.md— Entry for test(resource): add failing tests for built-in fs-directory type bootstrap (#523) (#537)Review Feedback Addressed
bootstrap fsprefix (CoreRasurae F1)unittest.mock.patchcontext managers (CoreRasurae F3)image=Noneparameter (CoreRasurae F4)_capture_output(CoreRasurae F5)And the resource add output should contain "Added resource"to Scenario 3 (hamza.khyari R2R2-TEST-1)# type: ignorecomments (hurui200320 M1)is not Truetois Falsefor clarity (Aditya F2)common.resourceto${CURDIR}/common.resource(hurui200320 L1)Closes #537
Review: TDD failing tests for built-in
fs-directorytype bootstrap (bug #523)Solid TDD PR. The test approach of constructing an in-memory SQLAlchemy database and calling
bootstrap_builtin_types()+show_type()/register_resource()directly is a clean way to isolate the bootstrap behavior without needing the full CLI stack. The feature file is clear and the two scenarios cover the right surface area (registry existence + CLI-level add).Since this is TDD-style, CI failures are expected — acknowledged.
A few comments inline — one substantive note about error visibility in the capture helper, and a couple of minor observations about consistency with the companion PR #568.
@@ -0,0 +9,4 @@# ── Registry bootstrap ─────────────────────────────────────Scenario: After initialization fs-directory type exists in the registryMinor: The scenarios in PR #566 use
@tdd @bug522tags, which allow selective execution (e.g.,behave --tags=@tddto run only TDD tests, orbehave --tags=~@tddto skip them). These scenarios don't have equivalent tags.Consider adding
@tdd @bug523tags for consistency:@@ -0,0 +40,4 @@"""Monkey-patch the resource CLI module to use our in-memory service."""import cleveragents.cli.commands.resource as resource_modorig_fn = resource_mod._get_registry_serviceMinor: The manual monkey-patching approach (
_patch_service/_unpatch_service) works, butunittest.mock.patchwould be more robust and is the pattern used in PR #566's step definitions. With manual patching, if an exception is raised between_patch_service()and thefinallyblock calling_unpatch_service(), the module state could be left dirty.The current code does use
try/finallywhich mitigates this, so it's not broken — just noting the consistency difference with the companion PRs.@@ -0,0 +73,4 @@func(*args, **kwargs)except SystemExit:failed = Trueexcept Exception:Substantive:
_capture_outputcatches bothSystemExitandExceptionbut discards the actual exception. Whenfailed=True, the only diagnostic available is whatever was written to the console buffer before the error — the exception message/traceback itself is lost.This will make debugging harder for the #523 fix author when their first attempt doesn't quite work. Consider capturing the exception:
That way assertion messages in the Then steps will include the actual error.
@@ -0,0 +135,4 @@# ── Then steps ───────────────────────────────────────────────@then('the resource type "{name}" should exist')Note: The companion PR #568 (git-checkout bootstrap) defines similar steps but with different matcher text for the same domain concepts:
This PR:
the resource type "{name}" should existPR #568:
the resource type "{name}" should be present in the registryThis PR:
the resource type kind should be "{kind}"PR #568:
the resource type "{name}" should have kind "{kind}"No runtime conflict since the matchers are distinct, but when both merge the step vocabulary will be inconsistent for the same domain. If there's an opportunity to align them (either in this PR or #568), it would make the test suite more uniform. Low priority.
@@ -0,0 +39,4 @@[Documentation] Verify that after bootstrap, registering an fs-directory resource... instance succeeds (no "Resource type not found" error).${script}= Catenate SEPARATOR=\n... from sqlalchemy import create_engineNit: Both Robot test cases repeat the engine/session/service setup boilerplate (lines 17-24 and 42-50). Could extract a shared Robot keyword or a Python helper script to reduce duplication. Not blocking — the inline scripts are readable and consistent with other Robot tests in the project.
Disposition of self-review findings (review #1940)
Addressed in commit
939fb562.F1 —
_capture_outputswallows exceptions (comment #53276)Fixed.
_capture_output()now captures exception details into afailure_reasonstring and appends it to the output buffer. BothSystemExit(with exit code) and general exceptions (with type name and message) are preserved. This gives the #523 fix author immediate visibility into what went wrong without needing to add debug instrumentation.F2 — Manual monkey-patching vs
unittest.mock.patch(comment #53278)Acknowledged, no change. The current
try/finallypattern instep_run_resource_add_fsmitigates the dirty-state risk. The companion PR #568 usesunittest.mock.patch(the preferred pattern), so there's already a good reference. Refactoring this PR to usepatchwould touch multiple helpers (_patch_service,_unpatch_service,_capture_output) and is better suited for a follow-up that consolidates the shared testing patterns across #567 and #568.F3 — Missing
@tdd @bug523tags (comment #53280)Fixed. Added
@tdd @bug523tags to both scenarios inresource_type_bootstrap_fs.feature, matching the convention established in PR #566.F4 — Robot boilerplate duplication (comment #53284)
Acknowledged, no change. The inline Python scripts in both Robot test cases repeat the engine/session/service setup. This is consistent with how other Robot tests in the project are structured (self-contained inline scripts). Extracting a shared keyword or helper script would be a good improvement, but is better done in a follow-up that addresses the same pattern across #567 and #568 together.
F5 — Step vocabulary inconsistency with PR #568 (comment #53282)
Acknowledged, deferred. PR #568's parameterized patterns (
"{name}" should be present in the registry) are the better design. Aligning this PR to match would riskAmbiguousSteperrors if Behave loads both step files. Deferring to a follow-up where steps can be consolidated into a shared module.Code Review -- PR #567 (Issue #537)
Scope: 3 commits on
feature/m3-test-resource-bootstrap-fsby Brent E. Edwards (d90d95c2initial tests,91dfe043changelog,939fb562self-review fixes).Reviewed against: Forgejo issue #537 acceptance criteria,
docs/specification.md, codebase conventions.Summary Table
@thenstep pattern collides withresource_type_model_steps.py-- will causeAttributeErrorat runtimebootstrap_builtin_types()so they pass now; no test exercises theagents initpath -- contradicts "expected to fail" claimunittest.mock.patchpatternimage=Noneparameter inresource_add()call_BUILTIN_TYPESis incomplete vs specification (pre-existing, not introduced by this PR)F1 -- CRITICAL: Duplicate Behave Step Definition Causes Runtime Collision
features/steps/resource_type_bootstrap_fs_steps.py:163vsfeatures/steps/resource_type_model_steps.py:471The step
@then('the resource type sandbox_strategy should be "{strategy}"')is defined in both files with the identical Behave pattern string. Behave loads all step files fromsteps/into a single global registry -- the last one loaded (alphabetically) silently overwrites the first. This means:resource_type_bootstrap_fs_steps.pyloads first,resource_type_model_steps.pyoverwrites it, and the bootstrap feature scenarios get the wrong implementation (readingcontext.rt_specinstead ofcontext.bootstrap_fs_type_spec), causingAttributeError.resource_type_model.featurebreaks instead.The two implementations also diverge semantically -- one does string comparison, the other does
SandboxStrategy(strategy)enum comparison.Recommendation: Rename the step in this PR to a unique pattern, e.g.,
@then('the bootstrap fs resource type sandbox_strategy should be "{strategy}"'), and update the feature file scenario to match.F2 -- HIGH: Tests Do Not Reproduce the Actual Bug -- Acceptance Criteria Gap
Issue #537 acceptance criteria state: "At least one Behave scenario verifies that
fs-directorytype is available afteragents init." The issue body, the changelog, the PR description, and the commit messages all claim the tests are "expected to fail until the fix is applied" becausebootstrap_builtin_types()is never called during initialization.However, the tests explicitly call
service.bootstrap_builtin_types()themselves in the_make_bootstrapped_service()helper (line 34). The Robot test does the same (lines 25 and 50). Sincebootstrap_builtin_types()itself is implemented correctly (it iterates_BUILTIN_TYPESand inserts them), these tests should pass right now, not fail.The actual bug (#523) is that
agents initnever invokesbootstrap_builtin_types(). None of the tests exercise theagents initcode path.Recommendation: Either:
bootstrap_builtin_types()and asserts the type exists (this would genuinely fail until #523 is fixed), orF3 -- MEDIUM: Inconsistent Mocking Pattern vs Codebase Convention
The codebase standard for mocking the service layer (used in 90+ test files) is
unittest.mock.patch:This PR uses direct attribute assignment instead:
This is not thread-safe, lacks automatic rollback guarantees outside the
try/finally, and diverges from the established convention.Recommendation: Refactor to use
unittest.mock.patchas a context manager.F4 -- MEDIUM: Missing
imageParameter in CLI Function CallThe
resource_add()function signature atsrc/cleveragents/cli/commands/resource.py:424includes animagekeyword argument (image: str | None = None). The step definition call omits it:Python fills in the default
None, so this works today, but explicitly listing all parameters documents intent and guards against future signature changes.Recommendation: Add
image=Noneto the call.F5 -- LOW:
_capture_outputAppends Failure Reason Without SeparatorIf the console buffer already has content, the failure reason is concatenated directly with no newline, producing hard-to-read diagnostic output like:
Recommendation: Add a newline separator:
output = output + "\n" + failure_reason.F6 -- LOW: Robot Framework Tests Embed Large Inline Python Scripts
Both Robot test cases embed 15+ line Python scripts as inline
Catenatestrings with${SPACE}indentation variables. This is hard to debug (no line numbers in tracebacks), hard to maintain (no syntax highlighting/linting), and fragile.Recommendation: Extract to standalone
.pyfiles in arobot/scripts/directory and invoke viaRun Process ${PYTHON} ${script_path}.F7 -- INFO: Specification Lists
fs-mountas Built-in But_BUILTIN_TYPESOmits Itdocs/specification.md:9826says: "Built-in types (git-checkout,git,fs-mount,fs-directory, and their child types) are hardcoded." But_BUILTIN_TYPESonly has 5 of the 34 spec'd types. This is pre-existing incomplete implementation, not introduced by this PR. Worth noting for the broader #523 fix effort -- the test assertions validate against the current_BUILTIN_TYPESdefinition, not the full spec.Verdict: Requesting changes. F1 (duplicate step collision) is a blocking issue. F2 (acceptance criteria gap) should be discussed and resolved before merge.
@@ -0,0 +31,4 @@Base.metadata.create_all(engine)factory = sessionmaker(bind=engine, expire_on_commit=False)service = ResourceRegistryService(session_factory=factory)service.bootstrap_builtin_types()F2 -- HIGH: This explicit call means the tests pass now, contradicting the "expected to fail" claim.
The issue (#537), PR description, changelog, and commit messages all state these tests are expected to fail until bug #523 is fixed. But
bootstrap_builtin_types()itself works correctly -- the bug is thatagents initnever calls it. By manually calling it here, the tests will pass immediately.Consider adding a separate scenario that exercises the actual
agents initpath (without this manual call) to genuinely reproduce the bug.@@ -0,0 +45,4 @@def _mock_get() -> ResourceRegistryService:return context.bootstrap_fs_service # type: ignore[attr-defined]resource_mod._get_registry_service = _mock_get # type: ignore[attr-defined]F3 -- MEDIUM: Inconsistent mocking pattern.
The codebase convention (used in 90+ test files) is
unittest.mock.patch:Direct attribute assignment is not thread-safe and lacks the automatic rollback guarantees that
patch()provides. Consider refactoring to match the established pattern.@@ -0,0 +83,4 @@output = buf.getvalue()if failure_reason:output = output + failure_reasonF5 -- LOW: No separator between output and failure_reason.
If
buf.getvalue()has content, the failure reason is concatenated directly without a newline, producing hard-to-read diagnostics like:Consider:
output = output + "\n" + failure_reason@@ -0,0 +123,4 @@orig = _patch_service(context)try:output, failed = _capture_output(resource_add,F4 -- MEDIUM: Missing
imageparameter.The
resource_add()signature includesimage: str | None = Nonebut this call omits it. While Python fills the default, explicitly passingimage=Nonedocuments intent and guards against future signature changes.@@ -0,0 +160,4 @@assert actual_str == kind, f"Expected kind '{kind}', got '{actual_str}'"@then('the resource type sandbox_strategy should be "{strategy}"')F1 -- CRITICAL: Duplicate Behave step pattern.
This exact pattern
'the resource type sandbox_strategy should be "{strategy}"'is already defined inresource_type_model_steps.py:471. Behave uses a global step registry -- the last file loaded alphabetically wins, silently breaking whichever feature file gets the wrong implementation.The two implementations also diverge: this one does string comparison, the other does
SandboxStrategy(strategy)enum comparison.Fix: Rename to a unique pattern, e.g.
'the bootstrap fs resource type sandbox_strategy should be "{strategy}"'and update the.featurefile to match.@@ -0,0 +13,4 @@Bootstrap Seeds Fs Directory Type Into Registry[Documentation] Verify that bootstrap_builtin_types() seeds fs-directory into the... database so that show_type("fs-directory") succeeds.${script}= Catenate SEPARATOR=\nF6 -- LOW: Large inline Python scripts are fragile and hard to maintain.
These 15+ line Python scripts embedded as
Catenatestrings have no syntax highlighting, no linting, and produce unhelpful tracebacks (no line numbers). The${SPACE}indentation is also fragile.Consider extracting to standalone
.pyfiles inrobot/scripts/and invoking viaRun Process ${PYTHON} ${script_path}.Thanks @CoreRasurae for the thorough review — F1 and F2 in particular were important catches. All findings addressed in commit
0eb125c9.Disposition of findings (review #1946)
F1 CRITICAL — Duplicate Behave step pattern collision
Fixed. Renamed all three Then step patterns to include a
bootstrap fsprefix, eliminating the collision withresource_type_model_steps.py:471:the resource type "{name}" should existthe bootstrap fs resource type "{name}" should existthe resource type kind should be "{kind}"the bootstrap fs resource type kind should be "{kind}"the resource type sandbox_strategy should be "{strategy}"the bootstrap fs resource type sandbox_strategy should be "{strategy}"Feature file scenarios updated to match. The When step was also renamed to
I query the fs bootstrap resource type registry for "{name}"to avoid any future collision with generic registry query steps.F2 HIGH — Tests don't reproduce the actual bug
Fixed. Added a new genuinely-failing scenario:
This creates a registry WITHOUT calling
bootstrap_builtin_types(), then assertsfs-directoryexists. It fails now (reproducing bug #523) and will pass after the fix integrates bootstrap into init. The underlying_make_service()helper now takes arun_bootstrapboolean parameter.The existing two scenarios are retained as regression tests verifying that
bootstrap_builtin_types()itself correctly seeds the data when called. The feature file comments now accurately distinguish the bug reproduction scenario from the regression tests.F3 MEDIUM — Inconsistent mocking pattern
Fixed. Replaced the manual
_patch_service()/_unpatch_service()functions withunittest.mock.patchcontext managers:patch("cleveragents.cli.commands.resource._get_registry_service", return_value=service)patch("cleveragents.cli.commands.resource.console", console)Both
_patch_serviceand_unpatch_servicehelper functions are removed entirely. The_capture_outputfunction now usespatchfor the console swap as well.F4 MEDIUM — Missing
imageparameterFixed. Added
image=Noneto theresource_add()call instep_run_resource_add_fs, explicitly documenting the full parameter set.F5 LOW — No separator between output and failure reason
Fixed. Changed from
output + failure_reasontooutput + "\n" + failure_reason, so diagnostic output reads:F6 LOW — Inline Robot Python scripts
Acknowledged, deferred. The inline scripts are consistent with other Robot tests in the project (e.g.,
resource_type_bootstrap_git.robot). Extracting to standalone.pyfiles inrobot/scripts/is a good improvement but is better done as a follow-up that addresses the pattern across both #567 and #568 together.F7 INFO —
_BUILTIN_TYPESincomplete vs specificationAcknowledged. This is pre-existing — the current
_BUILTIN_TYPEShas 5 of the 34 spec'd types. The test assertions validate against the current implementation, not the full spec. Worth noting for the broader #523 fix effort.Additional
Added
@wiptag to all three scenarios for CI filtering, matching the convention established in PR #566 after CoreRasurae's review there.Verification
nox -s lint— passednox -s typecheck— passed (0 errors, 0 warnings, 0 informations)PM Review — Day 25
Status
939fb562and0eb125c9. However, no follow-up review has been submitted.Action Item
@CoreRasurae: Please re-review. The CRITICAL step collision was fixed and a genuinely-failing scenario was added. If the fixes are satisfactory, please submit APPROVED.
Notes
The following description for this task was removed. I no longer have the ability to edit the description, so I'm putting it here:
Summary
fs-directoryresource type is available after initialization (bug #523)agents init,fs-directoryshould exist in the registry andagents resource add fs-directoryshould succeedbootstrap_builtin_types()is never called during init)Files Added
features/resource_type_bootstrap_fs.featureresource addsuccessfeatures/steps/resource_type_bootstrap_fs_steps.pyResourceRegistryServicerobot/resource_type_bootstrap_fs.robotTDD Workflow
This PR intentionally introduces failing tests. The bug-fix author should branch from
feature/m3-test-resource-bootstrap-fsso the fix commit inherits these tests and turns them green.Lint / Typecheck
nox -s lint— passed (one import-sorting fix applied automatically)nox -s typecheck— passed (0 errors, 0 warnings)ISSUES CLOSED: #537
Code Review -- PR #567 (Issue #537) -- Reviewer 2
Scope: 4 commits on
feature/m3-test-resource-bootstrap-fs(d90d95c2initial tests,91dfe043changelog,939fb562self-review fixes,0eb125c9CoreRasurae review fixes).Reviewed against: Issue #537 acceptance criteria,
docs/specification.md,workflows/review.md(9-phase),PROTOCOL.md.Summary Table
ResourceRegistryService.__init__()@wiptag not filtered by CI -- merging breaksunit_testsnox sessionbootstrap_builtin_types()and passPriority/Mediumlabel -- belongs on issues only (PROTOCOL.md §21.5)# type: ignore[attr-defined]for Behave context attrsR2-BUG-1: Scenario 1 Cannot Turn Green Under the Expected Fix Path
Severity: Major
Category: BUG / TEST
File:
features/resource_type_bootstrap_fs.feature:13-16Spec Reference: Issue #523 subtask 1, Issue #537 acceptance criteria
Description:
Scenario 1 creates a
ResourceRegistryServicedirectly via_make_service(context, run_bootstrap=False)-- bypassing theagents initpath entirely. It then assertsfs-directoryshould exist.Issue #523 says the fix goes in "init_command, initialize_project(), or a post-init_database() hook". Since this test never calls any of those, the scenario stays red after the fix is merged. This violates #537 acceptance criteria: "Tests are structured so they will pass once the fix in #523 is merged."
Execution trace:
_make_service(run_bootstrap=False)->ResourceRegistryService(session_factory=factory)-- constructor has no bootstrapservice.show_type("fs-directory")-> empty DB ->NotFoundErrorcontext.bootstrap_fs_type_found = Falseis True-> FAIL -- now and after any fix outside__init__Recommendation:
Either (a) rewrite Scenario 1 to exercise the actual init path (
init_command/initialize_project()), (b) coordinate with the #523 author that the fix must live in__init__(), or (c) drop Scenario 1 and keep only the regression tests (Scenarios 2-3).Verification:
Confirm with the #523 fix author where
bootstrap_builtin_types()will be called and verify the test exercises that path.R2-BUG-2:
@wipTag Not Filtered by CISeverity: Major
Category: BUG / PROCESS
File:
features/resource_type_bootstrap_fs.feature:12,20,30+noxfile.py:455-488+behave.iniDescription:
All three scenarios carry
@wip, but nothing excludes them from CI:behave.ini-- no tag filteringnoxfile.pyunit_testssession -- no--tags=-wipnoxfile.pyunit_testssession -- nosuccess_codes=[0, 1](unlikecoverage_report)After merge, Scenario 1 fails ->
unit_testsgoes red -> CI pipeline breaks.Recommendation:
Either (a) add
--tags=-wipto the behave invocation innoxfile.pyunit_testsandcoverage_reportsessions, (b) merge these tests in the same PR as the fix so they never fail in CI, or (c) tag with@skipinstead of@wipif a Behave hook exists for that.Verification:
nox -s unit_testsafter merge -- will fail on Scenario 1.R2-PROCESS-1: PR Body Is Empty
Severity: Major
Category: PROCESS
Description:
PROTOCOL.md S14 requires PRs to include a detailed description, closing keyword (
Closes #537), same milestone, and aType/label. The PR body is completely empty. Commit messages are thorough but the PR description itself must contain this information.Recommendation:
Add a body with a summary and
Closes #537.R2-TEST-1: Robot Documentation Claims "Expected to FAIL" But Tests Pass
Severity: Minor
Category: TEST
File:
robot/resource_type_bootstrap_fs.robot:2-4Description:
Documentation says "These tests are expected to FAIL until bug #523 is fixed." Both Robot tests explicitly call
service.bootstrap_builtin_types()(lines 25 and 50) and pass now. Only Behave Scenario 1 is a genuinely failing TDD test.Recommendation:
Update Robot docs to say these are regression tests for the bootstrap function, not TDD failing tests.
R2-TEST-2: CHANGELOG Says "Two Scenarios" -- There Are Three
Severity: Minor
Category: TEST
File:
CHANGELOG.md:5Description:
"Two scenarios verify..." but the feature file has three scenarios.
Recommendation:
Update to "Three scenarios" or rephrase.
R2-PROCESS-2:
Priority/MediumLabel on PRSeverity: Minor
Category: PROCESS
Description:
Per PROTOCOL.md S21.5: "No MoSCoW/, Points/, Priority/, or enhancement labels on PRs -- those belong on issues only."
Recommendation:
Remove
Priority/Mediumfrom the PR.R2-CODE-1: 13x
type: ignore[attr-defined]Severity: Nit
Category: CODE
File:
features/steps/resource_type_bootstrap_fs_steps.pyDescription:
13 suppression comments for Behave's dynamic context. Somewhat unavoidable but noisy.
Recommendation:
Author's discretion.
CoreRasurae F1-F7 Fix Verification
unittest.mock.patchcontext managers now.imageimage=Noneexplicit at line 134.Mechanical Verification
ruff check(step file)pyright(step file)features/steps/)ISSUES CLOSED:footerVerdict: REQUEST_CHANGES -- R2-BUG-1 and R2-BUG-2 are blocking. R2-PROCESS-1 should be addressed before merge.
Thanks @hamza.khyari for the thorough review — all findings addressed in commit
a9cdafe7.Disposition of findings (review #1983)
R2-BUG-1 MAJOR — Scenario 1 will never turn green with current Given step
Acknowledged and documented. Added a
NOTE FOR FIX AUTHORcomment block to the feature file explaining that if the chosen fix is to callbootstrap_builtin_types()insideinit_command()/initialize_project()(rather than inResourceRegistryService.__init__()), the Given step will need updating to exercise the init path instead of constructing a bare service.This is intentional TDD design: we write the assertion for the expected behavior, and the fix author adjusts the setup to match their implementation approach. The
@wiptag on Scenario 1 ensures it's explicitly flagged as the failing test.R2-BUG-2 MAJOR —
@wipnot filtered by CI; Scenarios 2-3 passFixed. Removed
@wipfrom Scenarios 2 and 3 (regression tests that already pass). Kept@wiponly on Scenario 1 (the genuine TDD failing test).Regarding CI filtering: you're correct that
noxfile.py:454-488runsbehave-parallelwithout--tags=-wipandbehave.inihas no tag filter. Per the TDD workflow, thefeature/m3-test-resource-bootstrap-fsbranch stays open (never merges to master directly). The fix author branches from this branch, makes Scenario 1 pass, removes@wip, and their fix PR merges everything to master. So@wipscenarios should never reach master in a failing state. That said, adding--tags=-wipto the nox session would be a good safety net — worth a follow-up issue.R2-PROCESS-1 MAJOR — PR body is empty
Fixed. Restored via
forgejo_update_pull_requestAPI. The body was cleared by a Forgejo API quirk when setting the milestone (passingmilestonewithoutbodycauses the API to clear the body field). Updated description now reflects three scenarios and distinguishes the TDD failing test from the regression tests.R2-TEST-1 MINOR — Robot docs say "expected to FAIL"
Fixed. Updated Robot
Documentationfrom "expected to FAIL until bug #523 is fixed" to "Regression tests for built-in fs-directory type bootstrap (bug #523)." Both Robot tests callbootstrap_builtin_types()explicitly and pass — they are regression tests, not TDD failing tests.R2-TEST-2 MINOR — CHANGELOG says "Two scenarios"
Fixed. Updated to "Three scenarios: one failing TDD test reproducing bug #523 (no bootstrap called during init), and two regression tests verifying
bootstrap_builtin_types()seeds correct data andagents resource add fs-directorysucceeds."R2-PROCESS-2 MINOR — Priority/Medium label on PR
Fixed. Removed
Priority/Mediumlabel (ID 860) from PR per PROTOCOL.md §21.5 (labels belong on issues, not PRs, except State/ and Type/).R2-CODE-1 NIT — 13×
type: ignore[attr-defined]Acknowledged, retained. Behave's
Contextobject is dynamically typed — attributes are set viacontext.foo = barin Given steps and accessed in When/Then steps. Pyright correctly reports these as unknown attributes sinceContexthas no static type stubs. The# type: ignore[attr-defined]suppression is the established convention across all Behave step files in this project and is listed as acceptable in the project's type-checking guidelines.Verification
nox -s lint— passed (All checks passed!)nox -s typecheck— passed (0 errors, 0 warnings, 0 informations)Round 2 -- Outstanding Item
All Round 1 findings are resolved. One new blocking finding from Round 2:
R2R2-TEST-1: Scenario 3 Needs a Positive Assertion
Scenario 3 only asserts absence of failure and absence of error text. A no-op
resource_add(prints nothing, creates nothing) passes both checks. The actual success output is"Added resource: local/test (id: ...)"but the test never verifies it.Fix: Add one step to Scenario 3:
With step definition:
Once this is in, I'll approve.
Disposition: hamza.khyari Round 2 Review (review #1987)
All findings from the Round 2 review have been addressed. Changes verified with
nox -s lint(all checks passed) andnox -s typecheck(0 errors).Finding Dispositions
And the resource add output should contain "Added resource"to Scenario 3 in the feature file, plus the correspondingstep_fs_output_should_containstep definition. Scenario 3 now has both positive and negative assertions.step_type_should_existandstep_resource_add_should_succeed. Messages now describe the failure generically — the bug context lives in the module docstring and feature-file comments._capture_outputto treatSystemExit(0)andSystemExit(None)as success. Only non-zero exit codes now setfailed = True. The broadexcept Exceptionis retained intentionally to ensure CLI failures are captured as test output rather than crashing the test runner.Additional fix (shared with PR #568)
tags = ~@wiptobehave.iniso@wipscenarios are excluded fromnox -s unit_testsby default. This prevents the TDD failing test (Scenario 1) from breaking the CI gate.Commits pushed
0c31636—fix(test): add tags = ~@wip to behave.ini to exclude @wip scenarios16c3871—fix(test): add positive assertion to Scenario 3 (R2R2-TEST-1)0ac972c—fix(test): fix step definitions for Round 2 review findingsRound 3 Review
Scope: 3 new commits since Round 2 (
0c316364,16c38718,0ac972c0). All prior findings from R1/R2 have been resolved.Prior findings: resolved
"Added resource"assertion added. Verified by running the actual code path:resource_addprints"Added resource: local/test (id: ...)"on success, and the assertion correctly matches._capture_outputSystemExit(0)misclassification — Fixed. Now treatsSystemExitwith codeNoneor0as success.behave.initags = ~@wipadded to exclude the TDD failing scenario from CI — Sound. No@wiptags exist on the main branch, so no existing tests are affected.DESIGN-1 (low, non-blocking) —
_capture_outputSystemExithandler is dead code for current usageFile:
features/steps/resource_type_bootstrap_fs_steps.py:65-68The
except SystemExitbranch in_capture_outputwill never execute in the current test setup.resource_addis called directly as a Python function (not through the Typer CLI runner), so on error it raisestyper.Abort()which inherits fromRuntimeError -> Exception, notSystemExit. TheAbortis caught by theexcept Exceptionclause at line 69 instead.The code is correct (defensive programming), but the
SystemExit(0)fix from commit0ac972c0addresses a scenario that cannot currently occur. Consider adding a brief inline comment explaining why both handlers exist.DESIGN-2 (low, non-blocking) —
typer.Abort()failure reason string is emptyFile:
features/steps/resource_type_bootstrap_fs_steps.py:71When
resource_addfails (e.g., type not found), it raisestyper.Abort(). Theexcept Exceptionhandler formats this asf"{type(exc).__name__}: {exc}"which produces"Abort: ".typer.Abort()takes no message argument, sostr(exc)is empty. The actual error text ("Resource type not found: fs-directory") is captured in the console output buffer, so diagnostics still work — but thefailure_reasonstring is uninformative. Consider special-casingAbortor relying solely on console output for the error message.TEST-1 (medium) — Scenario 1 (@wip TDD) title contradicts its assertion
File:
features/resource_type_bootstrap_fs.feature:20-24Scenario 1 is titled "fs-directory type is missing when bootstrap is not called during init" but the Then step asserts
the bootstrap fs resource type "fs-directory" **should exist**. The title describes the current buggy state; the assertion describes the desired fixed state. This is the standard TDD pattern (write the test you want to pass), but the title is misleading for anyone reading the feature file. Consider renaming to clarify that the assertion represents the fix-target, e.g. "fs-directory type should exist after init calls bootstrap" or adding a Gherkin comment on the Then line.SCOPE-1 (info, non-blocking) —
behave.inichange has global scopeFile:
behave.ini:3Adding
tags = ~@wipis a project-wide config change: any future@wip-tagged scenarios across the entire codebase will be automatically excluded from allbehave/behave-parallelruns. This is likely intentional, but should be documented (PR description or inline comment) so other contributors understand the convention.Disposition — Review #2002, Round 3 (hamza.khyari)
All 4 findings addressed across 3 commits. Quality gates pass locally:
nox -s lint✅,nox -s typecheck✅,nox -s unit_tests(feature file, EXIT 0) ✅.TEST-1: Scenario 1 title contradicts assertion (Medium) — FIXED ✅
Renamed from "fs-directory type is missing when bootstrap is not called during init" to "fs-directory type exists after init without explicit bootstrap call". The new title describes the desired fix-target behavior, aligning with the TDD assertion.
Commit:
0eeac1e7DESIGN-1:
_capture_outputSystemExit handler is dead code (Low) — DOCUMENTED ✅Added 5-line inline comment above the
except SystemExitbranch explaining it is a defensive guard for future Typer CLI runner usage, where exceptions are converted toSystemExit.Commit:
469b3fa4DESIGN-2:
typer.Abort()failure reason string is empty (Low) — FIXED ✅Added fallback logic: when
str(exc)is empty (as withtyper.Abort()), the failure reason now shows"(no message)"instead of a blank string. Added inline comment explaining why.Commit:
469b3fa4SCOPE-1:
behave.iniglobal scope undocumented (Info) — DOCUMENTED ✅Added 2-line comment above
tags = ~@wipexplaining the convention:@wip-tagged scenarios are excluded project-wide so TDD failing tests do not break CI.Commit:
8c9bb83fReady for re-review.
Code Review — PR #567 (Issue #537)
Reviewer: Rui Hu (@hurui200320)
Summary Table
# type: ignorecomments (file not type-checked)common.resourcepath in Robot fileC1 — CRITICAL (Blocking): Multiple commits and merge commits on branch
The branch has 14 commits: 11 non-merge + 3 merge commits. Per CONTRIBUTING.md:
Commit history:
Recommendation: Rebase the branch onto current
masterand squash all commits into a single commit with the exact first line from issue #537 Metadata:test(resource): add failing tests for built-in fs-directory type bootstrap.C2 — CRITICAL (Blocking): PR is not mergeable
Master has 11 new commits since the merge base (
d990fc1b). The branch has conflicts (likely inCHANGELOG.md). The rebase from C1 will also resolve this.Recommendation: After squashing per C1, rebase onto current
origin/master.H1 — HIGH: Confusing issue references; missing closing keyword
The PR body ends with
Refs: #537, but CONTRIBUTING.md requires a closing keyword (e.g.,Closes #537). Additionally, the PR title references(#523)— which is the bug issue, not the issue this PR implements. This PR implements #537 (the test issue that blocks #523). The mix of#523in the title and#537in the body without a closing keyword makes the issue relationship unclear.Recommendation: Use
Closes #537as the closing keyword per CONTRIBUTING.md. If a reference to bug #523 is desired for context, include it as a non-closing reference (e.g.,Refs: #523), clearly separated from the closing keyword. Consider also updating the PR title to reference #537 as the implemented issue.M1 — MEDIUM: Unnecessary
# type: ignorecomments (21 instances)File:
features/steps/resource_type_bootstrap_fs_steps.py(lines 19-20, 49, 94-95, 106-107, 116, 118-119, 121-122, 134, 147-148, 157, 166, 175, 186, 195, 204)Pyright is configured with
"include": ["src"](inpyrightconfig.json). Thefeatures/directory is not type-checked. All 21# type: ignorecomments (2×import-untyped, 19×attr-defined) have zero effect — they suppress warnings that Pyright will never produce for these files.Recommendation: Remove all
# type: ignorecomments from this file. They add noise without any benefit since the file is outside Pyright's analysis scope.M2 — MEDIUM: Robot tests are unit-level, not integration-level
File:
robot/resource_type_bootstrap_fs.robot(lines 14-63)Both Robot tests run inline Python scripts that directly instantiate
ResourceRegistryServicewith in-memory SQLite. Per CONTRIBUTING.md, Robot Framework tests underrobot/should be integration-level. These don't exercise the real CLI entry point (agents resource add), the real database, or the DI container.Recommendation: Since these tests are effectively unit-level, consider moving them to the
features/folder where unit tests belong. If they remain inrobot/, they should be refactored to exercise the actual CLI entry point when the #523 bug fix lands.L1 — LOW: Bare
common.resourcepath in Robot fileFile:
robot/resource_type_bootstrap_fs.robot(line 8)Uses
Resource common.resourceinstead of the project conventionResource ${CURDIR}/common.resource(used by 156 of 161 Robot files).Recommendation: Change to
Resource ${CURDIR}/common.resourcefor consistency.L2 — LOW: Scenario 1 (@wip) may not turn green after #523 fix
File:
features/resource_type_bootstrap_fs.feature(lines 20-24)If the fix for #523 places
bootstrap_builtin_types()ininit_command()/initialize_project()rather thanResourceRegistryService.__init__(), Scenario 1's Given step (a fresh in-memory resource registry without bootstrap) won't exercise the fix path. The "NOTE FOR FIX AUTHOR" comment at lines 15-18 documents this, which is adequate.Recommendation: Non-blocking. Already documented. The fix author will adjust.
Verdict: REQUEST CHANGES
C1 and C2 are blocking. The branch must be rebased onto current master and squashed into a single commit. H1 (closing keyword) should also be fixed.
The test code itself is well-structured. Step patterns are properly namespaced (no collisions found across 150+ step files). The
@wipmechanism for TDD is sound. Scenarios 2-3 properly verify bootstrap functionality with both positive and negative assertions.Code Review Report: Brent's Commits on feature/m3-test-resource-bootstrap-fs
Latest Commit Reviewed: 8c9bb83f9d
Author: Brent E. Edwards (brent.edwards@cleverthis.com)
Message: docs(config): document global @wip exclusion in behave.ini
Issue: #537 — test(resource): add failing tests for built-in fs-directory type bootstrap (#523)
Branch: feature/m3-test-resource-bootstrap-fs
Reviewer: Aditya
Scope of Changes
CHANGELOG.mdbehave.inifeatures/resource_type_bootstrap_fs.featurefeatures/steps/resource_type_bootstrap_fs_steps.pyrobot/resource_type_bootstrap_fs.robotFindings
F1. [MEDIUM]
behave.initags = ~@wipsilently breaksbehave --tags=@wipfor local developmentFile:
behave.ini(line 5)Behave combines tags from the config file and the CLI using logical AND — it does not
replace them. With
tags = ~@wipinbehave.ini, any developer who tries to run the TDDfailing scenario locally with the natural command:
gets the combined filter
~@wip AND @wip, which matches zero scenarios — the run reports"0 features passed, 0 failed, 0 skipped" with no error or explanation. The developer has no
indication that the ini setting is silently suppressing their tag filter.
The comment added in commit
8c9bb83fdocuments what the setting does, but does not warnabout this interaction. The correct way to exercise the
@wipscenario locally after thischange is to target it by file and line number:
Neither this workaround nor the root cause is documented anywhere in the repo.
Hamza's SCOPE-1 finding (now closed) addressed the need to document the global scope of the
change. This is a distinct concern — the concrete developer workflow consequence that
--tags=@wipno longer works — and was not previously raised by any reviewer.
Recommendation: Extend the
behave.inicomment to document the workaround explicitly:F2. [LOW]
step_resource_add_should_succeeduses impreciseis not Trueinstead ofis FalseFile:
features/steps/resource_type_bootstrap_fs_steps.py(line 186)is not Truepasses for any non-Truevalue — includingNone,0, or an empty string —not just
False. Whilecontext.bootstrap_fs_failedis always assigned a concreteboolinthis file, the looser check is inconsistent with every other boolean assertion in the codebase,
all of which use
is Falseorassert not ...:Recommendation: Replace with the explicit, idiomatic form:
Summary Table
behave.initags = ~@wipsilently breaksbehave --tags=@wip; workaround undocumentedis not Truein step assertion should beis False— inconsistent with codebase pattern8c9bb83f9dtodc1ecaab47test(resource): add failing tests for built-in fs-directory type bootstrap (#523)to test(resource): add failing tests for built-in fs-directory type bootstrap (#537)Review Feedback Addressed
All review comments from @CoreRasurae (#1946), @hamza.khyari (#1983, #1987, #2002), and @hurui200320 (#2004) have been addressed. The branch has been squashed into a single commit and rebased onto master per CONTRIBUTING.md.
Changes made:
Process (hurui200320 C1, C2, H1):
Closes #537Code quality (hurui200320 M1):
# type: ignorecomments —features/is outside Pyright scope perpyrightconfig.jsonTest fixes (Aditya F2):
is not Truetois Falsefor clarityRobot consistency (hurui200320 L1):
Resource common.resourcetoResource ${CURDIR}/common.resourceper project conventionPreviously resolved in earlier rounds:
bootstrap fsprefix (CoreRasurae F1)unittest.mock.patch(CoreRasurae F3)image=Noneparameter (CoreRasurae F4)_capture_output(CoreRasurae F5)"Added resource"assertion to Scenario 3 (hamza.khyari R2R2-TEST-1)@wipglobal exclusion in behave.ini (hamza.khyari SCOPE-1)Ready for final review and merge.
PM Note (Day 26 — M3 PR Triage):
This PR has had ~7 review rounds. All findings appear to have been addressed but the latest review is stale (REQUEST_CHANGES, but from before the squash). Has a merge conflict.
@brent.edwards — Please rebase onto current
master. After rebase, we'll need a fresh approval from one reviewer. All prior findings have been resolved.Unified TDD Test Toggle Convention
After auditing all 7 PRs that add failing BDD tests (#566, #567, #568, #591, #593, #594, #596), we are standardizing on a single
@wiptag mechanism.The standard
.feature).robot)@wiptag above the Scenario[Tags] wipto the test case@wiptagwipfrom[Tags]behave features/foo.feature:42(by line number)robot --include wip robot/foo.robotThe infrastructure (
tags = ~@wipinbehave.ini+--exclude wipinnoxfile.py) is established by PR #566. This PR must be merged after #566.Status of this PR
Behave: Correct. Scenario 1 (
@wip) is the TDD-failing test. Scenarios 2–3 have no@wipand pass because they callbootstrap_builtin_types()explicitly.Robot: Correct. Both Robot tests call bootstrap explicitly and pass — no
[Tags] wipneeded.Dependency: This PR does NOT modify
behave.iniornoxfile.py. It relies entirely on PR #566 being merged first. If #566 is not merged, the@wiptag on Scenario 1 is inert and that scenario will fail in CI.Required action
Either:
behave.inichanges from #566 to this PR to make it self-contained.Merge conflict resolved
Merged master (
01ac466c) into the branch at commitee59f8c2. The only conflict was inCHANGELOG.md(our#537entry vs. master's#536,#574,#588entries under## Unreleased) — resolved by keeping both, with our entry first.Quality gates verified locally:
nox -s lintnox -s typechecknox -s unit_testsPR is now
mergeable: trueand ready to merge.