Add 'fiscal/' from commit '102ba84500dbd9655a0471288c3e278652593ca9'
git-subtree-dir: fiscal git-subtree-mainline:d95165fc8fgit-subtree-split:102ba84500
This commit is contained in:
gti
@@ -0,0 +1,139 @@
|
||||
# Task 1 Report
|
||||
|
||||
## Status
|
||||
|
||||
Implemented the read-only discovery foundation. The FDB database was not opened,
|
||||
inspected, or modified.
|
||||
|
||||
## Files Changed
|
||||
|
||||
- `src/gti_discovery/__init__.py`: exports the read-only public interface.
|
||||
- `src/gti_discovery/read_only.py`: validates one `SELECT`/`WITH` statement and
|
||||
exposes `ReadOnlyConnection.execute` without connection write methods.
|
||||
- `tests/test_read_only.py`: covers accepted queries, normalization, prohibited
|
||||
statements, comments, multiple statements, quoted literals, and delegation.
|
||||
- `pyproject.toml`: defines the package and `src`-layout pytest configuration.
|
||||
- `.env.example`: documents an empty database-path placeholder without secrets.
|
||||
- `CLAUDE.md`: records the strict read-only project constraints and test command.
|
||||
|
||||
## Tests
|
||||
|
||||
Command:
|
||||
|
||||
```text
|
||||
python -m pytest tests/test_read_only.py -q
|
||||
```
|
||||
|
||||
Result: `18 passed in 0.04s`.
|
||||
|
||||
`git diff --check` also completed without whitespace errors.
|
||||
|
||||
## Concerns
|
||||
|
||||
- Commit was not created because Git has no configured author identity (`Author
|
||||
identity unknown`). Git configuration was not changed.
|
||||
- Pytest emits an existing `pytest-asyncio` deprecation warning because its event
|
||||
loop fixture scope is not configured; the focused tests still pass.
|
||||
- The validator intentionally rejects all SQL comments and any prohibited keyword
|
||||
outside quoted values. It does not attempt to parse the full Firebird grammar.
|
||||
- The wrapper expects the injected driver connection to provide `execute(sql,
|
||||
params)`; no Firebird driver is installed or required for this offline task.
|
||||
|
||||
## Reviewer Fixes
|
||||
|
||||
- Replaced the retained raw driver connection with a narrow execute-only
|
||||
delegate and added coverage proving `_connection`, `commit`, and `rollback`
|
||||
are unavailable on `ReadOnlyConnection`.
|
||||
- Added rejection for selectable procedure calls, `GEN_ID`, generator value
|
||||
expressions, `WITH LOCK`, and `FOR UPDATE`. The wrapper cannot enforce the
|
||||
driver's transaction/read-only mode, so deployment must use a read-only
|
||||
database account or driver-level setting.
|
||||
- Replaced global `split`/`join` normalization with whitespace collapsing only
|
||||
outside quoted literals; quoted contents are preserved byte-for-byte.
|
||||
- Added regression tests for side-effecting SELECT constructs and literal
|
||||
preservation.
|
||||
|
||||
## Fix Verification
|
||||
|
||||
Exact command:
|
||||
|
||||
```text
|
||||
python -m pytest tests -q
|
||||
```
|
||||
|
||||
Exact output:
|
||||
|
||||
```text
|
||||
C:\Users\Gustavo\AppData\Local\Programs\Python\Python312\Lib\site-packages\pytest_asyncio\plugin.py:299: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.
|
||||
The event loop scope for asynchronous fixtures will default to the "fixture" caching scope. Future versions of pytest-asyncio will default the fixture scope to "function". Set the default fixture loop scope explicitly to avoid unexpected behavior.
|
||||
|
||||
warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET))
|
||||
....................... [100%]
|
||||
23 passed in 0.06s
|
||||
```
|
||||
|
||||
## Re-review Fixes
|
||||
|
||||
- Replaced the instance-bound driver delegate with an opaque-token lookup in a
|
||||
module-private registry. `ReadOnlyConnection` stores only the token; its
|
||||
bound `execute` method has the wrapper as `__self__` and no closure.
|
||||
- Added a regression test that checks `__self__`, `__closure__`, wrapper slots,
|
||||
and reachable slot values cannot expose the raw driver or `commit`.
|
||||
- Added conservative rejection of unknown and qualified function calls, while
|
||||
allowing a small deterministic-function allowlist. This covers selectable
|
||||
procedures, `GEN_ID`, UDF/external-function style calls, generator values,
|
||||
locking, and update forms. True read-only enforcement still requires a
|
||||
database read-only account or read-only transaction/driver configuration.
|
||||
|
||||
## Re-review Verification
|
||||
|
||||
Exact command:
|
||||
|
||||
```text
|
||||
python -m pytest tests -q
|
||||
```
|
||||
|
||||
Exact output:
|
||||
|
||||
```text
|
||||
C:\Users\Gustavo\AppData\Local\Programs\Python\Python312\Lib\site-packages\pytest_asyncio\plugin.py:299: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.
|
||||
The event loop scope for asynchronous fixtures will default to the "fixture" caching scope. Future versions of pytest-asyncio will default the fixture scope to "function". Set the default fixture loop scope explicitly to avoid unexpected behavior.
|
||||
|
||||
warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET))
|
||||
.......................... [100%]
|
||||
26 passed in 0.07s
|
||||
```
|
||||
|
||||
## Third Fix Round
|
||||
|
||||
- Removed the module-level bound-method registry. `ReadOnlyConnection` now
|
||||
accepts only a narrow adapter-supplied execute callable and exposes no driver
|
||||
or connection attributes. Python process-level introspection is explicitly
|
||||
outside this guarantee.
|
||||
- Rejected all function-like SQL calls except a small deterministic-function
|
||||
allowlist, including qualified names with arbitrary whitespace around dots.
|
||||
Selectable procedures in `FROM`, generator calls, `GEN_ID`, `EXECUTE`, UDF,
|
||||
and external-function-style calls are rejected conservatively.
|
||||
- Documented that actual read-only enforcement requires a Firebird read-only
|
||||
account or read-only transaction/driver configuration.
|
||||
- Added regression coverage for spaced qualified calls and the practical public
|
||||
API boundary.
|
||||
|
||||
## Third Fix Verification
|
||||
|
||||
Exact command:
|
||||
|
||||
```text
|
||||
python -m pytest tests -q
|
||||
```
|
||||
|
||||
Exact output:
|
||||
|
||||
```text
|
||||
C:\Users\Gustavo\AppData\Local\Programs\Python\Python312\Lib\site-packages\pytest_asyncio\plugin.py:299: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.
|
||||
The event loop scope for asynchronous fixtures will default to the "fixture" caching scope. Future versions of pytest-asyncio will default the fixture scope to "function". Set the default fixture loop scope explicitly to avoid unexpected behavior.
|
||||
|
||||
warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET))
|
||||
................................... [100%]
|
||||
35 passed in 0.09s
|
||||
```
|
||||
+21
@@ -0,0 +1,21 @@
|
||||
# Final Fix Report — Control Panel Operational UX V1
|
||||
|
||||
Base: `c237820`
|
||||
|
||||
## Changes
|
||||
|
||||
- Legacy blockers without an exact installation binding remain fail-closed. Their detail page explains in Portuguese why resolution is unavailable and omits the resolution form. Direct HTML and JSON attempts return HTTP 409; missing blockers remain 404. Legacy details do not load context by guessing a company identity.
|
||||
- Resolution rejects derived `company_key` values longer than the database limit of 64 characters before creating context, job, or audit rows. No truncation or identity aliasing is performed.
|
||||
- Validation-error re-renders preserve the submitted key, value, scope, scope key, and reason, and reload current persisted context for a bound blocker.
|
||||
- Added regression coverage in `central/tests/test_blocker_resolution_panel.py` and `central/tests/test_control_panel_pages.py`.
|
||||
|
||||
## Verification
|
||||
|
||||
- TDD RED: focused tests initially failed on the missing company-key validation, unavailable-blocker response/UI, and form/context recovery behavior.
|
||||
- Focused: `python -m pytest central/tests/test_blocker_resolution_panel.py central/tests/test_control_panel_pages.py -q` — **26 passed**.
|
||||
- Full central suite: `python -m pytest central/tests -q` — **284 passed, 1 warning** (existing `openpyxl` print-area warning in `test_bootstrap_v1_extra.py`).
|
||||
- `git diff --check` — passed.
|
||||
|
||||
## Concerns
|
||||
|
||||
- Test output includes the existing `pytest-asyncio` configuration deprecation notice and one unrelated `openpyxl` warning; neither caused test failures.
|
||||
@@ -0,0 +1,101 @@
|
||||
# Task 1 Report: Presentation Helpers And Password Policy
|
||||
|
||||
## Status
|
||||
|
||||
Implemented Task 1 only. The existing server-rendered architecture and fiscal behavior were not changed.
|
||||
|
||||
## Changed Files
|
||||
|
||||
- `central/app/panel_presentation.py`: added timezone-aware UI datetime, age, duration, operational priority, and health-label helpers.
|
||||
- `central/app/config.py`: added `ui_timezone` (default `America/Bahia`) and `allow_weak_dev_password` settings.
|
||||
- `central/app/human_auth.py`: restored the 12-character minimum unless the process is non-production and `ALLOW_WEAK_DEV_PASSWORD=1`; the flag is read at call time.
|
||||
- `central/tests/test_panel_presentation.py`: added helper contract tests.
|
||||
- `central/tests/test_control_plane.py`: added explicit opt-in and production password-policy tests.
|
||||
- `docker-compose.yml`: explicitly enables weak development passwords for the local `fiscal-hub` service.
|
||||
|
||||
## RED Evidence
|
||||
|
||||
Command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_panel_presentation.py central/tests/test_control_plane.py -q
|
||||
```
|
||||
|
||||
Result: collection failed with `ModuleNotFoundError: No module named 'central.app.panel_presentation'`, proving the new helper module was absent.
|
||||
|
||||
The control-plane tests were then run independently:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_control_plane.py -q
|
||||
```
|
||||
|
||||
Result: `2 failed, 3 passed`. Both new password-policy tests failed because the pre-change implementation accepted short passwords without the required environment gate.
|
||||
|
||||
## GREEN Evidence
|
||||
|
||||
Focused command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_panel_presentation.py central/tests/test_control_plane.py -q
|
||||
```
|
||||
|
||||
Result: `10 passed`.
|
||||
|
||||
Focused command including configuration coverage:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_panel_presentation.py central/tests/test_control_plane.py central/tests/test_config.py -q
|
||||
```
|
||||
|
||||
Result: `11 passed`.
|
||||
|
||||
Full central regression:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests -q
|
||||
```
|
||||
|
||||
Result: `241 passed, 1 warning`. The warning is the existing `openpyxl` print-area warning from `central/tests/test_bootstrap_v1_extra.py`.
|
||||
|
||||
`git diff --check` and the staged diff check completed without whitespace errors.
|
||||
|
||||
## Commit
|
||||
|
||||
Implementation commit: `0846b9d1b076c60c57ad2a30a3071480a680bccf` (`feat: add operational panel presentation helpers`).
|
||||
|
||||
## Concerns
|
||||
|
||||
- The existing pytest environment emits one non-failing `pytest-asyncio` deprecation warning; the full central suite also emits the pre-existing `openpyxl` warning noted above.
|
||||
- `operational_priority` intentionally uses only operational blocker type, field, and reason signals; it does not infer fiscal severity.
|
||||
|
||||
## Reviewed Finding Fix
|
||||
|
||||
Finding: `create_admin` used the import-time `settings.allow_weak_dev_password` value as the default for the call-time environment lookup. If the flag was enabled during import and later removed, short passwords remained accepted.
|
||||
|
||||
Fix: `ALLOW_WEAK_DEV_PASSWORD` now defaults to `"0"` at every call. The existing `APP_ENV != production` check remains enforced, and Docker Compose still explicitly sets `ALLOW_WEAK_DEV_PASSWORD: "1"` for the local `fiscal-hub` service.
|
||||
|
||||
Regression RED command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_control_plane.py::test_removed_weak_password_flag_is_fail_closed_after_import -q
|
||||
```
|
||||
|
||||
Result before the fix: `1 failed`; after reloading configuration/auth with the flag enabled, removing the flag did not raise `ValueError` for a short password.
|
||||
|
||||
Regression GREEN command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_panel_presentation.py central/tests/test_control_plane.py -q
|
||||
```
|
||||
|
||||
Result: `11 passed`.
|
||||
|
||||
Full central regression:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests -q
|
||||
```
|
||||
|
||||
Result: `242 passed, 1 warning`; the warning is the existing `openpyxl` print-area warning.
|
||||
|
||||
Fix commit: `de571e3ac2ce214677e1f9baa673181350b7d411` (`fix: fail closed weak password opt-in`).
|
||||
@@ -0,0 +1,193 @@
|
||||
# Task 2 Report: Bounded Operational Query View-Models
|
||||
|
||||
## Status
|
||||
|
||||
Implemented Task 2 only. The control-plane query layer now supplies bounded operational view-models while preserving the existing JSON keys and technical health derivation. No fiscal logic, database schema, Firebird access, or remote write capability was changed.
|
||||
|
||||
## RED Evidence
|
||||
|
||||
Added `central/tests/test_operational_view_models.py` before changing production code.
|
||||
|
||||
Command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_operational_view_models.py -q
|
||||
```
|
||||
|
||||
Result before implementation: `6 failed`.
|
||||
|
||||
The failures were the expected missing-contract failures: `dashboard_summary` did not accept `now`, `list_installations` did not accept `sort`, `list_jobs` did not accept `client`, `list_blockers` did not accept `client`, and `blocker_detail` did not exist. This confirmed the tests exercised the new behavior rather than existing behavior.
|
||||
|
||||
## GREEN Evidence
|
||||
|
||||
Focused Task 2 tests:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_operational_view_models.py -q
|
||||
```
|
||||
|
||||
Result: `6 passed`.
|
||||
|
||||
Focused compatibility tests:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_operational_view_models.py central/tests/test_control_plane.py central/tests/test_jobs.py -q
|
||||
```
|
||||
|
||||
Result: `21 passed`.
|
||||
|
||||
Central regression suite:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests -q
|
||||
```
|
||||
|
||||
Result: `248 passed, 1 warning`.
|
||||
|
||||
The warning is the pre-existing `openpyxl` print-area warning from `central/tests/test_bootstrap_v1_extra.py`. The pytest environment also emits the existing `pytest-asyncio` fixture-loop-scope deprecation warning.
|
||||
|
||||
Additional verification:
|
||||
|
||||
```text
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Result: no whitespace errors. Git reported only its existing LF/CRLF normalization notice for the edited Python files.
|
||||
|
||||
## Changed Files
|
||||
|
||||
- `central/app/control_plane.py`: added bounded dashboard aggregates and attention items; enriched installation, job, and blocker view-models; added filters, stable pagination, worst-coverage sorting, presentation fields, and bounded blocker detail relations.
|
||||
- `central/tests/test_operational_view_models.py`: added focused TDD coverage for health reconciliation, distinct safe-blocked products, attention ordering/cap, fiscal/technical separation, job identity/duration, blocker filters/priorities, and detail relations.
|
||||
|
||||
## Contract Notes
|
||||
|
||||
- Dashboard health counts preserve lowercase compatibility keys and add uppercase `health_counts`.
|
||||
- `safe_blocked_products` counts distinct active `(tenant_id, product_code)` pairs; `open_blockers` counts active rows.
|
||||
- Safe fiscal blockers do not alter `technical_health`.
|
||||
- API timestamps remain ISO UTC values; presentation fields use Task 1 helpers.
|
||||
- Collection queries use joins/aggregates. Blocker detail related events, jobs, and audit rows are bounded and bulk-loaded.
|
||||
- Existing `resolved` behavior remains supported while `status` adds explicit `OPEN`, `RESOLVED`, and `ALL` handling.
|
||||
|
||||
## Commit
|
||||
|
||||
Implementation commit: `d729710` (`feat: add bounded operational view models`).
|
||||
|
||||
## Concerns
|
||||
|
||||
- `BlockedProduct` has no persisted installation foreign key. Blocker list/detail identity therefore uses the lowest-ID installation for the tenant as the representative context; agent and installation filters check tenant installation membership without duplicating blocker rows.
|
||||
- The existing non-failing pytest environment warnings remain as noted above.
|
||||
|
||||
## Task 2 Re-Review Fix Report
|
||||
|
||||
### Findings Fixed
|
||||
|
||||
- Unified fiscal completion rounding with deterministic one-decimal `ROUND_HALF_UP` behavior in Python and SQL. The SQL ratio is explicitly numeric before two-argument `ROUND`, preserving SQLite/PostgreSQL compatibility.
|
||||
- Extended `blocker_fingerprint` with an optional installation identifier. Legacy calls without an installation retain the original hash; authenticated upserts scope both generated and supplied fingerprints to the exact installation, so identical reports from two installations remain distinct.
|
||||
- Normalized blocker attention candidate fields with SQL `UPPER`, matching the normalized breakdown behavior for lowercase `cfop`, `st`, and other required fields.
|
||||
|
||||
### Re-Review RED Evidence
|
||||
|
||||
Command after adding the three regression tests:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_operational_view_models.py::test_worst_coverage_sort_cannot_disagree_with_one_decimal_display central/tests/test_operational_view_models.py::test_lowercase_required_blocker_field_reaches_attention_candidates central/tests/test_blocked_products.py::test_authenticated_installations_get_distinct_fingerprints_for_identical_blockers -q
|
||||
```
|
||||
|
||||
Result before implementation: `3 failed`.
|
||||
|
||||
The failures reproduced the SQL/Python 6.3 versus 6.2 ordering mismatch, omitted lowercase blocker attention, and deduplication of identical blocker reports across installations.
|
||||
|
||||
### Re-Review GREEN Evidence
|
||||
|
||||
Focused Task 2/control/blocker tests:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_operational_view_models.py central/tests/test_control_plane.py central/tests/test_jobs.py central/tests/test_blocked_products.py central/tests/test_operational_state.py -q
|
||||
```
|
||||
|
||||
Result: `37 passed`.
|
||||
|
||||
Full central suite:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests -q
|
||||
```
|
||||
|
||||
Result: `256 passed, 1 warning`.
|
||||
|
||||
The warning remains the pre-existing `openpyxl` print-area warning; the environment also emits the existing `pytest-asyncio` fixture-loop-scope deprecation warning.
|
||||
|
||||
PostgreSQL SQL compatibility check:
|
||||
|
||||
```text
|
||||
python -c "from sqlalchemy import select; from sqlalchemy.dialects.postgresql import dialect; from central.app.control_plane import _fiscal_completion_expression; print(select(_fiscal_completion_expression()).compile(dialect=dialect()))"
|
||||
```
|
||||
|
||||
Result: the expression compiles as numeric SQL using two-argument `ROUND`.
|
||||
|
||||
### Re-Review Fix Commit
|
||||
|
||||
Implementation commit: `2ee271c` (`fix: close remaining task 2 review findings`).
|
||||
|
||||
### Re-Review Concerns
|
||||
|
||||
- Legacy no-installation payloads intentionally retain their original fingerprint and tenant-level behavior; authenticated installation-scoped payloads use the new namespace.
|
||||
- The existing non-failing pytest environment warnings remain as noted above.
|
||||
|
||||
## Task 2 Review Fix Report
|
||||
|
||||
### Findings Fixed
|
||||
|
||||
- Added nullable `blocked_products.installation_id` with a foreign key to `installations.id` and index in Alembic revision `0018_blocker_installation_identity`; updated the SQLAlchemy model.
|
||||
- Updated blocker upsert and authenticated job completion to persist the authenticated Agent installation. Agent-facing blocker list/reprocess/export paths and control-panel agent detail routes now use exact installation ownership. Legacy null-owned rows remain tenant-level and render without fabricated server context.
|
||||
- Replaced dashboard installation/blocker materialization with SQL health counts, snapshot totals, open-row count, distinct tenant/product count, and blocker-field breakdown. Health, blocker, and version attention candidates are individually bounded before Python merge/truncation.
|
||||
- Replaced Python health filtering/slicing with a portable SQL `CASE` expression using the supplied `now`, failure/job fields, heartbeat cutoff, SQL count, order, offset, and limit.
|
||||
- Worst-coverage ordering now uses the same derived complete/total percentage, clamped and rounded consistently with presentation.
|
||||
|
||||
### Review-Fix RED Evidence
|
||||
|
||||
Command after adding the review regressions:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_operational_view_models.py central/tests/test_jobs.py -q
|
||||
```
|
||||
|
||||
Result: the new regressions failed as expected: exact ownership was unsupported, legacy blockers received the fabricated representative installation, dashboard SQL `DISTINCT` aggregation was absent, health-filtered SQL pagination was absent, and stored coverage ordering disagreed with derived coverage. The API assertion initially also exposed a test setup omission; the test was corrected to heartbeat/register the installation before implementation.
|
||||
|
||||
### Review-Fix GREEN Evidence
|
||||
|
||||
Focused review and compatibility tests:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_operational_view_models.py central/tests/test_control_plane.py central/tests/test_jobs.py central/tests/test_blocked_products.py -q
|
||||
```
|
||||
|
||||
Result: `30 passed`.
|
||||
|
||||
Full central suite:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests -q
|
||||
```
|
||||
|
||||
Result: `253 passed, 1 warning`.
|
||||
|
||||
The warning remains the pre-existing `openpyxl` print-area warning. The test environment also emits the existing `pytest-asyncio` fixture-loop-scope deprecation warning.
|
||||
|
||||
Additional verification:
|
||||
|
||||
```text
|
||||
python -m compileall -q central/app central/scripts
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Both completed without errors; Git's LF/CRLF normalization notices are environmental only.
|
||||
|
||||
### Review-Fix Commit
|
||||
|
||||
Implementation commit: `e35417f` (`fix: close task 2 operational review findings`).
|
||||
|
||||
### Review-Fix Concerns
|
||||
|
||||
- Legacy blocker rows intentionally remain tenant-level because their installation ownership is unavailable; they render `-`/null server context and are never assigned a representative installation.
|
||||
- The existing non-failing pytest environment warnings remain as noted above.
|
||||
@@ -0,0 +1,172 @@
|
||||
# Task 3 Report: Safe Blocker Resolution Workflow
|
||||
|
||||
## Status
|
||||
|
||||
Implemented Task 3 only. The old hidden-context blocker action was replaced by a structured, server-rendered, CSRF-protected resolution workflow. The JSON control-panel route now uses the same validated transaction service and rejects arbitrary context text.
|
||||
|
||||
## RED
|
||||
|
||||
Created `central/tests/test_blocker_resolution_panel.py` before production implementation.
|
||||
|
||||
Initial focused command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_blocker_resolution_panel.py -q
|
||||
```
|
||||
|
||||
Expected RED result:
|
||||
|
||||
```text
|
||||
ModuleNotFoundError: No module named 'central.app.blocker_resolution'
|
||||
```
|
||||
|
||||
The failure occurred during collection because the required production module did not yet exist.
|
||||
|
||||
## Implementation
|
||||
|
||||
- Added `central/app/blocker_resolution.py` with structured input validation through `validate_entry`, workflow-scope restrictions, exact PRODUCT scope-key enforcement, persisted installation/company context derivation, context upsert, bounded `REPROCESS_PRODUCTS` job creation, sanitized audit recording, and one commit with rollback on failure.
|
||||
- Added `GET /blockers/{blocker_id}` with installation-aware blocker detail and context form.
|
||||
- Added `POST /blockers/{blocker_id}/resolve-context` with human authentication and CSRF enforcement.
|
||||
- Updated `/v1/blockers/{blocker_id}/resolve-human` and added `/v1/blockers/{blocker_id}/resolve-context` to call the same service; neither accepts arbitrary context text.
|
||||
- Removed the blocker-list hidden fixed-context form and `data-confirm` action.
|
||||
- Added focused coverage for enum validation, unknown keys, unsupported NCM scope, empty reason, missing PRODUCT scope key, authentication, CSRF, context/job/audit persistence, exact product scoping, and secret redaction.
|
||||
|
||||
## GREEN
|
||||
|
||||
Focused command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_blocker_resolution_panel.py -q
|
||||
```
|
||||
|
||||
Result:
|
||||
|
||||
```text
|
||||
5 passed, 1 warning
|
||||
```
|
||||
|
||||
Central suite:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests -q
|
||||
```
|
||||
|
||||
Result:
|
||||
|
||||
```text
|
||||
261 passed, 1 warning
|
||||
```
|
||||
|
||||
The warnings are existing third-party test-environment warnings from `pytest_asyncio` configuration and `openpyxl` print-area parsing.
|
||||
|
||||
## Safety Review
|
||||
|
||||
- No Firebird, AI, SQL execution, shell, credential, or remote-write path was added.
|
||||
- The workflow accepts only `scope`, `scope_key`, `key`, `value`, and `reason`; context keys and enum values are validated against existing shared rules.
|
||||
- Company context is derived from persisted blocker/installation data, not arbitrary client `company_key` input.
|
||||
- Jobs contain exactly the blocked product code.
|
||||
- Audit context passes through existing redaction before persistence.
|
||||
|
||||
## Concerns
|
||||
|
||||
- Live PostgreSQL and agent execution were not exercised; verification used the repository's offline SQLite test fixtures.
|
||||
- The blocker remains open until the agent's normal reprocess completion path reports resolution; this prevents marking it resolved before the bounded job succeeds.
|
||||
|
||||
## Review Fixes
|
||||
|
||||
### RED
|
||||
|
||||
Added regression tests before changing production code for the four review findings:
|
||||
|
||||
- Legacy blockers with `installation_id=NULL` must fail closed without selecting an Agent or creating work.
|
||||
- Cross-tenant installation and Agent pointers must fail before context, job, or audit mutation.
|
||||
- Oversized values/reasons and non-string fields must be rejected cleanly.
|
||||
- Successful web resolution must redirect to the blocker detail page.
|
||||
|
||||
Focused command before the fixes:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_blocker_resolution_panel.py -q
|
||||
```
|
||||
|
||||
RED result:
|
||||
|
||||
```text
|
||||
4 failed, 5 passed
|
||||
```
|
||||
|
||||
The failures reproduced the legacy Agent fallback, cross-tenant acceptance, unbounded/malformed input, and `/blockers` redirect behavior.
|
||||
|
||||
### GREEN
|
||||
|
||||
- Resolution now requires a non-null exact installation and verifies both installation and Agent tenant ownership before mutation.
|
||||
- `value` is required to be a string of at most 512 characters; `reason` is required to be a string of at most 2000 characters.
|
||||
- Boundary-sized audit data retains all required audit fields without whole-context truncation.
|
||||
- Web success redirects to `/blockers/{blocker_id}`.
|
||||
|
||||
Focused command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_blocker_resolution_panel.py -q
|
||||
```
|
||||
|
||||
Result:
|
||||
|
||||
```text
|
||||
9 passed, 1 warning
|
||||
```
|
||||
|
||||
Full central command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests -q
|
||||
```
|
||||
|
||||
Result:
|
||||
|
||||
```text
|
||||
265 passed, 1 warning
|
||||
```
|
||||
|
||||
The existing `pytest_asyncio` configuration and `openpyxl` print-area warnings remain unchanged.
|
||||
|
||||
## UTF-8 Byte-Length Re-Review Fix
|
||||
|
||||
### RED
|
||||
|
||||
Added `test_resolution_input_rejects_multibyte_utf8_at_ascii_character_limits` before changing production code. It submits `é` repeated 512 times as `value` and 2000 times as `reason`; both are within the existing character counts but exceed the corresponding UTF-8 byte budgets.
|
||||
|
||||
Focused command:
|
||||
|
||||
```text
|
||||
python -m pytest central/tests/test_blocker_resolution_panel.py -q
|
||||
```
|
||||
|
||||
RED result:
|
||||
|
||||
```text
|
||||
1 failed, 9 passed
|
||||
```
|
||||
|
||||
The failure confirmed character-count validation accepted the multibyte inputs.
|
||||
|
||||
### GREEN
|
||||
|
||||
- `value` and `reason` limits now use `len(value.encode("utf-8"))` and `len(reason.encode("utf-8"))`.
|
||||
- Existing ASCII boundary inputs remain accepted.
|
||||
- Multibyte inputs fail closed with explicit UTF-8 byte-limit validation errors, before persistence or audit sanitization.
|
||||
- Redaction and remote-write boundaries were not changed.
|
||||
|
||||
Focused result:
|
||||
|
||||
```text
|
||||
10 passed, 1 warning
|
||||
```
|
||||
|
||||
Full central result:
|
||||
|
||||
```text
|
||||
266 passed, 1 warning
|
||||
```
|
||||
|
||||
The existing `pytest_asyncio` configuration and `openpyxl` print-area warnings remain unchanged.
|
||||
@@ -0,0 +1,44 @@
|
||||
# Task 4 Report: Server-Rendered Operational Pages
|
||||
|
||||
STATUS: COMPLETE
|
||||
|
||||
## Implemented
|
||||
|
||||
- Added authenticated page tests covering dashboard, agents, jobs, blockers, agent detail, blocker detail, filters, pagination state, human dates, and safe-form constraints.
|
||||
- Added server-side route filters for agents, blockers, jobs, and agent events while preserving bounded view-model calls.
|
||||
- Reworked the Jinja pages with Portuguese operational labels, separate technical/fiscal statuses, empty states, detail links, accessible controls, semantic table captions/headers, and secondary technical IDs.
|
||||
- Added dashboard attention, failed-job, outdated-agent, and blocker summary sections.
|
||||
- Preserved the Task 3 blocker detail and CSRF-protected resolution form, with evidence and related operational records.
|
||||
- Replaced raw primary timestamp rendering with humanized presentation values.
|
||||
- Replaced the old confirmation JavaScript with harmless progressive enhancement; no `window.confirm` or `data-confirm` remains in the panel pages.
|
||||
- Added responsive table overflow, mobile breakpoints, visible focus states, and text-bearing status badges.
|
||||
|
||||
## Verification
|
||||
|
||||
- RED: `python -m pytest central/tests/test_control_panel_pages.py -q` failed 5 tests against the pre-change templates/routes.
|
||||
- Focused: `python -m pytest central/tests/test_control_panel_pages.py -q` passed 5 tests.
|
||||
- Central suite: `python -m pytest central/tests -q` passed 271 tests with 1 existing openpyxl warning.
|
||||
- `git diff --check` passed.
|
||||
- No credentials, tokens, cookies, or secrets were added.
|
||||
|
||||
## Concerns
|
||||
|
||||
- The blocker page's affected-client count is derived from the bounded filtered result page; the canonical global blocker/product metrics continue to come from `dashboard_summary`.
|
||||
- Browser viewport screenshots were not run in this offline task; responsive behavior is covered by CSS breakpoints and horizontal table scrolling.
|
||||
|
||||
Commit message: `feat: refine operational control panel pages`
|
||||
|
||||
## Task 4 review fixes
|
||||
|
||||
- Added a dashboard list of at most 10 recent failed jobs with client, installation display name, hostname, job ID/kind, short error, created/finished display and age, and a job-ID-filtered link. The failed-job total and empty state remain visible.
|
||||
- Replaced page-local/global blocker metric mixing with SQL-backed aggregates over the complete relation selected by the current filters. Counts use distinct `(tenant_id, product_code)` identity and distinct tenant IDs; OPEN, RESOLVED, and ALL behavior is covered. Breakdown now follows the filtered relation.
|
||||
- Agent detail now loads its installation, agent, tenant, and snapshot by agent ID, without scanning the first 100 installations. Agent blocker breakdown and its filtered link include only blockers whose installation belongs to that agent; tenant-only legacy records are excluded.
|
||||
- Added event since/until filters and page-size+1 lookahead, preserving filters in pagination links and omitting Next on the final page. Job references link to jobs filtered by job ID; product references link to blockers filtered by agent/product.
|
||||
- Reprocess-all now opens an explicit server-rendered confirmation page with a required confirmation checkbox, CSRF token, and cancel link. Missing confirmation is rejected; valid confirmation retains the guarded enqueue path.
|
||||
- Added route regressions for these cases, including 55+ blockers, repeated product code across tenants, resolved/all status, agent ID beyond 100, exact blocker attribution, date filtering/pagination, links, and confirmation/CSRF behavior.
|
||||
- RED evidence: the new dashboard-row and date-filter cases failed before implementation because those views were absent; after correcting test setup, the new assertions detected the unimplemented behaviors. Focused regressions are green after implementation.
|
||||
- Verification: `python -m pytest central/tests/test_control_panel_pages.py -q` — 9 passed; `python -m pytest central/tests -q` — 275 passed, 1 existing openpyxl warning; `git diff --check` — passed.
|
||||
- Route smoke coverage is provided by authenticated TestClient requests. CSS breakpoint review confirmed the existing 1100px, 768px, and 520px responsive rules and table overflow remain intact.
|
||||
- The earlier concern that blocker metrics were page-local is resolved by the complete filtered SQL aggregates above. No writes to Firebird databases were performed.
|
||||
|
||||
Commit message: `fix: close operational panel page review findings`
|
||||
@@ -0,0 +1,79 @@
|
||||
# Task 5 Report: Fixture Coverage, Documentation, And Regression Tests
|
||||
|
||||
## Result
|
||||
|
||||
Completed the fixture regression coverage and operational UX documentation.
|
||||
The fixture retains its explicit non-production command gate and the default
|
||||
100-Agent/500-event/100-job shape. Its deterministic blocker count is now 51:
|
||||
50 distinct open tenant/product pairs, including two open blocker rows for the
|
||||
same first product. Safe-blocked snapshot counts match the fixture's open
|
||||
product pairs. Failed jobs have duration timestamps, and linked events point to
|
||||
their job and, where present, the correct blocker/product and installation.
|
||||
|
||||
## TDD evidence
|
||||
|
||||
1. Added assertions for the additional same-product blocker, unique fingerprints,
|
||||
exact UUID5 installation identities, ownership, failed-job durations, and
|
||||
job/blocker/product event links. The first focused run was RED as expected:
|
||||
fixture returned 50 blockers while the requirement expected 51.
|
||||
2. After implementing the blocker/job/event fixtures, added an assertion that
|
||||
each snapshot's `safe_blocked` value matched its open product pairs. The
|
||||
focused run was RED as expected because the old sample formula reported
|
||||
blocked products for installations without an open blocker.
|
||||
3. Updated the deterministic snapshot values to match the generated open
|
||||
product pairs. The focused test then passed.
|
||||
|
||||
## Verification commands
|
||||
|
||||
- RED: `python -m pytest central/tests/test_control_panel_fixture.py -q` —
|
||||
failed on the expected blocker count gap (`50 != 51`).
|
||||
- RED: `python -m pytest central/tests/test_control_panel_fixture.py -q` —
|
||||
failed on the expected safe-blocked snapshot mismatch (`1 != 0`).
|
||||
- GREEN: `python -m pytest central/tests/test_control_panel_fixture.py -q` —
|
||||
passed (`1 passed`).
|
||||
- Full central suite: `python -m pytest central/tests -q` — passed
|
||||
(`275 passed`). Pytest emitted its existing `pytest-asyncio` fixture-loop
|
||||
deprecation warning and one existing `openpyxl` print-area warning.
|
||||
- `git diff --cached --check` — passed with no whitespace errors.
|
||||
|
||||
## Documentation review
|
||||
|
||||
Updated `docs/CONTROL_PANEL_V1.md` to link to the new operational UX contract
|
||||
and distinguish the installation-wide confirmation step from the validated
|
||||
blocker context workflow. Added `docs/CONTROL_PANEL_OPERATIONAL_UX_V1.md`
|
||||
covering the health invariant, technical/fiscal distinction, blocker/product
|
||||
counts, dashboard attention and empty states, identity-first views, timestamp
|
||||
and timezone presentation, blocker filters and summaries, operational-only
|
||||
priority tiers, safe resolution/auth/CSRF/audit fields, reprocess confirmation,
|
||||
password policy, performance limits, responsiveness, fixture command, and
|
||||
write/deployment boundaries.
|
||||
|
||||
## Commit
|
||||
|
||||
- Implementation and product documentation: `4ba0fa8`
|
||||
(`test: complete control panel fixture UX coverage`)
|
||||
|
||||
## Concerns
|
||||
|
||||
- Resolved in the timestamp tooltip follow-up below: templates now use raw ISO
|
||||
UTC timestamps in `title` and retain localized human-readable visible text.
|
||||
- Full central tests retain the two pre-existing dependency warnings described
|
||||
above; they did not fail the suite.
|
||||
|
||||
## Timestamp tooltip follow-up
|
||||
|
||||
Resolved the full technical timestamp concern: agent, job, blocker, and
|
||||
populated dashboard failure timestamps now keep their localized human-readable
|
||||
text visible and expose the complete ISO UTC timestamp (including seconds and
|
||||
`+00:00`) in the `title` attribute. Blocker detail includes first- and
|
||||
last-seen timestamps. The existing raw ISO serializer now explicitly normalizes
|
||||
naive SQLite timestamps as UTC, matching the presentation layer's existing
|
||||
interpretation without changing the view-model shape.
|
||||
|
||||
### TDD and verification
|
||||
|
||||
- RED: `python -m pytest central/tests/test_control_panel_pages.py -q -k "localized_last_seen or localized_created or localized_seen_times or bounded_failed_job_rows"` — 4 expected failures because the tooltips did not contain the required raw ISO values.
|
||||
- GREEN: `python -m pytest central/tests/test_control_panel_pages.py -q` — passed (`12 passed`).
|
||||
- Full suite: `python -m pytest central/tests -q` — passed (`278 passed`, one existing `openpyxl` warning; pytest-asyncio also emits its existing fixture-loop deprecation warning).
|
||||
- `git diff --check` — passed.
|
||||
- Commit message: `fix: show technical timestamps in tooltips`.
|
||||
Reference in New Issue
Block a user