fix(suggested-fix-pending): apply PR #156 review fixes

- Page-level Resolve patches applied_pending → applied_success before
  opening the resolution flow, so resolved sessions don't carry a
  provisional pending fix.
- Page-level Escalate intercept now catches applied_pending in addition
  to verifying/partial; intercept copy generalized from "Verifying state"
  to "still needs an outcome."
- PendingBanner gains a Dismiss action, matching the PR body and the
  backend's allowed pending → dismissed transition.
- resolution_note_generator and escalation_package_generator system
  prompts no longer include real-looking pending examples (anti-parrot
  guardrail compliance).

Verified via Docker: prompt anti-parrot 2/2, suggested-fix outcome suite
21/21, frontend tsc -b clean, npm run build clean.

Co-Authored-By: Codex <noreply@openai.com>
This commit is contained in:
2026-04-30 23:02:46 -04:00
parent 7cee7228dc
commit 5bee264d70
8 changed files with 59 additions and 33 deletions

View File

@@ -2,7 +2,7 @@
**Task:** Add a fourth, non-terminal outcome to the suggested-fix banner — **Awaiting verification** (`applied_pending`). Today the verifying banner forces a synchronous verdict (worked / didn't / partial), but a lot of real fixes are async — engineer ran the script but is waiting on the client to power-cycle, AD replication, an O365 license sync. Without a fourth state the banner sits stale or the engineer guesses wrong. **Task:** Add a fourth, non-terminal outcome to the suggested-fix banner — **Awaiting verification** (`applied_pending`). Today the verifying banner forces a synchronous verdict (worked / didn't / partial), but a lot of real fixes are async — engineer ran the script but is waiting on the client to power-cycle, AD replication, an O365 license sync. Without a fourth state the banner sits stale or the engineer guesses wrong.
**Status:****Engineering complete; PR #156 open.** Backend tests + tsc clean, Alembic migration applies. Pending browser QA. **Status:****Engineering complete; PR #156 open.** Backend tests, prompt guardrail, frontend tsc/build clean; Alembic migration applies. Pending browser QA.
**Branch:** `feat/fix-pending-verification` (off `main` after the Escalation Mode merge). **Branch:** `feat/fix-pending-verification` (off `main` after the Escalation Mode merge).
@@ -15,9 +15,9 @@
| Schema | New `FixStatus="applied_pending"` + new `pending_reason` Text column on `session_suggested_fixes`. | | Schema | New `FixStatus="applied_pending"` + new `pending_reason` Text column on `session_suggested_fixes`. |
| Migration | `c0f3a4b7e91d` — adds `pending_reason`, extends status CHECK constraint. | | Migration | `c0f3a4b7e91d` — adds `pending_reason`, extends status CHECK constraint. |
| API | `PATCH /suggested-fixes/{id}/outcome` accepts `applied_pending`, requires `notes`, stamps `applied_at` only (NOT `verified_at`). Pending in/out transitions allowed (parked, like partial). | | API | `PATCH /suggested-fixes/{id}/outcome` accepts `applied_pending`, requires `notes`, stamps `applied_at` only (NOT `verified_at`). Pending in/out transitions allowed (parked, like partial). |
| Generators | `resolution_note_generator` and `escalation_package_generator` system prompts handle the new status: resolution note frames the fix as provisional; escalation package surfaces pending verification as the leading hypothesis with reference to what's being waited on. | | Generators | `resolution_note_generator` and `escalation_package_generator` system prompts handle the new status without real-looking examples; resolution note frames the fix as provisional; escalation package surfaces pending verification as the leading hypothesis with reference to what's being waited on. |
| Frontend | New `BannerMode='pending'` + `PendingBanner` component (info-tone, mirrors `PartialBanner`); "Waiting to verify…" overflow option in `VerifyingBanner`; nudge "Still checking" now records pending with a reason instead of just silencing; `AssistantChatPage` banner-mode derivation maps `applied_pending → 'pending'`. | | Frontend | New `BannerMode='pending'` + `PendingBanner` component (info-tone, mirrors `PartialBanner`) with worked / didn't / update reason / dismiss actions; "Waiting to verify…" overflow option in `VerifyingBanner`; nudge "Still checking" now records pending with a reason instead of just silencing; `AssistantChatPage` banner-mode derivation maps `applied_pending → 'pending'`; page-level Resolve/Escalate now handle pending honestly. |
| Tests | 4 new integration tests in `test_fix_outcome_endpoint.py`: pending-requires-notes, pending stores reason + applied_at-not-verified_at, pending→success transition, pending_reason update on re-PATCH. 21/21 pass. | | Tests | 4 new integration tests in `test_fix_outcome_endpoint.py`: pending-requires-notes, pending stores reason + applied_at-not-verified_at, pending→success transition, pending_reason update on re-PATCH. 21/21 pass. Prompt anti-parrot guardrail passes. Frontend `tsc -b` and Vite build pass via Docker. |
## Out of scope (intentionally) ## Out of scope (intentionally)
@@ -30,9 +30,12 @@
1. Trigger a suggested fix, click Apply, then open the verifying banner overflow → "Waiting to verify…" → enter a reason → confirm `PendingBanner` renders with the reason. 1. Trigger a suggested fix, click Apply, then open the verifying banner overflow → "Waiting to verify…" → enter a reason → confirm `PendingBanner` renders with the reason.
2. From `PendingBanner`, click "It worked" → confirm transition to terminal success and banner dismissal. 2. From `PendingBanner`, click "It worked" → confirm transition to terminal success and banner dismissal.
3. From `PendingBanner`, click "Update reason" → confirm reason updates server-side. 3. From `PendingBanner`, click "Update reason" → confirm reason updates server-side.
4. Trigger the nudge state (3+ post-apply messages) → click "Still checking" → enter a reason → confirm pending state takes over. 4. From `PendingBanner`, click "Dismiss" → confirm banner dismissal and no terminal timestamps.
5. Trigger the nudge state (3+ post-apply messages) → click "Still checking" → enter a reason → confirm pending state takes over.
6. From a pending fix, use page-level Resolve → confirm the fix is patched to `applied_success` before the resolution note.
7. From a pending fix, use page-level Escalate → confirm the outcome intercept appears before the conclude modal.
After QA passes, merge PR #156. After QA passes, commit/push the local review fixes and merge PR #156.
## Just-shipped (2026-04-30) ## Just-shipped (2026-04-30)

