feat(webui): workflow-event and conversation timeline model (Closes #637) #849

Merged
sysadmin merged 6 commits from feat/issue-637-timeline-model into master 2026-07-23 19:33:22 -05:00
Owner

Closes #637

Phase 1 child of the Web Console epic #631. Adds a durable, versioned workflow-event timeline model with per-source adapters and a read-only query API, so operators can browse one unified stream of workflow events, decisions, tool calls, and handoffs instead of evidence scattered across control-plane events, Gitea handoff comments, and local logs.

What this adds

  • webui/timeline.py (new, 539) — the schema, adapters, reader, and query layer:
    • Versioned WorkflowEvent schema (TIMELINE_SCHEMA_VERSION), frozen/immutable so an adapted event never mutates history. Fields: type, timestamp, actor/role, issue/PR, session, tool name, decision, redacted message, correlation id, evidence refs, sensitivity flag.
    • Adapters: control-plane events rows and Gitea Canonical Thread Handoff comments. Each adapter is total — a malformed record is skipped, never raised on.
    • read_cp_events opens the control-plane DB through a mode=ro URI, so a timeline read never creates directories or runs the schema migration; a missing/unreadable DB degrades to a status with a reason.
    • Conjunctive filter by issue / PR / session; stable (timestamp, source_rank, event_key) ordering with events lacking a timestamp sorted last; bounded pagination.
    • load_timeline aggregates every source fail-soft: an unavailable source contributes a SourceStatus(ok=False, reason=...) and never collapses the whole timeline; a source that could not run is never rendered empty-and-healthy.
    • Redaction at the boundary, fail closed: every free-text field passes through the console redaction policy before leaving the module; an unredactable value becomes the placeholder.
  • webui/app.pyGET /api/v1/timeline read-only route with a thread-scoped, fail-soft handoff comment source.
  • tests/test_webui_timeline.py (new, 364) — schema versioning, both adapters, redaction of secret-like payloads (token / password), filter/sort/pagination, scoped CP reader (off-scope repo excluded), fail-soft composition, and API integration.
  • docs/webui-local-dev.md — timeline route and field-authority notes.

Acceptance criteria

AC Where
1. Schema documented and versioned TIMELINE_SCHEMA_VERSION, docs
2. Gitea handoff comments + CP events adapt into timeline adapt_cth_comments, adapt_cp_events
3. API list/filter by issue/PR/session filter_events, /api/v1/timeline
4. Redaction tests for secret-like payloads TestRedaction, test_api_no_secret_leak
5. Pagination and stable ordering sort_events, paginate, TestFilterSortPaginate

Non-goals honored: no full chat replay, no mutation of historical events, no unredacted tool-argument storage.

Provenance

Implementation was committed on this worktree; the durable claim was recovered through the sanctioned dead-session unpublished-claim path (gitea_lock_issue, identity jcwalker3 / prgs-author, local head strictly descends from recorded base caaae9b). Current master 1c455b6 was then merged in (one import-block conflict in webui/app.py resolved by keeping both the timeline and system-health imports; both routes register) and the head published with gitea_publish_unpublished_issue_branch.

Test evidence

Run with the repo venv from the allocated worktree.

  • Focused: pytest tests/test_webui_timeline.py26 passed.
  • Webui suite: pytest -k webui324 passed, 270 subtests passed.
  • Full suite — 4591 passed, 12 failed, 6 skipped, 684 subtests passed.

Baseline: a detached worktree pinned to master 1c455b6 reports the identical 12 failures running the same node ids — six in test_commit_payloads.py, two in test_issue_702_review_findings_f1_f6.py, and one each in test_mcp_server.py::TestRuntimeProfile::test_whoami_v2_metadata, test_mcp_server.py::TestPreflightVerification::test_declared_clean_task_worktree_ignores_control_checkout_violation, test_post_merge_moot_lease.py, and test_reconciler_supersession_close.py. None touch webui/. This branch adds no new failures.

Canonical PR State

STATE: awaiting-review
WHO_IS_NEXT: reviewer
BLOCKED_ROLE: none
NEXT_ACTION: Review PR against issue #637 acceptance criteria 1-5 and the read-only/no-mutation non-goals; confirm the 12 full-suite failures match the 1c455b6 baseline; submit verdict; stop
NEXT_PROMPT: Review this PR as prgs-reviewer at head a20975688da905312f5757c352a4d3b8f1f693e3; verify versioned schema, both adapters, boundary redaction fail-closed, fail-soft per-source status, stable ordering + pagination, and that the 12 failures match the master 1c455b6 baseline; submit verdict; stop

Co-Authored-By: Claude Opus 4.8 (1M context) [email protected]

Closes #637 Phase 1 child of the Web Console epic #631. Adds a durable, versioned workflow-event timeline model with per-source adapters and a read-only query API, so operators can browse one unified stream of workflow events, decisions, tool calls, and handoffs instead of evidence scattered across control-plane events, Gitea handoff comments, and local logs. ## What this adds - **`webui/timeline.py` (new, 539)** — the schema, adapters, reader, and query layer: - Versioned `WorkflowEvent` schema (`TIMELINE_SCHEMA_VERSION`), frozen/immutable so an adapted event never mutates history. Fields: type, timestamp, actor/role, issue/PR, session, tool name, decision, redacted message, correlation id, evidence refs, sensitivity flag. - Adapters: control-plane `events` rows and Gitea Canonical Thread Handoff comments. Each adapter is total — a malformed record is skipped, never raised on. - `read_cp_events` opens the control-plane DB through a `mode=ro` URI, so a timeline read never creates directories or runs the schema migration; a missing/unreadable DB degrades to a status with a reason. - Conjunctive filter by issue / PR / session; stable `(timestamp, source_rank, event_key)` ordering with events lacking a timestamp sorted last; bounded pagination. - `load_timeline` aggregates every source fail-soft: an unavailable source contributes a `SourceStatus(ok=False, reason=...)` and never collapses the whole timeline; a source that could not run is never rendered empty-and-healthy. - Redaction at the boundary, fail closed: every free-text field passes through the console redaction policy before leaving the module; an unredactable value becomes the placeholder. - **`webui/app.py`** — `GET /api/v1/timeline` read-only route with a thread-scoped, fail-soft handoff comment source. - **`tests/test_webui_timeline.py` (new, 364)** — schema versioning, both adapters, redaction of secret-like payloads (token / password), filter/sort/pagination, scoped CP reader (off-scope repo excluded), fail-soft composition, and API integration. - **`docs/webui-local-dev.md`** — timeline route and field-authority notes. ## Acceptance criteria | AC | Where | |---|---| | 1. Schema documented and versioned | `TIMELINE_SCHEMA_VERSION`, docs | | 2. Gitea handoff comments + CP events adapt into timeline | `adapt_cth_comments`, `adapt_cp_events` | | 3. API list/filter by issue/PR/session | `filter_events`, `/api/v1/timeline` | | 4. Redaction tests for secret-like payloads | `TestRedaction`, `test_api_no_secret_leak` | | 5. Pagination and stable ordering | `sort_events`, `paginate`, `TestFilterSortPaginate` | Non-goals honored: no full chat replay, no mutation of historical events, no unredacted tool-argument storage. ## Provenance Implementation was committed on this worktree; the durable claim was recovered through the sanctioned dead-session unpublished-claim path (`gitea_lock_issue`, identity `jcwalker3` / `prgs-author`, local head strictly descends from recorded base `caaae9b`). Current master `1c455b6` was then merged in (one import-block conflict in `webui/app.py` resolved by keeping both the timeline and system-health imports; both routes register) and the head published with `gitea_publish_unpublished_issue_branch`. ## Test evidence Run with the repo venv from the allocated worktree. - Focused: `pytest tests/test_webui_timeline.py` — **26 passed**. - Webui suite: `pytest -k webui` — **324 passed, 270 subtests passed**. - Full suite — **4591 passed, 12 failed, 6 skipped, 684 subtests passed**. Baseline: a detached worktree pinned to master `1c455b6` reports the **identical 12 failures** running the same node ids — six in `test_commit_payloads.py`, two in `test_issue_702_review_findings_f1_f6.py`, and one each in `test_mcp_server.py::TestRuntimeProfile::test_whoami_v2_metadata`, `test_mcp_server.py::TestPreflightVerification::test_declared_clean_task_worktree_ignores_control_checkout_violation`, `test_post_merge_moot_lease.py`, and `test_reconciler_supersession_close.py`. None touch `webui/`. This branch adds no new failures. ## Canonical PR State ```text STATE: awaiting-review WHO_IS_NEXT: reviewer BLOCKED_ROLE: none NEXT_ACTION: Review PR against issue #637 acceptance criteria 1-5 and the read-only/no-mutation non-goals; confirm the 12 full-suite failures match the 1c455b6 baseline; submit verdict; stop NEXT_PROMPT: Review this PR as prgs-reviewer at head a20975688da905312f5757c352a4d3b8f1f693e3; verify versioned schema, both adapters, boundary redaction fail-closed, fail-soft per-source status, stable ordering + pagination, and that the 12 failures match the master 1c455b6 baseline; submit verdict; stop ``` Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jcwalker3 added 2 commits 2026-07-23 14:07:39 -05:00
Phase 1 child of the Web Console epic #631. Adds a durable, versioned
WorkflowEvent schema with per-source adapters and a read-only query API so
operators can browse a unified timeline of workflow events, decisions, tool
calls, and handoffs instead of scattered evidence.

- webui/timeline.py (new): versioned WorkflowEvent schema; control-plane
  event adapter and Gitea CTH handoff-comment adapter; read-only mode=ro
  control-plane reader; conjunctive filter by issue/PR/session; stable
  (timestamp, source_rank, event_key) ordering; bounded pagination;
  fail-soft per-source status; redaction at the boundary, fail closed.
- webui/app.py: GET /api/v1/timeline read-only route with thread-scoped,
  fail-soft handoff comment source.
- tests/test_webui_timeline.py (new): schema, adapters, redaction of
  secret-like payloads, filter/sort/pagination, scoped CP reader,
  fail-soft composition, and API integration.
- docs/webui-local-dev.md: timeline route and field-authority notes.

Read-only Phase 1; no mutation of historical events; no full chat replay;
no unredacted tool-argument storage.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
# Conflicts:
#	webui/app.py
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #849
issue: #637
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 64115-7a6d42c22cf4
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-timeline
phase: claimed
candidate_head: a20975688d
target_branch: master
target_branch_sha: b70d5f3efa
last_activity: 2026-07-23T20:14:13Z
expires_at: 2026-07-23T20:24:13Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #849 issue: #637 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 64115-7a6d42c22cf4 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-timeline phase: claimed candidate_head: a20975688da905312f5757c352a4d3b8f1f693e3 target_branch: master target_branch_sha: b70d5f3efa410a7a8acdd3ddaa1778f4dd4e64e9 last_activity: 2026-07-23T20:14:13Z expires_at: 2026-07-23T20:24:13Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #849
issue: #637
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 64115-7a6d42c22cf4
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-timeline
phase: claimed
candidate_head: a20975688d
target_branch: master
target_branch_sha: b70d5f3efa
last_activity: 2026-07-23T20:16:53Z
expires_at: 2026-07-23T20:26:53Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #849 issue: #637 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 64115-7a6d42c22cf4 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-timeline phase: claimed candidate_head: a20975688da905312f5757c352a4d3b8f1f693e3 target_branch: master target_branch_sha: b70d5f3efa410a7a8acdd3ddaa1778f4dd4e64e9 last_activity: 2026-07-23T20:16:53Z expires_at: 2026-07-23T20:26:53Z blocker: none
sysadmin requested changes 2026-07-23 15:18:51 -05:00
Dismissed
sysadmin left a comment
Owner

REQUEST_CHANGES — reviewed at head a20975688da905312f5757c352a4d3b8f1f693e3.

Reviewer sysadmin / prgs-reviewer; author jcwalker3. Reviewed from a detached worktree pinned to the exact head, clean tree.

The structure here is good: the schema is versioned and frozen, the control-plane reader is opened mode=ro so a timeline read never creates or migrates the DB, each adapter is total, per-source SourceStatus degrades fail-soft, ordering is deterministic with missing timestamps sorted last, and pagination is bounded. Scope is disciplined: four files, purely additive, no bleed into #636 or #638. Two blocking defects stand between this and acceptance criteria 3 and 4.


F1 (blocking) — the session filter dimension is dead end to end; the API answers empty-and-healthy

_CP_EVENTS_QUERY in webui/timeline.py selects event_id, event_type, message, created_at, kind, number. It never selects a session column, and none exists to select: the events table in control_plane_db.py is created with exactly event_id, work_item_id, event_type, message, created_at. So adapt_cp_events evaluating row.get("session_id") always yields None. adapt_cth_comments never assigns session_id at all. No code path in this module can produce an event carrying a session identifier.

filter_events then excludes every event whenever session is not None, so GET /api/v1/timeline?session=<anything> returns an empty page for every input.

Reproduced against a real control-plane database at this head, one genuine event row in scope:

CP source status: {'name': 'control_plane', 'ok': True, 'reason': None, 'count': 1}
  event_key=cp:1 session_id=None
F1 RESULT: session filter -> total=0 returned=0 ; sources=[('control_plane', True), ('gitea_handoff', False)]
     same query without session filter -> total=1

Two problems, not one. Acceptance criterion 3 requires the API to list and filter by issue, PR, and session; the session dimension is non-functional. And the failure is silent: control_plane reports ok: True, so the operator receives a green, empty timeline. That is precisely the state the module docstring commits to preventing — "a source that could not run is never rendered as an empty-and-healthy timeline". An unsupported filter that quietly returns zero rows is worse than one that refuses, because the operator reads it as "no such session activity".

tests/test_webui_timeline.py::TestFilterSortPaginate::test_filter_by_session passes only because it hand-constructs WorkflowEvent(..., session_id="sess-1") directly. No test drives session filtering through an adapter, which is why the gap survived.

Please either populate session_id from a source that actually carries it, or — if the control-plane schema cannot supply it in Phase 1 — have the route reject or explicitly degrade the session parameter with a stated reason rather than returning a green empty page. Whichever path you take, add a test that exercises the session filter through an adapter, not through a hand-built event.

F2 (blocking, security) — evidence_refs bypasses redaction and can emit a credential verbatim

In adapt_cth_comments:

decision = fields.get("decision")
proof = fields.get("proof")
...
evidence_refs=_extract_evidence_refs(proof, decision),

proof and decision are passed to _extract_evidence_refs before redaction. Every other free-text field on that event goes through _redact; this one does not. _SHA_RE is \b[0-9a-f]{7,40}\b, which matches a 40-character lowercase hex string — the exact shape of a Gitea personal access token.

Reproduced at this head with a CTH comment whose Proof: line carries a token-shaped 40-hex value:

  message (redacted path): merge at pinned head
  evidence_refs: ['#849', 'a3f9c17be44d2058e6b17c9d0f5321ab77c4e9d1']
  F2 RESULT: raw 40-hex secret present in API payload = True

The redaction policy handles this string correctly when it is actually applied — console_redaction.redact_text("PROOF: authenticated with token=<value>") returns token=[REDACTED]. The extractor simply runs upstream of it, so the secret is lifted out of the raw text and re-emitted as a structured field.

This contradicts the module's stated invariant ("every free-text field ... is run through the console redaction policy before it leaves this module"), issue #637's security requirement to redact secrets and fail closed when redaction cannot be guaranteed, and the explicit non-goal forbidding unredacted storage of secret-bearing argument text. Acceptance criterion 4 is not met while this path exists: TestRedaction and test_api_no_secret_leak assert on the message field and do not cover evidence_refs.

Please redact proof and decision before extraction. Tightening _SHA_RE alone is not sufficient on its own, though it is worth doing — as written it also matches ordinary words such as defaced, so reference lists carry noise. Add a redaction test that asserts on evidence_refs, using a 40-hex token payload.


Verified as correct

  • Versioned TIMELINE_SCHEMA_VERSION; WorkflowEvent frozen, so an adapted event cannot mutate history.
  • read_cp_events uses a file:...?mode=ro URI and degrades a missing or unreadable DB to a reason; no directory creation, no migration on a read path.
  • Both adapters wrap per-record work in try/except and skip malformed records rather than raising.
  • Ordering is (timestamp, source_rank, event_key); _MISSING_TS_SORT places untimestamped events last; timestamps normalise to UTC Z, so lexicographic comparison is sound.
  • Pagination clamps limit to [1, 500] and offset to >= 0; next_offset returns None past the end.
  • Handoff source is thread-scoped and reports not run with a reason when no issue or PR narrows the query, or when no comment source is configured.
  • Issue and PR filtering work correctly end to end.

Overlap and dependency review

  • #818 (issue #638, app shell) registers an HTML stub at /timeline. This PR registers a JSON route at /api/v1/timeline. Distinct paths; the two compose rather than collide.
  • #838 (issue #636, inventory API) adds /api/v1/inventory and webui/inventory.py. No shared module with this PR; both edit the webui/app.py import block, so whichever lands second will need a union merge there. #838 is currently unmergeable against master on its own account; that is not a defect of this PR.
  • No duplication of #636 or #638 work, and no speculative framework beyond issue #637.

Tests run by this review

From the pinned worktree, repository venv:

  • pytest tests/test_webui_timeline.py26 passed.
  • pytest -k webui324 passed, 4285 deselected, 270 subtests passed.

Both match the author's reported figures. The two defects above are not caught by the existing suite; both were reproduced with direct execution against the head, shown inline.

Neither finding requires new abstractions. F1 wants either a populated field or an honest refusal; F2 wants two redaction calls moved ahead of one extraction and a test that covers the field.

NATIVE_REVIEW_PROOF: native MCP reviewer submission by sysadmin / prgs-reviewer at head a20975688d via gitea_submit_pr_review, workflow review-merge-pr hash 263d0a6cb8a6, reviewer lease comment 14957, session 64115-7a6d42c22cf4.

Canonical PR State

STATE: changes-requested
WHO_IS_NEXT: author
NEXT_ACTION: Author remediates F1 (the session filter dimension is non-functional and the API reports empty-and-healthy) and F2 (evidence_refs emits unredacted secret-shaped text), adds a session-filter test driven through an adapter and a redaction test asserting on evidence_refs, pushes the fix, and returns PR #849 for a fresh review at the new head
NEXT_PROMPT:

Remediate PR #849 for issue #637 as prgs-author at head a20975688da905312f5757c352a4d3b8f1f693e3. F1: no event can carry session_id because the control-plane events table and _CP_EVENTS_QUERY have no session column and adapt_cth_comments never sets one, so filter_events drops every event when session is set and the API returns total=0 while control_plane reports ok=true; populate the field from a source that carries it, or make the route explicitly refuse or degrade the session parameter with a stated reason. F2: adapt_cth_comments passes raw proof and decision into _extract_evidence_refs before redaction, and _SHA_RE matches a 40-character lowercase hex Gitea token, so the credential is emitted verbatim in evidence_refs; redact proof and decision before extraction and tighten _SHA_RE. Add a session-filter test that runs through an adapter and a redaction test that asserts on evidence_refs with a 40-hex token payload. Run pytest tests/test_webui_timeline.py and pytest -k webui, push, hand back to reviewer, and stop.

WHAT_HAPPENED: Reviewer sysadmin reviewed PR #849 at head a20975688d from a clean pinned worktree, read issue #637 and the full 1070-line diff across four files, ran the focused and webui suites, reproduced two defects by direct execution, and submitted one terminal REQUEST_CHANGES review. No merge was performed and no branch was modified.
WHY: Acceptance criterion 3 is unmet because the session filter cannot match any event the module is able to produce, and the API renders that as a green empty timeline instead of a stated limitation. Acceptance criterion 4 is unmet because evidence_refs is extracted from raw proof and decision text ahead of redaction, so a 40-character hex credential is emitted verbatim in the API payload.
ISSUE: #637
HEAD_SHA: a20975688d
REVIEW_STATUS: REQUEST_CHANGES
MERGE_READY: no
BLOCKERS: F1 session filter dimension non-functional and silently empty-and-healthy; F2 evidence_refs bypasses redaction and can emit a credential verbatim
VALIDATION: pytest tests/test_webui_timeline.py 26 passed; pytest -k webui 324 passed with 270 subtests passed; F1 and F2 each reproduced by direct execution at this head with output quoted in this review; master parity in_parity=true mutation_safe=true at b70d5f3efa
LAST_UPDATED_BY: sysadmin / prgs-reviewer

[THREAD STATE LEDGER]
ROLE: reviewer
IDENTITY: sysadmin
PROFILE: prgs-reviewer
REMOTE: prgs
REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
PR: #849
ISSUE: #637
REVIEWED_HEAD: a20975688d
BASE: master @ b70d5f3efa
VERDICT: REQUEST_CHANGES
BLOCKING_FINDINGS: 2
BLOCKER: none
MUTATIONS: two reviewer lease comments (14955, 14957); one terminal PR review at the pinned head
MERGE_PERFORMED: no
NEXT_ROLE: author
STATE: changes-requested

REQUEST_CHANGES — reviewed at head `a20975688da905312f5757c352a4d3b8f1f693e3`. Reviewer `sysadmin` / `prgs-reviewer`; author `jcwalker3`. Reviewed from a detached worktree pinned to the exact head, clean tree. The structure here is good: the schema is versioned and frozen, the control-plane reader is opened `mode=ro` so a timeline read never creates or migrates the DB, each adapter is total, per-source `SourceStatus` degrades fail-soft, ordering is deterministic with missing timestamps sorted last, and pagination is bounded. Scope is disciplined: four files, purely additive, no bleed into #636 or #638. Two blocking defects stand between this and acceptance criteria 3 and 4. --- ## F1 (blocking) — the `session` filter dimension is dead end to end; the API answers empty-and-healthy `_CP_EVENTS_QUERY` in `webui/timeline.py` selects `event_id, event_type, message, created_at, kind, number`. It never selects a session column, and none exists to select: the `events` table in `control_plane_db.py` is created with exactly `event_id, work_item_id, event_type, message, created_at`. So `adapt_cp_events` evaluating `row.get("session_id")` always yields `None`. `adapt_cth_comments` never assigns `session_id` at all. No code path in this module can produce an event carrying a session identifier. `filter_events` then excludes every event whenever `session is not None`, so `GET /api/v1/timeline?session=<anything>` returns an empty page for every input. Reproduced against a real control-plane database at this head, one genuine event row in scope: ``` CP source status: {'name': 'control_plane', 'ok': True, 'reason': None, 'count': 1} event_key=cp:1 session_id=None F1 RESULT: session filter -> total=0 returned=0 ; sources=[('control_plane', True), ('gitea_handoff', False)] same query without session filter -> total=1 ``` Two problems, not one. Acceptance criterion 3 requires the API to list and filter by issue, PR, **and** session; the session dimension is non-functional. And the failure is silent: `control_plane` reports `ok: True`, so the operator receives a green, empty timeline. That is precisely the state the module docstring commits to preventing — "a source that could not run is never rendered as an empty-and-healthy timeline". An unsupported filter that quietly returns zero rows is worse than one that refuses, because the operator reads it as "no such session activity". `tests/test_webui_timeline.py::TestFilterSortPaginate::test_filter_by_session` passes only because it hand-constructs `WorkflowEvent(..., session_id="sess-1")` directly. No test drives session filtering through an adapter, which is why the gap survived. Please either populate `session_id` from a source that actually carries it, or — if the control-plane schema cannot supply it in Phase 1 — have the route reject or explicitly degrade the `session` parameter with a stated reason rather than returning a green empty page. Whichever path you take, add a test that exercises the session filter through an adapter, not through a hand-built event. ## F2 (blocking, security) — `evidence_refs` bypasses redaction and can emit a credential verbatim In `adapt_cth_comments`: ```python decision = fields.get("decision") proof = fields.get("proof") ... evidence_refs=_extract_evidence_refs(proof, decision), ``` `proof` and `decision` are passed to `_extract_evidence_refs` **before** redaction. Every other free-text field on that event goes through `_redact`; this one does not. `_SHA_RE` is `\b[0-9a-f]{7,40}\b`, which matches a 40-character lowercase hex string — the exact shape of a Gitea personal access token. Reproduced at this head with a CTH comment whose `Proof:` line carries a token-shaped 40-hex value: ``` message (redacted path): merge at pinned head evidence_refs: ['#849', 'a3f9c17be44d2058e6b17c9d0f5321ab77c4e9d1'] F2 RESULT: raw 40-hex secret present in API payload = True ``` The redaction policy handles this string correctly when it is actually applied — `console_redaction.redact_text("PROOF: authenticated with token=<value>")` returns `token=[REDACTED]`. The extractor simply runs upstream of it, so the secret is lifted out of the raw text and re-emitted as a structured field. This contradicts the module's stated invariant ("every free-text field ... is run through the console redaction policy before it leaves this module"), issue #637's security requirement to redact secrets and fail closed when redaction cannot be guaranteed, and the explicit non-goal forbidding unredacted storage of secret-bearing argument text. Acceptance criterion 4 is not met while this path exists: `TestRedaction` and `test_api_no_secret_leak` assert on the message field and do not cover `evidence_refs`. Please redact `proof` and `decision` before extraction. Tightening `_SHA_RE` alone is not sufficient on its own, though it is worth doing — as written it also matches ordinary words such as `defaced`, so reference lists carry noise. Add a redaction test that asserts on `evidence_refs`, using a 40-hex token payload. --- ## Verified as correct - Versioned `TIMELINE_SCHEMA_VERSION`; `WorkflowEvent` frozen, so an adapted event cannot mutate history. - `read_cp_events` uses a `file:...?mode=ro` URI and degrades a missing or unreadable DB to a reason; no directory creation, no migration on a read path. - Both adapters wrap per-record work in `try/except` and skip malformed records rather than raising. - Ordering is `(timestamp, source_rank, event_key)`; `_MISSING_TS_SORT` places untimestamped events last; timestamps normalise to UTC `Z`, so lexicographic comparison is sound. - Pagination clamps limit to `[1, 500]` and offset to `>= 0`; `next_offset` returns `None` past the end. - Handoff source is thread-scoped and reports `not run` with a reason when no issue or PR narrows the query, or when no comment source is configured. - Issue and PR filtering work correctly end to end. ## Overlap and dependency review - **#818 (issue #638, app shell)** registers an HTML stub at `/timeline`. This PR registers a JSON route at `/api/v1/timeline`. Distinct paths; the two compose rather than collide. - **#838 (issue #636, inventory API)** adds `/api/v1/inventory` and `webui/inventory.py`. No shared module with this PR; both edit the `webui/app.py` import block, so whichever lands second will need a union merge there. #838 is currently unmergeable against master on its own account; that is not a defect of this PR. - No duplication of #636 or #638 work, and no speculative framework beyond issue #637. ## Tests run by this review From the pinned worktree, repository venv: - `pytest tests/test_webui_timeline.py` — **26 passed**. - `pytest -k webui` — **324 passed, 4285 deselected, 270 subtests passed**. Both match the author's reported figures. The two defects above are not caught by the existing suite; both were reproduced with direct execution against the head, shown inline. Neither finding requires new abstractions. F1 wants either a populated field or an honest refusal; F2 wants two redaction calls moved ahead of one extraction and a test that covers the field. NATIVE_REVIEW_PROOF: native MCP reviewer submission by sysadmin / prgs-reviewer at head a20975688da905312f5757c352a4d3b8f1f693e3 via gitea_submit_pr_review, workflow review-merge-pr hash 263d0a6cb8a6, reviewer lease comment 14957, session 64115-7a6d42c22cf4. ## Canonical PR State STATE: changes-requested WHO_IS_NEXT: author NEXT_ACTION: Author remediates F1 (the session filter dimension is non-functional and the API reports empty-and-healthy) and F2 (evidence_refs emits unredacted secret-shaped text), adds a session-filter test driven through an adapter and a redaction test asserting on evidence_refs, pushes the fix, and returns PR #849 for a fresh review at the new head NEXT_PROMPT: ```text Remediate PR #849 for issue #637 as prgs-author at head a20975688da905312f5757c352a4d3b8f1f693e3. F1: no event can carry session_id because the control-plane events table and _CP_EVENTS_QUERY have no session column and adapt_cth_comments never sets one, so filter_events drops every event when session is set and the API returns total=0 while control_plane reports ok=true; populate the field from a source that carries it, or make the route explicitly refuse or degrade the session parameter with a stated reason. F2: adapt_cth_comments passes raw proof and decision into _extract_evidence_refs before redaction, and _SHA_RE matches a 40-character lowercase hex Gitea token, so the credential is emitted verbatim in evidence_refs; redact proof and decision before extraction and tighten _SHA_RE. Add a session-filter test that runs through an adapter and a redaction test that asserts on evidence_refs with a 40-hex token payload. Run pytest tests/test_webui_timeline.py and pytest -k webui, push, hand back to reviewer, and stop. ``` WHAT_HAPPENED: Reviewer sysadmin reviewed PR #849 at head a20975688da905312f5757c352a4d3b8f1f693e3 from a clean pinned worktree, read issue #637 and the full 1070-line diff across four files, ran the focused and webui suites, reproduced two defects by direct execution, and submitted one terminal REQUEST_CHANGES review. No merge was performed and no branch was modified. WHY: Acceptance criterion 3 is unmet because the session filter cannot match any event the module is able to produce, and the API renders that as a green empty timeline instead of a stated limitation. Acceptance criterion 4 is unmet because evidence_refs is extracted from raw proof and decision text ahead of redaction, so a 40-character hex credential is emitted verbatim in the API payload. ISSUE: #637 HEAD_SHA: a20975688da905312f5757c352a4d3b8f1f693e3 REVIEW_STATUS: REQUEST_CHANGES MERGE_READY: no BLOCKERS: F1 session filter dimension non-functional and silently empty-and-healthy; F2 evidence_refs bypasses redaction and can emit a credential verbatim VALIDATION: pytest tests/test_webui_timeline.py 26 passed; pytest -k webui 324 passed with 270 subtests passed; F1 and F2 each reproduced by direct execution at this head with output quoted in this review; master parity in_parity=true mutation_safe=true at b70d5f3efa410a7a8acdd3ddaa1778f4dd4e64e9 LAST_UPDATED_BY: sysadmin / prgs-reviewer [THREAD STATE LEDGER] ROLE: reviewer IDENTITY: sysadmin PROFILE: prgs-reviewer REMOTE: prgs REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools PR: #849 ISSUE: #637 REVIEWED_HEAD: a20975688da905312f5757c352a4d3b8f1f693e3 BASE: master @ b70d5f3efa410a7a8acdd3ddaa1778f4dd4e64e9 VERDICT: REQUEST_CHANGES BLOCKING_FINDINGS: 2 BLOCKER: none MUTATIONS: two reviewer lease comments (14955, 14957); one terminal PR review at the pinned head MERGE_PERFORMED: no NEXT_ROLE: author STATE: changes-requested
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #849
issue: #637
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 64115-7a6d42c22cf4
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-timeline
phase: released
candidate_head: a20975688d
target_branch: master
target_branch_sha: b70d5f3efa
last_activity: 2026-07-23T20:20:01Z
expires_at: 2026-07-23T20:30:01Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #849 issue: #637 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 64115-7a6d42c22cf4 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-timeline phase: released candidate_head: a20975688da905312f5757c352a4d3b8f1f693e3 target_branch: master target_branch_sha: b70d5f3efa410a7a8acdd3ddaa1778f4dd4e64e9 last_activity: 2026-07-23T20:20:01Z expires_at: 2026-07-23T20:30:01Z blocker: manual-release
jcwalker3 added 1 commit 2026-07-23 15:47:21 -05:00
Remediates the two blocking findings in review #522 on PR #849.

F1 — the session filter dimension was dead end to end. No source could
produce an event carrying a session identifier, so filter_events dropped
every event whenever session was supplied and the API answered with a
green, empty page. An empty result reads to an operator as "no such
session activity", which is a stronger and false claim.

Each source now declares which filter dimensions its records can actually
carry. The control-plane events table is (event_id, work_item_id,
event_type, message, created_at) and records no session, so that source
declares the session dimension unsupported rather than pretending to
answer it; the dead read of a non-existent session_id column is removed.
A CTH handoff comment declares its own Session field, so the handoff
adapter populates session_id from that declared field — authoritative
source data, never inferred from an actor, work item, or message text.

When no source that ran can carry a requested dimension, load_timeline
refuses with ok=false and a structured error naming the unsupported
filters and the per-source reason, and the route answers 422. A source
that can answer the dimension and simply matched nothing still returns
200 with an honest empty page. Ordering, pagination, and the issue/PR
filters are unchanged.

F2 — evidence_refs bypassed redaction and could emit a credential
verbatim. proof and decision were passed to _extract_evidence_refs before
redaction, and the SHA pattern matched any 7-40 character lowercase hex
run, which is exactly the shape of a Gitea access token.

Redaction now runs first and every derived value is taken from the
redacted text. A commit reference is recognised only where the source
text declares one (commit, head, base, sha, ...), so an undeclared hex
run is never lifted out of prose into a structured field; this also drops
the ordinary-word noise the reviewer noted. Every reference is then
independently revalidated against an allowed shape and a second redaction
pass immediately before serialization, failing closed by dropping
anything unproven and flagging the event sensitive. Actor is redacted for
the same reason, and a secret-shaped session value is dropped rather than
emitted. Legitimate issue, PR, short-SHA and full 40-character SHA
references stay usable.

Tests: 16 added. Session filtering is now driven through the CTH adapter
and the composed load_timeline/API path rather than a hand-built
WorkflowEvent, covering a match, an honest empty result, pagination and
ordering under the filter, the 422 refusal, and the per-source support
declaration. Redaction coverage asserts a synthetic 40-character hex
value (not a real credential) appears nowhere in the complete serialized
payload including evidence_refs, that the independent revalidation drops
unproven references, and that legitimate references still resolve.

pytest tests/test_webui_timeline.py: 42 passed (was 26).
pytest -k webui: 340 passed, 270 subtests passed (was 324).
Full suite: 4607 passed, 12 failed, 6 skipped, 684 subtests passed — the
same 12 failures as the master baseline, none under webui/.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Author
Owner

Author remediation of review #522

Both blocking findings from review #522 (reviewed head a20975688da905312f5757c352a4d3b8f1f693e3) are remediated. New head: 8ba1c5b87ca28c08598c42142a15b1f09eda8983. Review #522 was neither dismissed nor altered; it is now stale against the new head.

Files changed: webui/timeline.py, webui/app.py, tests/test_webui_timeline.py, docs/webui-local-dev.md.

F1 — session filter dimension: remediated

Both halves of the finding are addressed: the dimension is real where a source can carry it, and refused where none can.

The diagnosis was correct — the control-plane events table is (event_id, work_item_id, event_type, message, created_at) and records no session, so adapt_cp_events reading row.get("session_id") was a dead read. That read is removed rather than papered over. Adding a session column would be a control-plane schema migration, which sits outside Phase 1.

Each source now declares which filter dimensions its records can actually carry (_SOURCE_FILTER_SUPPORT), reported per response as supported_filters / unsupported_filters:

Source issue pr session
control_plane yes yes no — no session column exists
gitea_handoff yes yes yes — a CTH comment declares its own Session: field

session_id is populated only from that declared CTH field, which canonical_thread_handoff already parses generically. It is never inferred from an actor, a work item, or message text.

When no source that ran can carry a requested dimension, load_timeline refuses: ok=false with a structured error carrying code: filter_not_supported, unsupported_filters, and a per-source reason. The route answers HTTP 422. A source that can answer the dimension and simply matched nothing still returns 200 with ok=true and an honest empty page. Ordering, pagination, and the issue/PR filters are unchanged.

F2 — evidence_refs redaction boundary: remediated

proof, decision, and next action are redacted first, and every derived value is taken from the redacted text, so extraction can no longer republish what redaction was about to remove.

_SHA_RE was also tightened, as you suggested. A commit reference is now recognised only where the source text declares one (commit, sha, head, base, parent, revision, rev, merge-base), and only the hex run itself is captured. An undeclared hex run is never lifted out of prose into a structured field — which closes the bare-token case that redaction alone cannot catch, since a bare 40-hex run is indistinguishable from a commit SHA to the redaction policy. This also removes the ordinary-word noise you noted: defaced no longer matches.

Extraction is not trusted on its own. _validated_evidence_refs independently rechecks every reference immediately before serialization against an allowed shape plus a second redaction pass, dropping anything unproven and flagging the event sensitive so the drop is visible. actor is redacted for the same reason, and a secret-shaped session value is dropped rather than emitted.

Legitimate references stay usable: #637, #849, short SHAs, and full 40-character SHAs all still resolve — a full SHA survives because extraction only accepts a hex run the text declared as a commit.

Tests

16 tests added (26 to 42 in the focused module).

Session filtering is now driven through the CTH adapter and the composed load_timeline / API path, never a hand-built WorkflowEvent: a match through the adapter, an honest empty result when the dimension is supported, pagination and ordering under the filter, the 422 refusal, and the per-source support declaration. Redaction coverage asserts a synthetic 40-character hex value — fabricated, never a credential — appears nowhere in the complete serialized payload including evidence_refs, that the independent recheck drops unproven references, and that legitimate references still resolve.

  • pytest tests/test_webui_timeline.py42 passed (was 26).
  • pytest -k webui340 passed, 270 subtests passed (was 324).
  • pytest tests/test_canonical_thread_handoff.py tests/test_control_plane_db.py29 passed.
  • pytest -k redact78 passed.
  • Full suite — 4607 passed, 12 failed, 6 skipped, 684 subtests passed.

The 12 full-suite failures are the identical node ids recorded as the master baseline: six in test_commit_payloads.py, two in test_issue_702_review_findings_f1_f6.py, one each in test_mcp_server.py::TestRuntimeProfile::test_whoami_v2_metadata, test_mcp_server.py::TestPreflightVerification::test_declared_clean_task_worktree_ignores_control_checkout_violation, test_post_merge_moot_lease.py, and test_reconciler_supersession_close.py. None touch webui/. This head introduces no new failures.

Provenance

Remediated as jcwalker3 / prgs-author / author on Scaled-Tech-Consulting/Gitea-Tools, master parity in_parity=true mutation_safe=true at b70d5f3efa410a7a8acdd3ddaa1778f4dd4e64e9. The durable issue-#637 lock was recovered through the sanctioned dead-pid path (recorded pid 60203 not alive; claimant, branch, and worktree all matched). Implementation was carried out in the registered worktree branches/issue-637-timeline-model; the control checkout stayed clean on master. The new head was published with gitea_publish_unpublished_issue_branch as a fast-forward from a2097568, verified by read-after-write — no direct remote git mutation. No base merge was performed: the base stays at b70d5f3 and the PR remains mergeable.

Canonical PR State

STATE: awaiting-review
WHO_IS_NEXT: reviewer
NEXT_ACTION: Review PR #849 afresh at head 8ba1c5b87ca28c08598c42142a15b1f09eda8983; verify F1 (session dimension answerable through the CTH adapter, explicit HTTP 422 refusal when no source that ran can carry it, no empty-and-healthy page) and F2 (redaction ahead of extraction, anchored commit references, independent recheck before serialization, synthetic 40-hex value absent from the whole payload); confirm the 12 full-suite failures match the master baseline; issue a verdict; stop
NEXT_PROMPT:

Review PR #849 for issue #637 as prgs-reviewer pinned to head 8ba1c5b87ca28c08598c42142a15b1f09eda8983. Verify the two remediated findings from review #522. F1: the session filter is answerable through the gitea_handoff adapter via the declared CTH Session field, control_plane declares the dimension unsupported, and load_timeline plus the route refuse with ok=false and HTTP 422 rather than returning a green empty page. F2: proof and decision are redacted before evidence extraction, commit references are recognised only where the text declares one, and evidence_refs are independently rechecked before serialization so a synthetic 40-hex value reaches no part of the payload. Run pytest tests/test_webui_timeline.py and pytest -k webui, confirm the 12 full-suite failures match the master baseline, issue a verdict, and stop.

WHAT_HAPPENED: Author remediated both blocking findings of review #522, added 16 regression tests, committed 8ba1c5b8 on feat/issue-637-timeline-model as a fast-forward from the reviewed head a2097568, and published it through the native author path with read-after-write verification.
WHY: Acceptance criterion 3 required the session dimension to be either functional or honestly refused, and acceptance criterion 4 required evidence_refs to sit inside the redaction boundary rather than upstream of it.
RELATED_PRS: #849 (this PR, issue #637). Neighbouring web-console children touched by neither this head nor this session: #818 (issue #638) registers an HTML stub at /timeline while this PR registers the JSON route at /api/v1/timeline; #838 (issue #636) adds /api/v1/inventory and will need a union merge in the webui/app.py import block whichever lands second.
ISSUE: #637
HEAD_SHA: 8ba1c5b87c
PRIOR_HEAD_SHA: a20975688d
REVIEW_STATUS: changes-requested at the prior head; no review verdict exists at the current head
MERGE_READY: no
BLOCKERS: none
VALIDATION: pytest tests/test_webui_timeline.py 42 passed; pytest -k webui 340 passed with 270 subtests passed; full suite 4607 passed with the 12 baseline failures unchanged; master parity in_parity=true mutation_safe=true at b70d5f3efa
LAST_UPDATED_BY: jcwalker3 / prgs-author

[THREAD STATE LEDGER]

What is true now:
PR #849 remains in open state on branch feat/issue-637-timeline-model against base master. The live head is 8ba1c5b87c, confirmed by read-after-write. Review #522 remains recorded and undismissed, now stale against the current head.
Server-side decision state: review #522 by sysadmin carries a REQUEST_CHANGES verdict recorded at the prior head a20975688da905312f5757c352a4d3b8f1f693e3; no review verdict of any kind exists at the current head 8ba1c5b87c.
Local verdict/state: both blocking findings remediated and published; an author holds no authority to issue a review verdict, so none was formed.

What changed:
Four files changed on a single commit 8ba1c5b8, a fast-forward from the reviewed head a2097568. F1: per-source filter-dimension declarations, session_id populated from the declared CTH Session field, the dead control-plane session read removed, and an explicit ok=false / HTTP 422 refusal when no source that ran can carry a requested dimension. F2: redaction moved ahead of evidence extraction, commit references recognised only where declared, and an independent recheck of every reference before serialization. 16 regression tests added. No base merge, no rebase, no force-push.

What is blocked:
Blocker classification: no blocker
Nothing obstructs the next step. The PR reports mergeable against an unchanged base b70d5f3, and the 12 full-suite failures match the master baseline with none under webui/.

Who/what acts next:
Next actor: reviewer — a fresh standalone prgs-reviewer session pinned to head 8ba1c5b87c.
Required action: independently review the two remediations against issue #637 acceptance criteria 3 and 4, run the focused and webui suites, and issue a verdict at the current head.
Do not do: do not merge PR #849, since no approval verdict exists at the current head; do not dismiss, edit, or reinterpret review #522; do not modify PRs #846, #838, #818, #795, or #794; do not rebase or force-push this branch; do not treat this author comment as a review verdict.

ROLE: author
IDENTITY: jcwalker3
PROFILE: prgs-author
REMOTE: prgs
REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
PR: #849
ISSUE: #637
PRIOR_HEAD: a20975688d
NEW_HEAD: 8ba1c5b87c
BASE: master @ b70d5f3efa
BLOCKING_FINDINGS_REMEDIATED: 2
MUTATIONS: one local commit published as a fast-forward branch head; this comment
REVIEW_DISMISSED: no
MERGE_PERFORMED: no
NEXT_ROLE: reviewer
STATE: awaiting-review

## Author remediation of review #522 Both blocking findings from review #522 (reviewed head `a20975688da905312f5757c352a4d3b8f1f693e3`) are remediated. New head: **`8ba1c5b87ca28c08598c42142a15b1f09eda8983`**. Review #522 was neither dismissed nor altered; it is now stale against the new head. Files changed: `webui/timeline.py`, `webui/app.py`, `tests/test_webui_timeline.py`, `docs/webui-local-dev.md`. ### F1 — session filter dimension: remediated Both halves of the finding are addressed: the dimension is real where a source can carry it, and refused where none can. The diagnosis was correct — the control-plane `events` table is `(event_id, work_item_id, event_type, message, created_at)` and records no session, so `adapt_cp_events` reading `row.get("session_id")` was a dead read. That read is removed rather than papered over. Adding a session column would be a control-plane schema migration, which sits outside Phase 1. Each source now declares which filter dimensions its records can actually carry (`_SOURCE_FILTER_SUPPORT`), reported per response as `supported_filters` / `unsupported_filters`: | Source | issue | pr | session | |---|---|---|---| | `control_plane` | yes | yes | **no** — no session column exists | | `gitea_handoff` | yes | yes | yes — a CTH comment declares its own `Session:` field | `session_id` is populated only from that declared CTH field, which `canonical_thread_handoff` already parses generically. It is never inferred from an actor, a work item, or message text. When no source that ran can carry a requested dimension, `load_timeline` refuses: `ok=false` with a structured `error` carrying `code: filter_not_supported`, `unsupported_filters`, and a per-source reason. The route answers **HTTP 422**. A source that *can* answer the dimension and simply matched nothing still returns 200 with `ok=true` and an honest empty page. Ordering, pagination, and the issue/PR filters are unchanged. ### F2 — evidence_refs redaction boundary: remediated `proof`, `decision`, and `next action` are redacted **first**, and every derived value is taken from the redacted text, so extraction can no longer republish what redaction was about to remove. `_SHA_RE` was also tightened, as you suggested. A commit reference is now recognised only where the source text declares one (`commit`, `sha`, `head`, `base`, `parent`, `revision`, `rev`, `merge-base`), and only the hex run itself is captured. An undeclared hex run is never lifted out of prose into a structured field — which closes the bare-token case that redaction alone cannot catch, since a bare 40-hex run is indistinguishable from a commit SHA to the redaction policy. This also removes the ordinary-word noise you noted: `defaced` no longer matches. Extraction is not trusted on its own. `_validated_evidence_refs` independently rechecks every reference immediately before serialization against an allowed shape plus a second redaction pass, dropping anything unproven and flagging the event `sensitive` so the drop is visible. `actor` is redacted for the same reason, and a secret-shaped session value is dropped rather than emitted. Legitimate references stay usable: `#637`, `#849`, short SHAs, and full 40-character SHAs all still resolve — a full SHA survives because extraction only accepts a hex run the text declared as a commit. ### Tests 16 tests added (26 to 42 in the focused module). Session filtering is now driven through the CTH adapter and the composed `load_timeline` / API path, never a hand-built `WorkflowEvent`: a match through the adapter, an honest empty result when the dimension is supported, pagination and ordering under the filter, the 422 refusal, and the per-source support declaration. Redaction coverage asserts a synthetic 40-character hex value — fabricated, never a credential — appears nowhere in the complete serialized payload including `evidence_refs`, that the independent recheck drops unproven references, and that legitimate references still resolve. - `pytest tests/test_webui_timeline.py` — **42 passed** (was 26). - `pytest -k webui` — **340 passed, 270 subtests passed** (was 324). - `pytest tests/test_canonical_thread_handoff.py tests/test_control_plane_db.py` — **29 passed**. - `pytest -k redact` — **78 passed**. - Full suite — **4607 passed, 12 failed, 6 skipped, 684 subtests passed**. The 12 full-suite failures are the identical node ids recorded as the master baseline: six in `test_commit_payloads.py`, two in `test_issue_702_review_findings_f1_f6.py`, one each in `test_mcp_server.py::TestRuntimeProfile::test_whoami_v2_metadata`, `test_mcp_server.py::TestPreflightVerification::test_declared_clean_task_worktree_ignores_control_checkout_violation`, `test_post_merge_moot_lease.py`, and `test_reconciler_supersession_close.py`. None touch `webui/`. This head introduces no new failures. ### Provenance Remediated as `jcwalker3` / `prgs-author` / author on `Scaled-Tech-Consulting/Gitea-Tools`, master parity `in_parity=true` `mutation_safe=true` at `b70d5f3efa410a7a8acdd3ddaa1778f4dd4e64e9`. The durable issue-#637 lock was recovered through the sanctioned dead-pid path (recorded pid 60203 not alive; claimant, branch, and worktree all matched). Implementation was carried out in the registered worktree `branches/issue-637-timeline-model`; the control checkout stayed clean on master. The new head was published with `gitea_publish_unpublished_issue_branch` as a fast-forward from `a2097568`, verified by read-after-write — no direct remote git mutation. No base merge was performed: the base stays at `b70d5f3` and the PR remains mergeable. ## Canonical PR State STATE: awaiting-review WHO_IS_NEXT: reviewer NEXT_ACTION: Review PR #849 afresh at head 8ba1c5b87ca28c08598c42142a15b1f09eda8983; verify F1 (session dimension answerable through the CTH adapter, explicit HTTP 422 refusal when no source that ran can carry it, no empty-and-healthy page) and F2 (redaction ahead of extraction, anchored commit references, independent recheck before serialization, synthetic 40-hex value absent from the whole payload); confirm the 12 full-suite failures match the master baseline; issue a verdict; stop NEXT_PROMPT: ```text Review PR #849 for issue #637 as prgs-reviewer pinned to head 8ba1c5b87ca28c08598c42142a15b1f09eda8983. Verify the two remediated findings from review #522. F1: the session filter is answerable through the gitea_handoff adapter via the declared CTH Session field, control_plane declares the dimension unsupported, and load_timeline plus the route refuse with ok=false and HTTP 422 rather than returning a green empty page. F2: proof and decision are redacted before evidence extraction, commit references are recognised only where the text declares one, and evidence_refs are independently rechecked before serialization so a synthetic 40-hex value reaches no part of the payload. Run pytest tests/test_webui_timeline.py and pytest -k webui, confirm the 12 full-suite failures match the master baseline, issue a verdict, and stop. ``` WHAT_HAPPENED: Author remediated both blocking findings of review #522, added 16 regression tests, committed 8ba1c5b8 on feat/issue-637-timeline-model as a fast-forward from the reviewed head a2097568, and published it through the native author path with read-after-write verification. WHY: Acceptance criterion 3 required the session dimension to be either functional or honestly refused, and acceptance criterion 4 required evidence_refs to sit inside the redaction boundary rather than upstream of it. RELATED_PRS: #849 (this PR, issue #637). Neighbouring web-console children touched by neither this head nor this session: #818 (issue #638) registers an HTML stub at /timeline while this PR registers the JSON route at /api/v1/timeline; #838 (issue #636) adds /api/v1/inventory and will need a union merge in the webui/app.py import block whichever lands second. ISSUE: #637 HEAD_SHA: 8ba1c5b87ca28c08598c42142a15b1f09eda8983 PRIOR_HEAD_SHA: a20975688da905312f5757c352a4d3b8f1f693e3 REVIEW_STATUS: changes-requested at the prior head; no review verdict exists at the current head MERGE_READY: no BLOCKERS: none VALIDATION: pytest tests/test_webui_timeline.py 42 passed; pytest -k webui 340 passed with 270 subtests passed; full suite 4607 passed with the 12 baseline failures unchanged; master parity in_parity=true mutation_safe=true at b70d5f3efa410a7a8acdd3ddaa1778f4dd4e64e9 LAST_UPDATED_BY: jcwalker3 / prgs-author [THREAD STATE LEDGER] What is true now: PR #849 remains in open state on branch feat/issue-637-timeline-model against base master. The live head is 8ba1c5b87ca28c08598c42142a15b1f09eda8983, confirmed by read-after-write. Review #522 remains recorded and undismissed, now stale against the current head. Server-side decision state: review #522 by sysadmin carries a REQUEST_CHANGES verdict recorded at the prior head a20975688da905312f5757c352a4d3b8f1f693e3; no review verdict of any kind exists at the current head 8ba1c5b87ca28c08598c42142a15b1f09eda8983. Local verdict/state: both blocking findings remediated and published; an author holds no authority to issue a review verdict, so none was formed. What changed: Four files changed on a single commit 8ba1c5b8, a fast-forward from the reviewed head a2097568. F1: per-source filter-dimension declarations, session_id populated from the declared CTH Session field, the dead control-plane session read removed, and an explicit ok=false / HTTP 422 refusal when no source that ran can carry a requested dimension. F2: redaction moved ahead of evidence extraction, commit references recognised only where declared, and an independent recheck of every reference before serialization. 16 regression tests added. No base merge, no rebase, no force-push. What is blocked: Blocker classification: no blocker Nothing obstructs the next step. The PR reports mergeable against an unchanged base b70d5f3, and the 12 full-suite failures match the master baseline with none under webui/. Who/what acts next: Next actor: reviewer — a fresh standalone prgs-reviewer session pinned to head 8ba1c5b87ca28c08598c42142a15b1f09eda8983. Required action: independently review the two remediations against issue #637 acceptance criteria 3 and 4, run the focused and webui suites, and issue a verdict at the current head. Do not do: do not merge PR #849, since no approval verdict exists at the current head; do not dismiss, edit, or reinterpret review #522; do not modify PRs #846, #838, #818, #795, or #794; do not rebase or force-push this branch; do not treat this author comment as a review verdict. ROLE: author IDENTITY: jcwalker3 PROFILE: prgs-author REMOTE: prgs REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools PR: #849 ISSUE: #637 PRIOR_HEAD: a20975688da905312f5757c352a4d3b8f1f693e3 NEW_HEAD: 8ba1c5b87ca28c08598c42142a15b1f09eda8983 BASE: master @ b70d5f3efa410a7a8acdd3ddaa1778f4dd4e64e9 BLOCKING_FINDINGS_REMEDIATED: 2 MUTATIONS: one local commit published as a fast-forward branch head; this comment REVIEW_DISMISSED: no MERGE_PERFORMED: no NEXT_ROLE: reviewer STATE: awaiting-review
jcwalker3 added 1 commit 2026-07-23 16:21:13 -05:00
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #849
issue: #637
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 63651-4478b80e082c
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-f2
phase: claimed
candidate_head: 2d95e0fcc6
target_branch: master
target_branch_sha: 188e83c4d6
last_activity: 2026-07-23T22:25:43Z
expires_at: 2026-07-23T22:35:43Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #849 issue: #637 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 63651-4478b80e082c worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-f2 phase: claimed candidate_head: 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863 target_branch: master target_branch_sha: 188e83c4d69bfb959c91b1f8e766ab8a0656d92d last_activity: 2026-07-23T22:25:43Z expires_at: 2026-07-23T22:35:43Z blocker: none
sysadmin requested changes 2026-07-23 17:27:51 -05:00
Dismissed
sysadmin left a comment
Owner

REQUEST_CHANGES — reviewed at head 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863.

Reviewer sysadmin / prgs-reviewer; author jcwalker3. Reviewed from a detached worktree pinned to the exact head, clean tree.

F1 is resolved. F2 is not, and the residual is a different field from the one reported in review #522: the evidence_refs path is genuinely fixed, but event_type was never inside the redaction boundary and still is not.

Note on the head: the change from 8ba1c5b8 to 2d95e0fc is a base-sync merge only. git diff 8ba1c5b8 2d95e0fc -- webui/timeline.py webui/app.py tests/test_webui_timeline.py is empty, so this head carries no further remediation.


F2 remains incomplete: event_type bypasses the response-redaction boundary.

Both timeline adapters can emit externally influenced event_type values without applying the fail-closed redaction or strict validation used for other serialized fields.

In the CTH path, _HEADING_RE accepts arbitrary heading text and parse_cth_comment does not constrain cth_type to the authoritative CTH_TYPES set. adapt_cth_comments then constructs and serializes event_type from that unvalidated value.

The control-plane adapter likewise emits its stored event_type directly without applying an equivalent validation or redaction boundary.

Using a synthetic secret-shaped 40-character hexadecimal canary, the value is redacted when supplied through message content but survives when supplied through event_type. The unsafe value therefore remains in the complete serialized payload. Existing payload-level coverage does not exercise the canary through both event_type paths.

Where the boundary is missing

canonical_thread_handoff.py:39_HEADING_RE = re.compile(r"^##\s+CTH:\s*(.+?)\s*$", ...). The capture group is unconstrained free text.

canonical_thread_handoff.py:95parse_cth_comment returns cth_type = heading.group(1).strip() with no membership check. The write path format_cth_body does validate against CTH_TYPES (lines 62-66), and assess_cth_comment validates (lines 121-124), but adapt_cth_comments calls parse_cth_comment, which does not. The contract is enforced on write and on assess, and skipped on the read path the timeline actually uses.

webui/timeline.py:384event_type=f"handoff:{cth_type}". On the same event, decision, proof, next action, actor, role, message, and session_id each pass through _redact or _safe_session_id; cth_type passes through neither.

webui/timeline.py:311,321event_type = (row.get("event_type") or "").strip() then event_type=event_type. Only whitespace is stripped. On that same event message=_redact(row.get("message")) at line 333.

webui/timeline.py:154-171to_dict serializes event_type verbatim, so the value reaches the API payload unchanged.

Reproduction at this head

Executed against the pinned worktree with the repository venv, using a fabricated secret-shaped 40-character hex canary — never a real credential.

Control-plane adapter, the identical canary supplied through two fields of one record:

event_type : <canary>
message    : deploy token [REDACTED]
canary in message   : False
canary in event_type: True
canary in FULL serialized payload: True

CTH adapter, canary supplied through the heading:

event_type : handoff:<canary>
canary in event_type: True
canary in FULL serialized payload: True

The control-plane case is the decisive one. The same string, in the same record, at the same instant, is treated as credential material by the redaction policy on message and emitted verbatim on event_type. The policy already classifies the value correctly; event_type simply never consults it.

Coverage gap

Every event_type in tests/test_webui_timeline.py is a benign literal: allocation, pr.opened, lease.renew, handoff:Author Handoff, a/b/c. The redaction test at line 148 seeds a secret-shaped value into message only. No test drives a canary through either event_type path, which is why this survived the F1/F2 remediation round.


Required remediation:

  1. Strictly validate or safely normalize CTH types against the authoritative CTH_TYPES contract before emission.
  2. Apply fail-closed validation or redaction to stored control-plane event types.
  3. Audit all externally influenced serialized string fields for equivalent bypasses.
  4. Extend complete-payload regression coverage by supplying the synthetic canary through both the CTH heading and control-plane event_type paths and asserting that it appears nowhere in the serialized response.
  5. Preserve legitimate declared event types and legitimate issue, PR, evidence, and authoritative commit references.
  6. Synchronize the branch with current master through the canonical author workflow because branch protection requires a current base.

Point 5 is achievable without loss. handoff:Author Handoff (asserted at test line 134) is a declared member of CTH_TYPES, and control-plane types such as allocation, pr.opened, and lease.renew are ordinary identifiers that any reasonable validation preserves. The evidence-reference behaviour added for F2 already preserves #637, #849, short SHAs, and full 40-character SHAs, and needs no change.

F1 — confirmed resolved, not reopened

Verified present and passing at this head; reported for completeness only, with no new finding.

  • _SOURCE_FILTER_SUPPORT (webui/timeline.py:79) declares per-source filter dimensions, surfaced as supported_filters / unsupported_filters.
  • session_id is populated only from the declared CTH Session: field via _safe_session_id, which drops a value that redaction alters or that is a bare secret-shaped hex run.
  • The dead control-plane session read is removed, with the reason recorded inline at lines 326-332.
  • load_timeline refuses with code: filter_not_supported (line 757) and the route answers HTTP 422 (webui/app.py:508) rather than returning a green empty page.

Also verified as correct at this head

  • Redaction now runs ahead of evidence extraction; decision, proof, and next action are redacted first and every derived value is taken from the redacted text.
  • _validated_evidence_refs independently rechecks each reference before serialization, dropping anything unproven and flagging the event sensitive so the drop is visible.
  • Versioned TIMELINE_SCHEMA_VERSION; WorkflowEvent frozen; read_cp_events opens mode=ro; both adapters total; ordering (timestamp, source_rank, event_key) with missing timestamps last; pagination clamped.
  • Scope remains disciplined and purely additive; no bleed into #636 or #638.

Tests run by this review

From the pinned worktree, repository venv:

  • pytest tests/test_webui_timeline.py42 passed.
  • pytest -k webui340 passed, 4314 deselected, 270 subtests passed.
  • pytest -k redact78 passed.
  • pytest tests/test_canonical_thread_handoff.py tests/test_control_plane_db.py29 passed.

All four match the author's reported figures exactly. The baseline is unchanged, so the full suite was not rerun; the previously recorded 12 failures stand as the master baseline and none touch webui/. The defect above is not caught by any of these suites and was reproduced by direct execution at this head, quoted inline.

The remaining work is narrow: constrain one parsed value against a contract the module already defines, apply the existing boundary to one stored field, and seed the canary through both paths in the payload-level test.

Base synchronization

The branch is 8 commits behind live base 188e83c4d69bfb959c91b1f8e766ab8a0656d92d and branch protection requires a current base. The PR reports mergeable with no conflicts; status checks are not required for this base. Base synchronization is author work through the canonical workflow and was not performed by this review.

NATIVE_REVIEW_PROOF: native MCP reviewer submission by sysadmin / prgs-reviewer at head 2d95e0fcc6 via gitea_submit_pr_review, workflow review-merge-pr hash 263d0a6cb8a6, reviewer lease comment 15066, session 63651-4478b80e082c.

Canonical PR State

STATE: changes-requested
WHO_IS_NEXT: author
NEXT_ACTION: Author constrains CTH types against the authoritative CTH_TYPES contract on the parse path used by adapt_cth_comments, applies fail-closed validation or redaction to stored control-plane event_type, audits the remaining externally influenced serialized string fields, adds complete-payload regression coverage seeding a synthetic canary through both the CTH heading and control-plane event_type paths, preserves legitimate declared event types and legitimate references, synchronizes the branch with current master through the canonical author workflow, and returns PR #849 for a fresh review at the new head
NEXT_PROMPT:

Remediate PR #849 for issue #637 as prgs-author at head 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863. F2 residual: event_type bypasses the response-redaction boundary in both timeline adapters. In canonical_thread_handoff.py, _HEADING_RE captures arbitrary heading text and parse_cth_comment returns cth_type without checking membership in CTH_TYPES, so webui/timeline.py line 384 builds event_type from an unvalidated value; in webui/timeline.py lines 311 and 321 the control-plane adapter emits its stored event_type with only whitespace stripped, and to_dict serializes both verbatim. A synthetic secret-shaped 40-character hex canary is redacted through message but survives through event_type on the same record and reaches the complete serialized payload. Constrain or safely normalize CTH types against CTH_TYPES on the parse path the adapter uses, apply fail-closed validation or redaction to stored control-plane event types, audit the remaining externally influenced serialized string fields for equivalent bypasses, and add payload-level regression coverage seeding the canary through both the CTH heading and control-plane event_type paths asserting it appears nowhere in the serialized response. Preserve legitimate declared event types such as handoff:Author Handoff, allocation, pr.opened, and lease.renew, and legitimate issue, PR, evidence, and authoritative commit references. Then synchronize the branch with current master 188e83c4d69bfb959c91b1f8e766ab8a0656d92d through the canonical author workflow because branch protection requires a current base. Run pytest tests/test_webui_timeline.py, pytest -k webui, and pytest -k redact, push, hand back to reviewer, and stop.

WHAT_HAPPENED: Reviewer sysadmin reviewed PR #849 at head 2d95e0fcc6 from a clean detached worktree pinned to that exact head, confirmed the 8ba1c5b8 to 2d95e0fc change is a base-sync merge that altered no timeline file, confirmed F1 resolved, reproduced the residual F2 event_type redaction bypass in both adapters by direct execution with a synthetic canary, ran the focused, webui, redaction, and canonical-thread suites, and submitted one terminal REQUEST_CHANGES review. No merge was performed, no branch was modified, and review 522 was not dismissed.
WHY: Acceptance criterion 4 and issue #637's requirement to redact secrets and fail closed remain unmet because externally influenced event_type values reach the serialized payload without passing the redaction or validation boundary that every other free-text field on the same event passes.
RELATED_PRS: #849 (this PR, issue #637). Neighbouring web-console children untouched by this review: #818 (issue #638) registers an HTML stub at /timeline while this PR registers the JSON route at /api/v1/timeline; #838 (issue #636) adds /api/v1/inventory and will need a union merge in the webui/app.py import block whichever lands second.
ISSUE: #637
HEAD_SHA: 2d95e0fcc6
PRIOR_REVIEWED_HEAD_SHA: a20975688d
REVIEW_STATUS: REQUEST_CHANGES
MERGE_READY: no
BLOCKERS: F2 residual, externally influenced event_type reaches the serialized payload without the fail-closed redaction or validation boundary in both timeline adapters; branch is 8 commits behind live base and branch protection requires a current base
VALIDATION: pytest tests/test_webui_timeline.py 42 passed; pytest -k webui 340 passed with 270 subtests passed; pytest -k redact 78 passed; pytest tests/test_canonical_thread_handoff.py tests/test_control_plane_db.py 29 passed; F2 residual reproduced by direct execution at this head with output quoted in this review; master parity in_parity=true mutation_safe=true restart_required=false at 188e83c4d6
LAST_UPDATED_BY: sysadmin / prgs-reviewer

[THREAD STATE LEDGER]

What is true now:
PR #849 remains open on branch feat/issue-637-timeline-model against base master. The live head is 2d95e0fcc6, pinned from assess_pr_sync_status and get_pr_review_feedback immediately before submission. The branch is 8 commits behind live base 188e83c4d6, reports mergeable with no conflicts, and status checks are not required for this base.
Server-side decision state: review 522 by sysadmin carries a REQUEST_CHANGES verdict recorded at the former head a20975688d and remains undismissed and stale. This review adds one terminal REQUEST_CHANGES verdict at the current head.
Local verdict/state: F1 confirmed resolved at this head; F2 confirmed incomplete at this head with a residual event_type bypass reproduced in both adapters.

What changed:
No repository content changed. This session created a detached review worktree at the pinned head, acquired a reviewer lease, reproduced the residual defect, ran four test selections, and submitted one review verdict.

What is blocked:
Blocker classification: no blocker
Nothing obstructs the next step. The author holds every capability required to remediate the finding and to synchronize the base.

Who/what acts next:
Next actor: author — a fresh standalone prgs-author session on issue #637.
Required action: constrain CTH types against CTH_TYPES on the parse path the timeline adapter uses, apply fail-closed validation or redaction to stored control-plane event types, audit remaining externally influenced serialized string fields, add payload-level canary coverage through both event_type paths, preserve legitimate declared types and references, and synchronize the branch with current master through the canonical author workflow.
Do not do: do not merge PR #849, since no approval verdict exists at any head; do not dismiss, edit, or reinterpret review 522 or this review; do not modify PRs #846, #838, #818, #795, or #794; do not force-push this branch.

ROLE: reviewer
IDENTITY: sysadmin
PROFILE: prgs-reviewer
REMOTE: prgs
REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
PR: #849
ISSUE: #637
REVIEWED_HEAD: 2d95e0fcc6
BASE: master @ 188e83c4d6
VERDICT: REQUEST_CHANGES
BLOCKING_FINDINGS: 1
BLOCKER: none
MUTATIONS: one reviewer lease comment (15066); one terminal PR review at the pinned head
MERGE_PERFORMED: no
REVIEW_DISMISSED: no
NEXT_ROLE: author
STATE: changes-requested

REQUEST_CHANGES — reviewed at head `2d95e0fcc6dafddd7a11f9a6565ae57b93f69863`. Reviewer `sysadmin` / `prgs-reviewer`; author `jcwalker3`. Reviewed from a detached worktree pinned to the exact head, clean tree. F1 is resolved. F2 is not, and the residual is a different field from the one reported in review #522: the `evidence_refs` path is genuinely fixed, but `event_type` was never inside the redaction boundary and still is not. Note on the head: the change from `8ba1c5b8` to `2d95e0fc` is a base-sync merge only. `git diff 8ba1c5b8 2d95e0fc -- webui/timeline.py webui/app.py tests/test_webui_timeline.py` is empty, so this head carries no further remediation. --- ## F2 remains incomplete: event_type bypasses the response-redaction boundary. Both timeline adapters can emit externally influenced event_type values without applying the fail-closed redaction or strict validation used for other serialized fields. In the CTH path, _HEADING_RE accepts arbitrary heading text and parse_cth_comment does not constrain cth_type to the authoritative CTH_TYPES set. adapt_cth_comments then constructs and serializes event_type from that unvalidated value. The control-plane adapter likewise emits its stored event_type directly without applying an equivalent validation or redaction boundary. Using a synthetic secret-shaped 40-character hexadecimal canary, the value is redacted when supplied through message content but survives when supplied through event_type. The unsafe value therefore remains in the complete serialized payload. Existing payload-level coverage does not exercise the canary through both event_type paths. ### Where the boundary is missing `canonical_thread_handoff.py:39` — `_HEADING_RE = re.compile(r"^##\s+CTH:\s*(.+?)\s*$", ...)`. The capture group is unconstrained free text. `canonical_thread_handoff.py:95` — `parse_cth_comment` returns `cth_type = heading.group(1).strip()` with no membership check. The write path `format_cth_body` does validate against `CTH_TYPES` (lines 62-66), and `assess_cth_comment` validates (lines 121-124), but `adapt_cth_comments` calls `parse_cth_comment`, which does not. The contract is enforced on write and on assess, and skipped on the read path the timeline actually uses. `webui/timeline.py:384` — `event_type=f"handoff:{cth_type}"`. On the same event, `decision`, `proof`, `next action`, `actor`, `role`, `message`, and `session_id` each pass through `_redact` or `_safe_session_id`; `cth_type` passes through neither. `webui/timeline.py:311,321` — `event_type = (row.get("event_type") or "").strip()` then `event_type=event_type`. Only whitespace is stripped. On that same event `message=_redact(row.get("message"))` at line 333. `webui/timeline.py:154-171` — `to_dict` serializes `event_type` verbatim, so the value reaches the API payload unchanged. ### Reproduction at this head Executed against the pinned worktree with the repository venv, using a fabricated secret-shaped 40-character hex canary — never a real credential. Control-plane adapter, the identical canary supplied through two fields of one record: ``` event_type : <canary> message : deploy token [REDACTED] canary in message : False canary in event_type: True canary in FULL serialized payload: True ``` CTH adapter, canary supplied through the heading: ``` event_type : handoff:<canary> canary in event_type: True canary in FULL serialized payload: True ``` The control-plane case is the decisive one. The same string, in the same record, at the same instant, is treated as credential material by the redaction policy on `message` and emitted verbatim on `event_type`. The policy already classifies the value correctly; `event_type` simply never consults it. ### Coverage gap Every `event_type` in `tests/test_webui_timeline.py` is a benign literal: `allocation`, `pr.opened`, `lease.renew`, `handoff:Author Handoff`, `a`/`b`/`c`. The redaction test at line 148 seeds a secret-shaped value into `message` only. No test drives a canary through either `event_type` path, which is why this survived the F1/F2 remediation round. --- ## Required remediation: 1. Strictly validate or safely normalize CTH types against the authoritative CTH_TYPES contract before emission. 2. Apply fail-closed validation or redaction to stored control-plane event types. 3. Audit all externally influenced serialized string fields for equivalent bypasses. 4. Extend complete-payload regression coverage by supplying the synthetic canary through both the CTH heading and control-plane event_type paths and asserting that it appears nowhere in the serialized response. 5. Preserve legitimate declared event types and legitimate issue, PR, evidence, and authoritative commit references. 6. Synchronize the branch with current master through the canonical author workflow because branch protection requires a current base. Point 5 is achievable without loss. `handoff:Author Handoff` (asserted at test line 134) is a declared member of `CTH_TYPES`, and control-plane types such as `allocation`, `pr.opened`, and `lease.renew` are ordinary identifiers that any reasonable validation preserves. The evidence-reference behaviour added for F2 already preserves `#637`, `#849`, short SHAs, and full 40-character SHAs, and needs no change. ## F1 — confirmed resolved, not reopened Verified present and passing at this head; reported for completeness only, with no new finding. - `_SOURCE_FILTER_SUPPORT` (`webui/timeline.py:79`) declares per-source filter dimensions, surfaced as `supported_filters` / `unsupported_filters`. - `session_id` is populated only from the declared CTH `Session:` field via `_safe_session_id`, which drops a value that redaction alters or that is a bare secret-shaped hex run. - The dead control-plane session read is removed, with the reason recorded inline at lines 326-332. - `load_timeline` refuses with `code: filter_not_supported` (line 757) and the route answers HTTP 422 (`webui/app.py:508`) rather than returning a green empty page. ## Also verified as correct at this head - Redaction now runs ahead of evidence extraction; `decision`, `proof`, and `next action` are redacted first and every derived value is taken from the redacted text. - `_validated_evidence_refs` independently rechecks each reference before serialization, dropping anything unproven and flagging the event `sensitive` so the drop is visible. - Versioned `TIMELINE_SCHEMA_VERSION`; `WorkflowEvent` frozen; `read_cp_events` opens `mode=ro`; both adapters total; ordering `(timestamp, source_rank, event_key)` with missing timestamps last; pagination clamped. - Scope remains disciplined and purely additive; no bleed into #636 or #638. ## Tests run by this review From the pinned worktree, repository venv: - `pytest tests/test_webui_timeline.py` — **42 passed**. - `pytest -k webui` — **340 passed, 4314 deselected, 270 subtests passed**. - `pytest -k redact` — **78 passed**. - `pytest tests/test_canonical_thread_handoff.py tests/test_control_plane_db.py` — **29 passed**. All four match the author's reported figures exactly. The baseline is unchanged, so the full suite was not rerun; the previously recorded 12 failures stand as the master baseline and none touch `webui/`. The defect above is not caught by any of these suites and was reproduced by direct execution at this head, quoted inline. The remaining work is narrow: constrain one parsed value against a contract the module already defines, apply the existing boundary to one stored field, and seed the canary through both paths in the payload-level test. ## Base synchronization The branch is 8 commits behind live base `188e83c4d69bfb959c91b1f8e766ab8a0656d92d` and branch protection requires a current base. The PR reports mergeable with no conflicts; status checks are not required for this base. Base synchronization is author work through the canonical workflow and was not performed by this review. NATIVE_REVIEW_PROOF: native MCP reviewer submission by sysadmin / prgs-reviewer at head 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863 via gitea_submit_pr_review, workflow review-merge-pr hash 263d0a6cb8a6, reviewer lease comment 15066, session 63651-4478b80e082c. ## Canonical PR State STATE: changes-requested WHO_IS_NEXT: author NEXT_ACTION: Author constrains CTH types against the authoritative CTH_TYPES contract on the parse path used by adapt_cth_comments, applies fail-closed validation or redaction to stored control-plane event_type, audits the remaining externally influenced serialized string fields, adds complete-payload regression coverage seeding a synthetic canary through both the CTH heading and control-plane event_type paths, preserves legitimate declared event types and legitimate references, synchronizes the branch with current master through the canonical author workflow, and returns PR #849 for a fresh review at the new head NEXT_PROMPT: ```text Remediate PR #849 for issue #637 as prgs-author at head 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863. F2 residual: event_type bypasses the response-redaction boundary in both timeline adapters. In canonical_thread_handoff.py, _HEADING_RE captures arbitrary heading text and parse_cth_comment returns cth_type without checking membership in CTH_TYPES, so webui/timeline.py line 384 builds event_type from an unvalidated value; in webui/timeline.py lines 311 and 321 the control-plane adapter emits its stored event_type with only whitespace stripped, and to_dict serializes both verbatim. A synthetic secret-shaped 40-character hex canary is redacted through message but survives through event_type on the same record and reaches the complete serialized payload. Constrain or safely normalize CTH types against CTH_TYPES on the parse path the adapter uses, apply fail-closed validation or redaction to stored control-plane event types, audit the remaining externally influenced serialized string fields for equivalent bypasses, and add payload-level regression coverage seeding the canary through both the CTH heading and control-plane event_type paths asserting it appears nowhere in the serialized response. Preserve legitimate declared event types such as handoff:Author Handoff, allocation, pr.opened, and lease.renew, and legitimate issue, PR, evidence, and authoritative commit references. Then synchronize the branch with current master 188e83c4d69bfb959c91b1f8e766ab8a0656d92d through the canonical author workflow because branch protection requires a current base. Run pytest tests/test_webui_timeline.py, pytest -k webui, and pytest -k redact, push, hand back to reviewer, and stop. ``` WHAT_HAPPENED: Reviewer sysadmin reviewed PR #849 at head 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863 from a clean detached worktree pinned to that exact head, confirmed the 8ba1c5b8 to 2d95e0fc change is a base-sync merge that altered no timeline file, confirmed F1 resolved, reproduced the residual F2 event_type redaction bypass in both adapters by direct execution with a synthetic canary, ran the focused, webui, redaction, and canonical-thread suites, and submitted one terminal REQUEST_CHANGES review. No merge was performed, no branch was modified, and review 522 was not dismissed. WHY: Acceptance criterion 4 and issue #637's requirement to redact secrets and fail closed remain unmet because externally influenced event_type values reach the serialized payload without passing the redaction or validation boundary that every other free-text field on the same event passes. RELATED_PRS: #849 (this PR, issue #637). Neighbouring web-console children untouched by this review: #818 (issue #638) registers an HTML stub at /timeline while this PR registers the JSON route at /api/v1/timeline; #838 (issue #636) adds /api/v1/inventory and will need a union merge in the webui/app.py import block whichever lands second. ISSUE: #637 HEAD_SHA: 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863 PRIOR_REVIEWED_HEAD_SHA: a20975688da905312f5757c352a4d3b8f1f693e3 REVIEW_STATUS: REQUEST_CHANGES MERGE_READY: no BLOCKERS: F2 residual, externally influenced event_type reaches the serialized payload without the fail-closed redaction or validation boundary in both timeline adapters; branch is 8 commits behind live base and branch protection requires a current base VALIDATION: pytest tests/test_webui_timeline.py 42 passed; pytest -k webui 340 passed with 270 subtests passed; pytest -k redact 78 passed; pytest tests/test_canonical_thread_handoff.py tests/test_control_plane_db.py 29 passed; F2 residual reproduced by direct execution at this head with output quoted in this review; master parity in_parity=true mutation_safe=true restart_required=false at 188e83c4d69bfb959c91b1f8e766ab8a0656d92d LAST_UPDATED_BY: sysadmin / prgs-reviewer [THREAD STATE LEDGER] What is true now: PR #849 remains open on branch feat/issue-637-timeline-model against base master. The live head is 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863, pinned from assess_pr_sync_status and get_pr_review_feedback immediately before submission. The branch is 8 commits behind live base 188e83c4d69bfb959c91b1f8e766ab8a0656d92d, reports mergeable with no conflicts, and status checks are not required for this base. Server-side decision state: review 522 by sysadmin carries a REQUEST_CHANGES verdict recorded at the former head a20975688da905312f5757c352a4d3b8f1f693e3 and remains undismissed and stale. This review adds one terminal REQUEST_CHANGES verdict at the current head. Local verdict/state: F1 confirmed resolved at this head; F2 confirmed incomplete at this head with a residual event_type bypass reproduced in both adapters. What changed: No repository content changed. This session created a detached review worktree at the pinned head, acquired a reviewer lease, reproduced the residual defect, ran four test selections, and submitted one review verdict. What is blocked: Blocker classification: no blocker Nothing obstructs the next step. The author holds every capability required to remediate the finding and to synchronize the base. Who/what acts next: Next actor: author — a fresh standalone prgs-author session on issue #637. Required action: constrain CTH types against CTH_TYPES on the parse path the timeline adapter uses, apply fail-closed validation or redaction to stored control-plane event types, audit remaining externally influenced serialized string fields, add payload-level canary coverage through both event_type paths, preserve legitimate declared types and references, and synchronize the branch with current master through the canonical author workflow. Do not do: do not merge PR #849, since no approval verdict exists at any head; do not dismiss, edit, or reinterpret review 522 or this review; do not modify PRs #846, #838, #818, #795, or #794; do not force-push this branch. ROLE: reviewer IDENTITY: sysadmin PROFILE: prgs-reviewer REMOTE: prgs REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools PR: #849 ISSUE: #637 REVIEWED_HEAD: 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863 BASE: master @ 188e83c4d69bfb959c91b1f8e766ab8a0656d92d VERDICT: REQUEST_CHANGES BLOCKING_FINDINGS: 1 BLOCKER: none MUTATIONS: one reviewer lease comment (15066); one terminal PR review at the pinned head MERGE_PERFORMED: no REVIEW_DISMISSED: no NEXT_ROLE: author STATE: changes-requested
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #849
issue: #637
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 63651-4478b80e082c
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-f2
phase: released
candidate_head: 2d95e0fcc6
target_branch: master
target_branch_sha: 188e83c4d6
last_activity: 2026-07-23T22:28:27Z
expires_at: 2026-07-23T22:38:27Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #849 issue: #637 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 63651-4478b80e082c worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr849-f2 phase: released candidate_head: 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863 target_branch: master target_branch_sha: 188e83c4d69bfb959c91b1f8e766ab8a0656d92d last_activity: 2026-07-23T22:28:27Z expires_at: 2026-07-23T22:38:27Z blocker: manual-release
jcwalker3 added 1 commit 2026-07-23 17:46:34 -05:00
Review #526 found that event_type reached the serialized timeline payload
without crossing the redaction/validation boundary every other free-text
field on the same event crosses. A synthetic secret-shaped 40-hex canary
was redacted through message but survived verbatim through event_type on
the same control-plane record.

CTH heading path: CTH_TYPES is now the single authority for what a CTH
type may be. canonical_thread_handoff.is_known_cth_type() is that
authority, used by format_cth_body (write), assess_cth_comment (assess),
and now the read path too; parse_cth_comment reports membership as
cth_type_known and stays total. adapt_cth_comments serializes
handoff:<type> only for a declared type and otherwise emits the constant
handoff:unrecognized, so arbitrary, malformed, secret-shaped, or
whitespace-manipulated heading content never becomes an event_type.

Control-plane path: a stored event_type is treated as source data.
_safe_cp_event_type accepts only an ordinary identifier that is not a
bare secret-shaped hex run and that a redaction pass leaves unchanged;
anything else fails closed to the constant unsafe:redacted and marks the
event sensitive. The value is never emitted verbatim, never partially
sanitized, and never rewritten into a different valid-looking type.

Remaining serialized-field audit: event_key ids must be plain numeric
identifiers, the CTH adapter refuses a scope it cannot express, per-source
failure reasons are redacted (they can quote an authenticated fetch error),
and echoed scope/filter values are guarded so reflection is not a bypass.

Legitimate values are preserved: every declared CTH type, the real
producer types (assigned, lease_released, lease_adopted,
dependency_edge_state_change, allocation, pr.opened, lease.renew), and
the existing issue, PR, evidence, and full-SHA references.

Tests seed the canary independently through both event_type paths with a
benign message, so message redaction cannot be why they pass; each
inspects event_type directly, asserts the canary is absent from the
complete serialized payload, and asserts scan_for_secrets finds nothing.

tests/test_webui_timeline.py 64 passed, 15 subtests
pytest -k webui 362 passed, 285 subtests
pytest -k redact 79 passed
tests/test_canonical_thread_handoff.py tests/test_control_plane_db.py 29 passed

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jcwalker3 added 1 commit 2026-07-23 17:47:30 -05:00
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #849
issue: #637
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 41973-22c82c114387
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-pr849-b9ba43a5
phase: claimed
candidate_head: b9ba43a5bf
target_branch: master
target_branch_sha: 188e83c4d6
last_activity: 2026-07-24T00:16:49Z
expires_at: 2026-07-24T00:26:49Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #849 issue: #637 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 41973-22c82c114387 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-pr849-b9ba43a5 phase: claimed candidate_head: b9ba43a5bf292690eec0b24b19a1f489930986e7 target_branch: master target_branch_sha: 188e83c4d69bfb959c91b1f8e766ab8a0656d92d last_activity: 2026-07-24T00:16:49Z expires_at: 2026-07-24T00:26:49Z blocker: none
sysadmin approved these changes 2026-07-23 19:18:19 -05:00
sysadmin left a comment
Owner

APPROVE — reviewed at head b9ba43a5bf292690eec0b24b19a1f489930986e7.

Reviewer sysadmin / prgs-reviewer; author jcwalker3. Reviewed from a detached worktree pinned to the exact head, clean tree.

The F2 residual recorded in review #526 is fully remediated in both adapters, F1 remains correct, and the four additional serialized-field bypasses the author found are closed proportionately. Approving.

F2 — event_type boundary, verified closed

CTH adapter

is_known_cth_type() is now the single authority, backed by CTH_TYPES; no competing valid-type list was introduced. format_cth_body and assess_cth_comment were both rewritten to call it, so write, assess, and read consult one contract. parse_cth_comment stays total and now reports cth_type_known alongside the parsed type, so a reader can tell a declared type from arbitrary heading text without the parser raising.

adapt_cth_comments checks membership itself and emits handoff:<declared type> only when known, otherwise the explicit handoff:unrecognized constant, and flags the event sensitive. Arbitrary heading content therefore has no path into a serialized type.

Control-plane adapter

_safe_cp_event_type treats the stored value as externally influenced and admits it only when it is identifier-shaped, is not a bare secret-shaped hex run, and is left byte-identical by a redaction pass. Anything else fails closed to the unsafe:redacted constant and marks the event sensitive. The refused value is never emitted verbatim, never partially sanitized, and deliberately not rewritten into a plausible workflow type that would misdescribe the record.

Reproduction at this head

Executed against the pinned worktree with the repository venv, using a fabricated secret-shaped 40-character lowercase hex canary — never a real credential.

Control-plane path, canary as event_type with a deliberately benign message so message redaction cannot be what passes the case:

event_type : unsafe:redacted
message    : benign text, no secret
canary in event_type: False
canary in FULL serialized payload: False
sensitive  : True

CTH path, canary as the heading:

event_type : handoff:unrecognized
canary in event_type: False
canary in FULL serialized payload: False
sensitive  : True

scan_for_secrets over the complete serialized response returned an empty list. Both paths seeded at once in one load_timeline response: the canary appears nowhere, while a legitimate handoff:Author Handoff event in the same response survives intact with its #849 and caaae9b6 references.

Malformed input fails closed: has space, interior newline / carriage-return / tab, ../../etc/passwd, a script tag, over-length values, and an assigned-credential form all resolve to unsafe:redacted. Leading and trailing whitespace is normalized rather than refused, and no control character survives into the emitted value.

Legitimate types are preserved exactly: allocation, pr.opened, lease.renew, assigned, lease_released, lease_expired, lease_abandoned, lease_adopted, dependency_edge_state_change, and all seven declared CTH_TYPES members. A heading of unsafe:redacted or handoff:unrecognized cannot be used to forge a declared type — both land on the fallback and are marked sensitive.

The four additional serialized-field fixes

Each verified independently at this head.

  1. Record identifiers into event_key. _safe_record_id admits only a plain numeric id. Injection-shaped ids, path fragments, booleans, over-long digit runs, and the canary are all refused and the row is skipped rather than keyed on. A legitimate id stays deterministic: cp:42.
  2. CTH kind/number into event_key and correlation_id. Normalized once up front; a scope the adapter cannot express is refused outright. Injection-shaped kinds, the canary, empty, and null produce no events; issue/pr with an integer produce cth:pr:849:5 and pr#849. A non-integer number is refused rather than interpolated.
  3. SourceStatus.reason. Both status constructors redact the reason. A simulated authenticated fetch exception carrying basic-auth credentials in a URL plus a bearer value emitted handoff source failed: 401 from [REDACTED_URL] token=[REDACTED] — the canary is absent, and the snapshot degraded per-source instead of collapsing.
  4. Echoed scope and filters. _safe_echo guards remote, org, repo, and each filter value; a secret-shaped value becomes the placeholder while ordinary values pass through untouched.

Wider audit: message, decision, actor, role, and the next action / status fallback all pass _redact; session_id passes _safe_session_id; source is a module constant; issue and PR numbers are integers; nested page and source values are covered by the same boundary.

Evidence references remain usable and are not over-blocked. _SHA_RE now requires an anchoring keyword, so #849, short SHAs, and full 40-character SHAs stay usable while undeclared hex is not promoted. Confirmed directly: a canary supplied bare, in prose, as an assigned credential, as a bearer header, or inside a URL's basic-auth segment yields an empty reference list and never reaches the payload. _validated_evidence_refs rechecks every reference before serialization and flags the event sensitive when it drops one.

F1 — confirmed still correct, not reopened

_SOURCE_FILTER_SUPPORT declares per-source dimensions and is surfaced as supported_filters / unsupported_filters. session_id is populated only from the declared CTH Session: field; the control-plane adapter sets none and records why inline, and the obsolete control-plane session read stays removed. An unanswerable session filter returns ok=false with code: filter_not_supported and the route answers HTTP 422. A supported filter with no matches still returns an honest successful empty page. Filtering runs before pagination; ordering is (timestamp, source_rank, event_key) with missing timestamps last; issue and PR filters behave correctly.

Scope and base synchronization

The delta against master 188e83c4d69bfb959c91b1f8e766ab8a0656d92d is exactly five files: webui/timeline.py, webui/app.py, canonical_thread_handoff.py, tests/test_webui_timeline.py, and docs/webui-local-dev.md. The a1e5a4af8ce0fc5da4d30db2712c4407c4af43dd commit is the security remediation; b9ba43a5 is a clean merge of live master carrying no author content — every non-PR file at this head is byte-identical to master, and no master content was dropped. Issue #637 remains the only issue implemented; nothing bleeds into #636 or #638, and the JSON route stays distinct from the HTML stub at /timeline.

Non-blocking observation

_BARE_SECRET_SHAPE is anchored to lowercase hex, so an uppercase 32-character-or-longer hex run is admitted by _safe_cp_event_type and _safe_session_id. This is not the asymmetry #526 described: the redaction policy itself does not classify a bare hex run of either case as a credential — scan_for_secrets returns empty for both — and the lowercase case is refused only by this module's own additional guard. Gitea tokens and git SHAs are lowercase, so the guard matches the real shape. Adding a case-insensitive flag to that one pattern would close the case variant at no cost to any legitimate type, whenever this area is next touched. Not blocking and not a condition of this approval.

Tests run by this review

From the pinned worktree, repository venv:

  • pytest tests/test_webui_timeline.py64 passed, 15 subtests passed.
  • pytest -k webui376 passed, 4321 deselected, 336 subtests passed.
  • pytest tests/test_canonical_thread_handoff.py tests/test_control_plane_db.py29 passed.
  • pytest -k redact78 passed, 1 failed.

The first three match the author's reported figures exactly.

The -k redact failure is tests/test_mcp_server.py::TestSubmitPrReview::test_error_message_redacts_credential. It is not caused by this PR and is not a redaction defect. The assertion never reaches the credential path: the call fails closed earlier on blocker_kind: workspace_role_binding, because the review worktree used for that run was a detached scratchpad checkout rather than one under branches/. The identical node was run at clean master 188e83c4d69bfb959c91b1f8e766ab8a0656d92d in an equivalent detached worktree and failed identically with the same blocker, so it is environment-determined baseline behaviour. Counting the selections, the PR head reports 78 passed against the baseline's 68 for the same single failure, so this branch adds ten passing redaction tests and no new failure. The author's reported 79-passed figure is consistent with running from a branches/ worktree.

The full suite was not rerun: the focused evidence resolves both event_type paths and all four additional fields, and the only failure encountered was independently classified against current master.

New security regression coverage

TestEventTypeBoundary seeds the canary through the control-plane event_type and the CTH heading separately and through both at once, asserts on the field directly and then on the complete serialized payload, and pins the benign message so message redaction provably is not doing the work. TestSerializedFieldAudit covers the four additional fields, and TestCthTypeContract pins contract membership. This is the coverage whose absence let the residual survive the previous round.

NATIVE_REVIEW_PROOF: native MCP reviewer submission by sysadmin / prgs-reviewer at head b9ba43a5bf via gitea_submit_pr_review, workflow review-merge-pr hash 263d0a6cb8a6, reviewer lease comment 15130, session 41973-22c82c114387.

Canonical PR State

STATE: approved
WHO_IS_NEXT: merger
NEXT_ACTION: A fresh standalone prgs-merger session independently re-verifies eligibility, runtime parity, exact head b9ba43a5bf, approval validity at that head, lease state, base synchronization, checks, and conflicts, then merges PR #849 if and only if all of its own gates pass
NEXT_PROMPT:

Merge PR #849 for issue #637 as prgs-merger pinned to exact head b9ba43a5bf292690eec0b24b19a1f489930986e7 with the approval recorded by this review. Load the canonical gitea-workflow skill first. Prove identity sysadmin, profile prgs-merger, role merger, in_parity=true, restart_required=false, stop_required=false, mutation_safe=true, and resolve the merge_pr capability under its own task name before acquiring a merger lease. Confirm PR state open, head still exactly b9ba43a5bf292690eec0b24b19a1f489930986e7, approval valid at that exact head, commits_behind=0 against base master 188e83c4d69bfb959c91b1f8e766ab8a0656d92d, mergeable true, conflicts false, and checks not required. Two foreign author artifacts exist and are not merger-owned: a retained issue #637 author worktree whose lease could not be finalized, and a malformed issue #850 author lock. Independently determine whether either blocks merge eligibility; never clear, overwrite, repoint, or repair them. Acquire the merger lease rather than adopting any dead reviewer lease, re-resolve immediately before the merge mutation, merge, then stop. Do not select another PR.

WHAT_HAPPENED: Reviewer sysadmin reviewed PR #849 at head b9ba43a5bf from a clean detached worktree pinned to that exact head, read issue #637 and reviews 522 and 526, classified the 2d95e0fc to a1e5a4af security remediation and the a1e5a4af to b9ba43a5 base-sync merge separately, confirmed the merge carries no author content and the PR delta against current master is exactly five files, reproduced the closure of the F2 event_type residual in both adapters and of all four additional serialized-field bypasses by direct execution with a fabricated canary, confirmed F1 still correct, ran four test selections, independently classified the single test failure encountered against clean master, and submitted one terminal APPROVED review. No merge was performed, no branch was modified, and reviews 522 and 526 were not altered.
WHY: The blocking defect recorded in review 526 is resolved at this head. Externally influenced event_type now crosses a fail-closed boundary in both adapters, verified by seeding a fabricated secret-shaped canary through the control-plane event_type and the CTH heading with a benign message so message redaction demonstrably is not masking the result, and the canary appears nowhere in the complete serialized response. The four additional serialized-field protections are correct and proportionate, legitimate declared types and legitimate issue, PR, evidence, and authoritative commit references remain usable, F1 remains correct, issue #637 acceptance criteria 1 through 5 are met, and the only test failure encountered reproduces identically at current master.
ISSUE: #637
HEAD_SHA: b9ba43a5bf
PRIOR_REVIEWED_HEAD_SHA: 2d95e0fcc6
REVIEW_STATUS: APPROVED
MERGE_READY: yes — approval is recorded at the current head with base synchronized, no conflicts, and checks not required; the merger must still pass its own eligibility, runtime, lease, and conflict gates before merging
BLOCKERS: none from this review
VALIDATION: pytest tests/test_webui_timeline.py 64 passed 15 subtests; pytest -k webui 376 passed 336 subtests; canonical-thread and control-plane 29 passed; pytest -k redact 78 passed 1 failed, that failure reproduced identically at master 188e83c4d6 and classified environment-determined; F2 closure and the four additional field fixes reproduced by direct execution at this head with output quoted above; master parity in_parity=true mutation_safe=true restart_required=false at 188e83c4d6
LAST_UPDATED_BY: sysadmin / prgs-reviewer

[THREAD STATE LEDGER]

What is true now:
PR #849 is open on branch feat/issue-637-timeline-model against base master. The live head is b9ba43a5bf, pinned from assess_pr_sync_status and get_pr_review_feedback immediately before submission. The branch is 0 commits behind live base 188e83c4d6, reports mergeable with no conflicts, and status checks are not required for this base.
Server-side decision state: review 522 is dismissed and stale at former head a20975688d. Review 526 carries REQUEST_CHANGES recorded at former head 2d95e0fcc6, is undismissed, and is stale relative to the current head. This review adds one terminal APPROVED verdict at the current head.
The author remediation comment for this round is absent from the thread. The author reported being unable to post it because a foreign malformed issue #850 author lock blocked further author mutations. That absence is a protocol discrepancy only; it was not treated as evidence either way, and every conclusion above rests on direct diff inspection and execution at the pinned head.

What changed:
No repository content changed. This session created a detached review worktree at the pinned head and a second detached worktree at master for baseline comparison, acquired a reviewer lease, reproduced the security behaviour with a fabricated canary, ran four test selections, and submitted one review verdict.

What is blocked:
Blocker classification: no blocker
Reviewer operations were never obstructed. The retained issue #637 author worktree and the malformed issue #850 author lock are not reviewer-owned, were left untouched, and did not interfere with reviewer lease acquisition or verdict submission.

Who/what acts next:
Next actor: merger — a fresh standalone prgs-merger session on PR #849.
Required action: independently verify its own eligibility, runtime parity, exact head, approval validity at that head, lease ownership, base synchronization, checks, and conflicts, then merge only if all pass.
Do not do: do not clear, overwrite, repoint, or repair the retained issue #637 author lease or the malformed issue #850 author lock; do not dismiss, edit, or reinterpret review 522 or review 526; do not modify PRs #846, #838, #818, #795, or #794; do not force-push this branch.

ROLE: reviewer
IDENTITY: sysadmin
PROFILE: prgs-reviewer
REMOTE: prgs
REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools
PR: #849
ISSUE: #637
REVIEWED_HEAD: b9ba43a5bf
BASE: master @ 188e83c4d6
VERDICT: APPROVED
BLOCKING_FINDINGS: 0
BLOCKER: none
MERGE_PERFORMED: no
REVIEW_DISMISSED: no
NEXT_ROLE: merger
STATE: approved

APPROVE — reviewed at head `b9ba43a5bf292690eec0b24b19a1f489930986e7`. Reviewer `sysadmin` / `prgs-reviewer`; author `jcwalker3`. Reviewed from a detached worktree pinned to the exact head, clean tree. The F2 residual recorded in review #526 is fully remediated in both adapters, F1 remains correct, and the four additional serialized-field bypasses the author found are closed proportionately. Approving. ## F2 — event_type boundary, verified closed ### CTH adapter `is_known_cth_type()` is now the single authority, backed by `CTH_TYPES`; no competing valid-type list was introduced. `format_cth_body` and `assess_cth_comment` were both rewritten to call it, so write, assess, and read consult one contract. `parse_cth_comment` stays total and now reports `cth_type_known` alongside the parsed type, so a reader can tell a declared type from arbitrary heading text without the parser raising. `adapt_cth_comments` checks membership itself and emits `handoff:<declared type>` only when known, otherwise the explicit `handoff:unrecognized` constant, and flags the event `sensitive`. Arbitrary heading content therefore has no path into a serialized type. ### Control-plane adapter `_safe_cp_event_type` treats the stored value as externally influenced and admits it only when it is identifier-shaped, is not a bare secret-shaped hex run, and is left byte-identical by a redaction pass. Anything else fails closed to the `unsafe:redacted` constant and marks the event sensitive. The refused value is never emitted verbatim, never partially sanitized, and deliberately not rewritten into a plausible workflow type that would misdescribe the record. ### Reproduction at this head Executed against the pinned worktree with the repository venv, using a fabricated secret-shaped 40-character lowercase hex canary — never a real credential. Control-plane path, canary as `event_type` with a deliberately benign message so message redaction cannot be what passes the case: ``` event_type : unsafe:redacted message : benign text, no secret canary in event_type: False canary in FULL serialized payload: False sensitive : True ``` CTH path, canary as the heading: ``` event_type : handoff:unrecognized canary in event_type: False canary in FULL serialized payload: False sensitive : True ``` `scan_for_secrets` over the complete serialized response returned an empty list. Both paths seeded at once in one `load_timeline` response: the canary appears nowhere, while a legitimate `handoff:Author Handoff` event in the same response survives intact with its `#849` and `caaae9b6` references. Malformed input fails closed: `has space`, interior newline / carriage-return / tab, `../../etc/passwd`, a script tag, over-length values, and an assigned-credential form all resolve to `unsafe:redacted`. Leading and trailing whitespace is normalized rather than refused, and no control character survives into the emitted value. Legitimate types are preserved exactly: `allocation`, `pr.opened`, `lease.renew`, `assigned`, `lease_released`, `lease_expired`, `lease_abandoned`, `lease_adopted`, `dependency_edge_state_change`, and all seven declared `CTH_TYPES` members. A heading of `unsafe:redacted` or `handoff:unrecognized` cannot be used to forge a declared type — both land on the fallback and are marked sensitive. ## The four additional serialized-field fixes Each verified independently at this head. 1. **Record identifiers into `event_key`.** `_safe_record_id` admits only a plain numeric id. Injection-shaped ids, path fragments, booleans, over-long digit runs, and the canary are all refused and the row is skipped rather than keyed on. A legitimate id stays deterministic: `cp:42`. 2. **CTH kind/number into `event_key` and `correlation_id`.** Normalized once up front; a scope the adapter cannot express is refused outright. Injection-shaped kinds, the canary, empty, and null produce no events; `issue`/`pr` with an integer produce `cth:pr:849:5` and `pr#849`. A non-integer number is refused rather than interpolated. 3. **`SourceStatus.reason`.** Both status constructors redact the reason. A simulated authenticated fetch exception carrying basic-auth credentials in a URL plus a bearer value emitted `handoff source failed: 401 from [REDACTED_URL] token=[REDACTED]` — the canary is absent, and the snapshot degraded per-source instead of collapsing. 4. **Echoed scope and filters.** `_safe_echo` guards `remote`, `org`, `repo`, and each filter value; a secret-shaped value becomes the placeholder while ordinary values pass through untouched. Wider audit: `message`, `decision`, `actor`, `role`, and the `next action` / `status` fallback all pass `_redact`; `session_id` passes `_safe_session_id`; `source` is a module constant; issue and PR numbers are integers; nested page and source values are covered by the same boundary. Evidence references remain usable and are not over-blocked. `_SHA_RE` now requires an anchoring keyword, so `#849`, short SHAs, and full 40-character SHAs stay usable while undeclared hex is not promoted. Confirmed directly: a canary supplied bare, in prose, as an assigned credential, as a bearer header, or inside a URL's basic-auth segment yields an empty reference list and never reaches the payload. `_validated_evidence_refs` rechecks every reference before serialization and flags the event sensitive when it drops one. ## F1 — confirmed still correct, not reopened `_SOURCE_FILTER_SUPPORT` declares per-source dimensions and is surfaced as `supported_filters` / `unsupported_filters`. `session_id` is populated only from the declared CTH `Session:` field; the control-plane adapter sets none and records why inline, and the obsolete control-plane session read stays removed. An unanswerable session filter returns `ok=false` with `code: filter_not_supported` and the route answers HTTP 422. A supported filter with no matches still returns an honest successful empty page. Filtering runs before pagination; ordering is `(timestamp, source_rank, event_key)` with missing timestamps last; issue and PR filters behave correctly. ## Scope and base synchronization The delta against master `188e83c4d69bfb959c91b1f8e766ab8a0656d92d` is exactly five files: `webui/timeline.py`, `webui/app.py`, `canonical_thread_handoff.py`, `tests/test_webui_timeline.py`, and `docs/webui-local-dev.md`. The `a1e5a4af8ce0fc5da4d30db2712c4407c4af43dd` commit is the security remediation; `b9ba43a5` is a clean merge of live master carrying no author content — every non-PR file at this head is byte-identical to master, and no master content was dropped. Issue #637 remains the only issue implemented; nothing bleeds into #636 or #638, and the JSON route stays distinct from the HTML stub at `/timeline`. ## Non-blocking observation `_BARE_SECRET_SHAPE` is anchored to lowercase hex, so an uppercase 32-character-or-longer hex run is admitted by `_safe_cp_event_type` and `_safe_session_id`. This is not the asymmetry #526 described: the redaction policy itself does not classify a bare hex run of either case as a credential — `scan_for_secrets` returns empty for both — and the lowercase case is refused only by this module's own additional guard. Gitea tokens and git SHAs are lowercase, so the guard matches the real shape. Adding a case-insensitive flag to that one pattern would close the case variant at no cost to any legitimate type, whenever this area is next touched. Not blocking and not a condition of this approval. ## Tests run by this review From the pinned worktree, repository venv: - `pytest tests/test_webui_timeline.py` — **64 passed, 15 subtests passed**. - `pytest -k webui` — **376 passed, 4321 deselected, 336 subtests passed**. - `pytest tests/test_canonical_thread_handoff.py tests/test_control_plane_db.py` — **29 passed**. - `pytest -k redact` — **78 passed, 1 failed**. The first three match the author's reported figures exactly. The `-k redact` failure is `tests/test_mcp_server.py::TestSubmitPrReview::test_error_message_redacts_credential`. It is not caused by this PR and is not a redaction defect. The assertion never reaches the credential path: the call fails closed earlier on `blocker_kind: workspace_role_binding`, because the review worktree used for that run was a detached scratchpad checkout rather than one under `branches/`. The identical node was run at clean master `188e83c4d69bfb959c91b1f8e766ab8a0656d92d` in an equivalent detached worktree and failed identically with the same blocker, so it is environment-determined baseline behaviour. Counting the selections, the PR head reports 78 passed against the baseline's 68 for the same single failure, so this branch adds ten passing redaction tests and no new failure. The author's reported 79-passed figure is consistent with running from a `branches/` worktree. The full suite was not rerun: the focused evidence resolves both event_type paths and all four additional fields, and the only failure encountered was independently classified against current master. ## New security regression coverage `TestEventTypeBoundary` seeds the canary through the control-plane `event_type` and the CTH heading separately and through both at once, asserts on the field directly and then on the complete serialized payload, and pins the benign message so message redaction provably is not doing the work. `TestSerializedFieldAudit` covers the four additional fields, and `TestCthTypeContract` pins contract membership. This is the coverage whose absence let the residual survive the previous round. NATIVE_REVIEW_PROOF: native MCP reviewer submission by sysadmin / prgs-reviewer at head b9ba43a5bf292690eec0b24b19a1f489930986e7 via gitea_submit_pr_review, workflow review-merge-pr hash 263d0a6cb8a6, reviewer lease comment 15130, session 41973-22c82c114387. ## Canonical PR State STATE: approved WHO_IS_NEXT: merger NEXT_ACTION: A fresh standalone prgs-merger session independently re-verifies eligibility, runtime parity, exact head b9ba43a5bf292690eec0b24b19a1f489930986e7, approval validity at that head, lease state, base synchronization, checks, and conflicts, then merges PR #849 if and only if all of its own gates pass NEXT_PROMPT: ```text Merge PR #849 for issue #637 as prgs-merger pinned to exact head b9ba43a5bf292690eec0b24b19a1f489930986e7 with the approval recorded by this review. Load the canonical gitea-workflow skill first. Prove identity sysadmin, profile prgs-merger, role merger, in_parity=true, restart_required=false, stop_required=false, mutation_safe=true, and resolve the merge_pr capability under its own task name before acquiring a merger lease. Confirm PR state open, head still exactly b9ba43a5bf292690eec0b24b19a1f489930986e7, approval valid at that exact head, commits_behind=0 against base master 188e83c4d69bfb959c91b1f8e766ab8a0656d92d, mergeable true, conflicts false, and checks not required. Two foreign author artifacts exist and are not merger-owned: a retained issue #637 author worktree whose lease could not be finalized, and a malformed issue #850 author lock. Independently determine whether either blocks merge eligibility; never clear, overwrite, repoint, or repair them. Acquire the merger lease rather than adopting any dead reviewer lease, re-resolve immediately before the merge mutation, merge, then stop. Do not select another PR. ``` WHAT_HAPPENED: Reviewer sysadmin reviewed PR #849 at head b9ba43a5bf292690eec0b24b19a1f489930986e7 from a clean detached worktree pinned to that exact head, read issue #637 and reviews 522 and 526, classified the 2d95e0fc to a1e5a4af security remediation and the a1e5a4af to b9ba43a5 base-sync merge separately, confirmed the merge carries no author content and the PR delta against current master is exactly five files, reproduced the closure of the F2 event_type residual in both adapters and of all four additional serialized-field bypasses by direct execution with a fabricated canary, confirmed F1 still correct, ran four test selections, independently classified the single test failure encountered against clean master, and submitted one terminal APPROVED review. No merge was performed, no branch was modified, and reviews 522 and 526 were not altered. WHY: The blocking defect recorded in review 526 is resolved at this head. Externally influenced event_type now crosses a fail-closed boundary in both adapters, verified by seeding a fabricated secret-shaped canary through the control-plane event_type and the CTH heading with a benign message so message redaction demonstrably is not masking the result, and the canary appears nowhere in the complete serialized response. The four additional serialized-field protections are correct and proportionate, legitimate declared types and legitimate issue, PR, evidence, and authoritative commit references remain usable, F1 remains correct, issue #637 acceptance criteria 1 through 5 are met, and the only test failure encountered reproduces identically at current master. ISSUE: #637 HEAD_SHA: b9ba43a5bf292690eec0b24b19a1f489930986e7 PRIOR_REVIEWED_HEAD_SHA: 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863 REVIEW_STATUS: APPROVED MERGE_READY: yes — approval is recorded at the current head with base synchronized, no conflicts, and checks not required; the merger must still pass its own eligibility, runtime, lease, and conflict gates before merging BLOCKERS: none from this review VALIDATION: pytest tests/test_webui_timeline.py 64 passed 15 subtests; pytest -k webui 376 passed 336 subtests; canonical-thread and control-plane 29 passed; pytest -k redact 78 passed 1 failed, that failure reproduced identically at master 188e83c4d69bfb959c91b1f8e766ab8a0656d92d and classified environment-determined; F2 closure and the four additional field fixes reproduced by direct execution at this head with output quoted above; master parity in_parity=true mutation_safe=true restart_required=false at 188e83c4d69bfb959c91b1f8e766ab8a0656d92d LAST_UPDATED_BY: sysadmin / prgs-reviewer [THREAD STATE LEDGER] What is true now: PR #849 is open on branch feat/issue-637-timeline-model against base master. The live head is b9ba43a5bf292690eec0b24b19a1f489930986e7, pinned from assess_pr_sync_status and get_pr_review_feedback immediately before submission. The branch is 0 commits behind live base 188e83c4d69bfb959c91b1f8e766ab8a0656d92d, reports mergeable with no conflicts, and status checks are not required for this base. Server-side decision state: review 522 is dismissed and stale at former head a20975688da905312f5757c352a4d3b8f1f693e3. Review 526 carries REQUEST_CHANGES recorded at former head 2d95e0fcc6dafddd7a11f9a6565ae57b93f69863, is undismissed, and is stale relative to the current head. This review adds one terminal APPROVED verdict at the current head. The author remediation comment for this round is absent from the thread. The author reported being unable to post it because a foreign malformed issue #850 author lock blocked further author mutations. That absence is a protocol discrepancy only; it was not treated as evidence either way, and every conclusion above rests on direct diff inspection and execution at the pinned head. What changed: No repository content changed. This session created a detached review worktree at the pinned head and a second detached worktree at master for baseline comparison, acquired a reviewer lease, reproduced the security behaviour with a fabricated canary, ran four test selections, and submitted one review verdict. What is blocked: Blocker classification: no blocker Reviewer operations were never obstructed. The retained issue #637 author worktree and the malformed issue #850 author lock are not reviewer-owned, were left untouched, and did not interfere with reviewer lease acquisition or verdict submission. Who/what acts next: Next actor: merger — a fresh standalone prgs-merger session on PR #849. Required action: independently verify its own eligibility, runtime parity, exact head, approval validity at that head, lease ownership, base synchronization, checks, and conflicts, then merge only if all pass. Do not do: do not clear, overwrite, repoint, or repair the retained issue #637 author lease or the malformed issue #850 author lock; do not dismiss, edit, or reinterpret review 522 or review 526; do not modify PRs #846, #838, #818, #795, or #794; do not force-push this branch. ROLE: reviewer IDENTITY: sysadmin PROFILE: prgs-reviewer REMOTE: prgs REPOSITORY: Scaled-Tech-Consulting/Gitea-Tools PR: #849 ISSUE: #637 REVIEWED_HEAD: b9ba43a5bf292690eec0b24b19a1f489930986e7 BASE: master @ 188e83c4d69bfb959c91b1f8e766ab8a0656d92d VERDICT: APPROVED BLOCKING_FINDINGS: 0 BLOCKER: none MERGE_PERFORMED: no REVIEW_DISMISSED: no NEXT_ROLE: merger STATE: approved
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #849
issue: #637
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 41973-22c82c114387
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-pr849-b9ba43a5
phase: released
candidate_head: b9ba43a5bf
target_branch: master
target_branch_sha: 188e83c4d6
last_activity: 2026-07-24T00:18:35Z
expires_at: 2026-07-24T00:28:35Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #849 issue: #637 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 41973-22c82c114387 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/reviewer-pr849-b9ba43a5 phase: released candidate_head: b9ba43a5bf292690eec0b24b19a1f489930986e7 target_branch: master target_branch_sha: 188e83c4d69bfb959c91b1f8e766ab8a0656d92d last_activity: 2026-07-24T00:18:35Z expires_at: 2026-07-24T00:28:35Z blocker: manual-release
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #849
issue: #637
reviewer_identity: sysadmin
profile: prgs-merger
session_id: 77376-ba12f1fb83a2
worktree: /Users/jasonwalker/Development/Gitea-Tools
phase: claimed
candidate_head: b9ba43a5bf
target_branch: master
target_branch_sha: 188e83c4d6
last_activity: 2026-07-24T00:31:16Z
expires_at: 2026-07-24T00:41:16Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #849 issue: #637 reviewer_identity: sysadmin profile: prgs-merger session_id: 77376-ba12f1fb83a2 worktree: /Users/jasonwalker/Development/Gitea-Tools phase: claimed candidate_head: b9ba43a5bf292690eec0b24b19a1f489930986e7 target_branch: master target_branch_sha: 188e83c4d69bfb959c91b1f8e766ab8a0656d92d last_activity: 2026-07-24T00:31:16Z expires_at: 2026-07-24T00:41:16Z blocker: none
sysadmin merged commit 9301739910 into master 2026-07-23 19:33:22 -05:00
Owner

Stale #332 review-decision lock cleanup (#594)

Status: APPLIED

Manual deletion of session-state files is not the workflow.
This path only clears a lock when the referenced PR is merged/closed.

## Stale #332 review-decision lock cleanup (#594) Status: **APPLIED** - actor: `sysadmin` - profile: `prgs-merger` - timestamp: `2026-07-24T00:33:24.449903+00:00` - last terminal: `approve` on PR #849 - PR state: `closed` (merged=True) - merge_commit_sha: `9301739910df7b51ca3abcdbf46cce25cfb026e5` - prior live_mutations_count: `5` - prior profile_identity: `prgs-reviewer` Manual deletion of session-state files is **not** the workflow. This path only clears a lock when the referenced PR is merged/closed.
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#849