View File

@@ -2,48 +2,51 @@
# HANDOFF.md # HANDOFF.md
**Last updated:** 2026-04-30 (session 4pending-verification feature shipped, PR #156 open) **Last updated:** 2026-05-01 (session 5PR #156 review fixes applied)
**Active task:** Suggested-fix `applied_pending` outcome. Branch: `feat/fix-pending-verification` (1 commit, rebased onto main). PR #156 open, ready for browser QA. **Active task:** Suggested-fix `applied_pending` outcome. Branch: `feat/fix-pending-verification`. PR #156 open; review fixes applied locally and ready for browser QA, then commit/merge.
**Just-merged:** PR #155 (Escalation Mode wedge) merged into main as `ac42f97`. **Just-merged:** PR #155 (Escalation Mode wedge) merged into main as `ac42f97`.
## Where this session ended ## Where this session ended
Single-PR cleanup pass after Escalation Mode browser QA, then a new feature. PR #156 was reviewed for missed bugs and three fixes were applied:
1. **Codex review fixes on the escalation branch** committed as `f10649a` (atomic claim conditional UPDATE, self-claim 403 + queue self-exclusion, preassigned handoff UUID for the compatibility payload, removed legacy `claiming` / `handleStartHere` frontend dead code that broke `tsc --noEmit`). 1. Page-level **Resolve** now treats `applied_pending` as a verifying state and patches the fix to `applied_success` before opening the resolution flow, avoiding provisional notes on a resolved session.
2. **PR #155 merged** to main via Gitea API (un-drafted, retitled, merged with a merge commit). 2. Page-level **Escalate** intercept now catches `applied_pending` as well as verifying/partial, so a pending fix cannot bypass outcome capture before handoff. Intercept copy was generalized from "Verifying state" to "still needs an outcome."
3. **New branch `feat/fix-pending-verification`** off main, single commit `00663a4`. Adds `applied_pending` non-terminal status + `pending_reason` column + `PendingBanner` UI + 4 new integration tests. Rebased onto post-merge main. 3. `PendingBanner` now includes a **Dismiss** action, matching the PR body and backend's allowed pending → dismissed transition.
4. **PR #156 opened** for the new feature. 4. Real-looking pending examples were removed from `resolution_note_generator` and `escalation_package_generator` system prompts to stay aligned with the prompt anti-parrot guardrail.
5. **Cleanup:** 10 stray `core.*` dump files removed from the worktree; merged `feat/escalation-metric-endpoint` deleted locally and on the remote.
**Validation on PR #156:** **Validation on PR #156:**
- `pytest tests/test_fix_outcome_endpoint.py` ✅ 21/21 pass (including 4 new pending tests). - `docker exec resolutionflow_backend pytest --override-ini="addopts=" tests/test_prompt_anti_parrot.py -q` ✅ 2/2 pass.
- `tsc --noEmit -p tsconfig.app.json` ✅ exit 0. - `docker exec resolutionflow_backend pytest --override-ini="addopts=" tests/test_fix_outcome_endpoint.py -q` ✅ 21/21 pass.
- `alembic upgrade heads` ✅ ran `c0f3a4b7e91d` cleanly. - `docker exec -w /app resolutionflow_frontend npx tsc -b` ✅ exit 0.
- `docker exec -w /app resolutionflow_frontend npm run build` ✅ exit 0; only existing Vite large-chunk warning.
- `git diff --check` ✅ clean.
- Previous session also verified `alembic upgrade heads` applied migration `c0f3a4b7e91d` cleanly.
## Resume point — DO THIS NEXT ## Resume point — DO THIS NEXT
**Browser QA on PR #156** (see CURRENT_TASK.md "Resume point" for the 4-step checklist). Then merge. **Browser QA on PR #156** (see CURRENT_TASK.md "Resume point" for the checklist). Include the new review-fix paths: PendingBanner Dismiss, page-level Resolve from pending, and Escalate from pending. Then commit local fixes, push, and merge when QA is green.
## Key files changed this session ## Key files for PR #156
- `backend/app/models/session_suggested_fix.py` — CHECK constraint extended; `pending_reason` Text column. - `backend/app/models/session_suggested_fix.py` — CHECK constraint extended; `pending_reason` Text column.
- `backend/app/schemas/session_suggested_fix.py``applied_pending` added to both `FixStatus` and `FixOutcome` literals; `pending_reason` on response model; updated docstring on `SessionSuggestedFixOutcomeRequest`. - `backend/app/schemas/session_suggested_fix.py``applied_pending` added to both `FixStatus` and `FixOutcome` literals; `pending_reason` on response model; updated docstring on `SessionSuggestedFixOutcomeRequest`.
- `backend/alembic/versions/71efd2102f49_add_pending_status_to_suggested_fixes.py` — new migration (rev `c0f3a4b7e91d`, down `71efd2102f49`). - `backend/alembic/versions/71efd2102f49_add_pending_status_to_suggested_fixes.py` — new migration (rev `c0f3a4b7e91d`, down `71efd2102f49`).
- `backend/app/api/endpoints/session_suggested_fixes.py``patch_outcome` accepts pending, requires notes, stamps applied_at only. - `backend/app/api/endpoints/session_suggested_fixes.py``patch_outcome` accepts pending, requires notes, stamps applied_at only.
- `backend/app/services/{resolution_note,escalation_package}_generator.py` — system-prompt handling for the new status; `pending_reason` line in input bundle. - `backend/app/services/{resolution_note,escalation_package}_generator.py` — system-prompt handling for the new status; `pending_reason` line in input bundle; real-looking examples removed.
- `backend/tests/test_fix_outcome_endpoint.py` — 4 new tests. - `backend/tests/test_fix_outcome_endpoint.py` — 4 new tests.
- `frontend/src/api/sessionSuggestedFixes.ts` — types updated; `pending_reason` on `SessionSuggestedFix`. - `frontend/src/api/sessionSuggestedFixes.ts` — types updated; `pending_reason` on `SessionSuggestedFix`.
- `frontend/src/components/pilot/ProposalBanner.tsx``'pending'` `BannerMode`; `PendingBanner` component; "Waiting to verify…" overflow option; nudge "Still checking" wired to record pending. - `frontend/src/components/pilot/ProposalBanner.tsx``'pending'` `BannerMode`; `PendingBanner` component + Dismiss; "Waiting to verify…" overflow option; nudge "Still checking" wired to record pending.
- `frontend/src/pages/AssistantChatPage.tsx` — banner-mode derivation maps `applied_pending → 'pending'`. - `frontend/src/components/pilot/EscalateInterceptDialog.tsx` — copy generalized for pending/partial/verifying outcome capture.
- `frontend/src/pages/AssistantChatPage.tsx` — banner-mode derivation maps `applied_pending → 'pending'`; Resolve/Escalate paths now handle pending.
## Watch-outs ## Watch-outs
- Dev stack: backend `:8000`, frontend `:5173`, postgres `:5433` (docker-compose). HMR works. - Dev stack: backend `:8000`, frontend `:5173`, postgres `:5433` (docker-compose). HMR works. On this host use `docker exec` for Python/npm commands; see `.ai/PROJECT_CONTEXT.md`.
- Test users (Acme MSP, password `TestPass123!`): `engineer@resolutionflow.example.com`, `teamadmin@resolutionflow.example.com`. - Test users (Acme MSP, password `TestPass123!`): `engineer@resolutionflow.example.com`, `teamadmin@resolutionflow.example.com`.
- Multi-head alembic state on main is pre-existing (heads `070`, `c0f3a4b7e91d`, `024`); not introduced by this work but worth knowing if `alembic upgrade head` complains — use `upgrade heads` (plural). - Multi-head alembic state on main is pre-existing (heads `070`, `c0f3a4b7e91d`, `024`); not introduced by this work but worth knowing if `alembic upgrade head` complains — use `upgrade heads` (plural).
- `pending_reason` is preserved as audit trail when an engineer advances pending → success/failed/dismissed; it is not auto-cleared. Intentional. - `pending_reason` is preserved as audit trail when an engineer advances pending → success/failed/dismissed; it is not auto-cleared. Intentional.
- Pre-existing local branches still in the working copy: `chore/post-153-handoff`, `feat/flowpilot-migration`, `fix/ci-*`, `fix/e2e-test-selectors` — left alone. - Working tree also has documentation edits in `.ai/PROJECT_CONTEXT.md` and `AGENTS.md` describing Docker exec commands. Those were not part of the feature fix but should be preserved.

View File

@@ -12,6 +12,14 @@
--- ---
## 2026-05-01 02:24 UTC — Codex — Review-fix PR #156 pending-verification flow
- Reviewed PR #156 for bugs and found three actionable gaps: pending fixes could be resolved from the page-level Resolve path without updating the fix outcome, the PendingBanner lacked the dismiss action described in the PR body, and new system-prompt examples used real-looking pending reasons contrary to the prompt anti-parrot lesson.
- Applied fixes locally on `feat/fix-pending-verification`: page-level Resolve now patches `applied_pending` to `applied_success`; page-level Escalate now intercepts `applied_pending` before handoff; PendingBanner now has Dismiss; escalation intercept copy no longer says only "Verifying state"; generator prompts no longer include real-looking pending examples.
- Verified via running containers: prompt anti-parrot guardrail `2 passed`, suggested-fix outcome suite `21 passed`, frontend `npx tsc -b` clean, frontend `npm run build` clean except the existing Vite large-chunk warning, and `git diff --check` clean.
- Left for next session: browser QA PR #156 using CURRENT_TASK.md checklist, then commit/push local review fixes and merge.
- Files touched: `backend/app/services/resolution_note_generator.py`, `backend/app/services/escalation_package_generator.py`, `frontend/src/components/pilot/ProposalBanner.tsx`, `frontend/src/components/pilot/EscalateInterceptDialog.tsx`, `frontend/src/pages/AssistantChatPage.tsx`, `.ai/HANDOFF.md`, `.ai/CURRENT_TASK.md`, `.ai/SESSION_LOG.md`.
## 2026-04-30 — Claude Code — Land PR #155, ship pending-verification feature on PR #156 ## 2026-04-30 — Claude Code — Land PR #155, ship pending-verification feature on PR #156
- Committed Codex's review-pass changes (atomic conditional `UPDATE` for `claim_session`, self-claim 403, queue self-exclusion, pre-flush handoff UUID, frontend dead-code removal) as `f10649a` on `feat/escalation-metric-endpoint`. - Committed Codex's review-pass changes (atomic conditional `UPDATE` for `claim_session`, self-claim 403, queue self-exclusion, pre-flush handoff UUID, frontend dead-code removal) as `f10649a` on `feat/escalation-metric-endpoint`.

View File

@@ -64,8 +64,8 @@ provided. State that it did not resolve the issue.
- applied_partial: Include the fix as a partially tried path. Include partial \ - applied_partial: Include the fix as a partially tried path. Include partial \
notes if provided. Indicate it was not fully completed or not verified. notes if provided. Indicate it was not fully completed or not verified.
- applied_pending: List the fix as applied but awaiting verification. Include \ - applied_pending: List the fix as applied but awaiting verification. Include \
the pending reason if provided (e.g. "client power-cycling router"). Make it \ the pending reason if provided. Make it clear the next engineer should follow \
clear the next engineer should follow up to confirm it worked. up to confirm it worked.
- applied_success: Note that the fix was applied and verified but escalation \ - applied_success: Note that the fix was applied and verified but escalation \
is still needed for another reason (unusual — reflect this accurately). is still needed for another reason (unusual — reflect this accurately).
- dismissed: Do not mention the fix as a tried path; it was only considered. - dismissed: Do not mention the fix as a tried path; it was only considered.

View File

@@ -84,7 +84,7 @@ that actual resolution, not just the failed attempt.
- applied_partial: Note that the fix was partially applied. If partial_notes \ - applied_partial: Note that the fix was partially applied. If partial_notes \
are provided, include them. Then describe the final resolution path taken. are provided, include them. Then describe the final resolution path taken.
- applied_pending: Note that the fix was applied and verification is pending. \ - applied_pending: Note that the fix was applied and verification is pending. \
If pending_reason is provided, include it (e.g. "awaiting client power-cycle"). \ If pending_reason is provided, include it as the provided waiting reason. \
Frame the resolution as provisional — the fix is in place but not yet \ Frame the resolution as provisional — the fix is in place but not yet \
confirmed. Do not write closure language. confirmed. Do not write closure language.
- dismissed: Treat the fix as considered and set aside. Do not center the note \ - dismissed: Treat the fix as considered and set aside. Do not center the note \

View File

@@ -49,8 +49,8 @@ export function EscalateInterceptDialog({
Before escalating what happened with the fix? Before escalating what happened with the fix?
</div> </div>
<div className="text-[12px] text-muted-foreground leading-[1.5] mb-3"> <div className="text-[12px] text-muted-foreground leading-[1.5] mb-3">
&ldquo;{fixTitle}&rdquo; is still in the Verifying state. Tag its outcome so &ldquo;{fixTitle}&rdquo; still needs an outcome. Tag it so the senior
the senior picking this up knows what&apos;s been tried. picking this up knows what&apos;s been tried.
</div> </div>
<div className="flex flex-col gap-1.5"> <div className="flex flex-col gap-1.5">
<button <button

View File

@@ -260,7 +260,7 @@ function PartialBanner({ fix, onOutcome, onApply }: ProposalBannerProps) {
) )
} }
function PendingBanner({ fix, onOutcome }: ProposalBannerProps) { function PendingBanner({ fix, onOutcome, onDismiss }: ProposalBannerProps) {
return ( return (
<div className="relative border-t border-info/30 bg-gradient-to-b from-info-dim/40 to-info-dim/20 px-5 py-3 animate-slide-up"> <div className="relative border-t border-info/30 bg-gradient-to-b from-info-dim/40 to-info-dim/20 px-5 py-3 animate-slide-up">
<div className="absolute left-0 top-0 bottom-0 w-[3px] bg-info" /> <div className="absolute left-0 top-0 bottom-0 w-[3px] bg-info" />
@@ -286,6 +286,12 @@ function PendingBanner({ fix, onOutcome }: ProposalBannerProps) {
)} )}
</div> </div>
<div className="flex items-center gap-2 shrink-0 pt-0.5"> <div className="flex items-center gap-2 shrink-0 pt-0.5">
<button
onClick={onDismiss}
className="px-3 py-[9px] rounded-lg text-muted-foreground text-[12.5px] hover:bg-white/[0.08] hover:text-primary"
>
Dismiss
</button>
<button <button
onClick={() => { onClick={() => {
const reason = window.prompt( const reason = window.prompt(

View File

@@ -966,7 +966,8 @@ export default function AssistantChatPage() {
const inVerifyState = const inVerifyState =
activeFix && ( activeFix && (
(!!activeFix.applied_at && activeFix.status === 'proposed') || (!!activeFix.applied_at && activeFix.status === 'proposed') ||
activeFix.status === 'applied_partial' activeFix.status === 'applied_partial' ||
activeFix.status === 'applied_pending'
) )
if (inVerifyState && activeFix) { if (inVerifyState && activeFix) {
setEscalateIntercept({ fixId: activeFix.id, fixTitle: activeFix.title }) setEscalateIntercept({ fixId: activeFix.id, fixTitle: activeFix.title })
@@ -1004,10 +1005,15 @@ export default function AssistantChatPage() {
} }
}, [activeChatId, escalateIntercept]) }, [activeChatId, escalateIntercept])
// Phase 8: Resolve click — auto-mark applied_success if in verifying state // Phase 8: Resolve click — auto-mark applied_success if in verifying/pending state
// before opening the resolution note preview. // before opening the resolution note preview.
const handleResolveClick = useCallback(async () => { const handleResolveClick = useCallback(async () => {
if (activeFix && activeFix.applied_at && activeFix.status === 'proposed' && activeChatId) { const shouldMarkFixSuccessful =
activeFix
&& activeFix.applied_at
&& (activeFix.status === 'proposed' || activeFix.status === 'applied_pending')
&& activeChatId
if (shouldMarkFixSuccessful) {
try { try {
const updated = await sessionSuggestedFixesApi.patchOutcome(activeChatId, activeFix.id, 'applied_success') const updated = await sessionSuggestedFixesApi.patchOutcome(activeChatId, activeFix.id, 'applied_success')
setActiveFix(updated) setActiveFix(updated)