diff --git a/documentation/backlog/bugs.md b/documentation/backlog/bugs.md index 186df9e..8ed8906 100644 --- a/documentation/backlog/bugs.md +++ b/documentation/backlog/bugs.md @@ -63,6 +63,7 @@ Pending bugs carry the same diminishing-returns score defined in `improvements.m | 553 | [The Assisted card and the Copy Application Packet can describe the same form differently](#553-the-assisted-card-and-the-copy-application-packet-can-describe-the-same-form-differently) | Minor | Pending | 0.7 = 2×1.0÷3 | standard | Filed 2026-08-15 by #548's inverse-regression reviewer. In one narrow state — a fill that reached a form which was never preflighted and recorded no questions — `CompletedSummary` says Career Agent attempted the form while `CopyPacket`, one panel below on the same card, says "Career Agent has not read this employer's form". `DeriveFormInventory` deliberately does not consult `fill_attempted_at` (ADR-007 decision 8), which is right for its own question and is what leaves the two panels able to disagree. Rarer since #548's marker moved behind the fill's guards, but not closed. | | 555 | [Known profile values do not reliably prefill employer forms in Assisted Fill](#555-known-profile-values-do-not-reliably-prefill-employer-forms-in-assisted-fill) | Major | Done (2026-08-16) | — | standard | See `documentation/backlog_history/bugs_done_details.md` item #555 for the full account. | | 556 | [The role gate admits management-track titles ("Director of DevOps") because it only ever checks keyword overlap, never career track vs. seniority](#556-the-role-gate-admits-management-track-titles-director-of-devops-because-it-only-ever-checks-keyword-overlap-never-career-track-vs-seniority) | Major | Done (2026-08-17) | — | standard | See `documentation/backlog_history/bugs_done_details.md` item #556 for the full account. | +| 557 | [A revalidated legacy (CAPTCHA/manual-account) handoff loses its confirmation control once the browser closes, even after a genuine submit](#557-a-revalidated-legacy-captchamanual-account-handoff-loses-its-confirmation-control-once-the-browser-closes-even-after-a-genuine-submit) | Minor | Done (2026-08-20) | — | standard | Found live during the controlled dogfood run (application #4, Wurl, job 1240, `BLOCKED_CAPTCHA`-origin). `actionForRevalidation`'s `application_ready` case gated `RequiresExplicitSubmit` on `status == "AWAITING_REVIEW"` literally, so a `BLOCKED_CAPTCHA`/`MANUAL_REQUIRED`-origin job that skipped the guided Continue step and was submitted directly in the assisted browser had no "I saw a confirmation — Mark Applied" control left once the browser closed. Fixed by gating on `isAssistedEligibleStatus(status)` instead — the same predicate `ConfirmAssistedSubmission` itself already uses — so the UI affordance can never be narrower than what the backend will actually accept. See `documentation/backlog_history/bugs_done_details.md` item #557 for the full account. | | 546 | [The fill path logs the operator's own address values, and nothing downstream strips them](#546-the-fill-path-logs-the-operators-own-address-values-and-nothing-downstream-strips-them) | Major | Fixed (2026-08-14) | — | mechanical | Found by the adversarial pass of the same audit. `fillComboboxFromCandidates` logged `%q` of the value it typed; Location and Country are required react-select fields on every Greenhouse form, so `career_agent.log` holds the operator's home city, state and country. `SanitizeChildLogLine` strips markup and truncates and passes clean prose through, so `cmd/assist`'s first fill would put them in `dashboard.log` too. | | 545 | [Lever's custom question cards extract a placeholder, an option, or a raw name attribute instead of the question](#545-levers-custom-question-cards-extract-a-placeholder-an-option-or-a-raw-name-attribute-instead-of-the-question) | Minor | Done (2026-08-14) | — | deep-reasoning | See `documentation/backlog_history/bugs_done_details.md` item #545 for the full account. Re-scored 1.3 -> 2.0 (V 4 -> 6) before working, on a live queue measurement: Lever is 20 of 26 actionable applications and is the ATS that must be finished by hand, so it is where the Copy Application Packet is the product. | | 544 | [A skill-scoped experience question is answered with the operator's total career years](#544-a-skill-scoped-experience-question-is-answered-with-the-operators-total-career-years) | Major | Fixed (2026-08-13) | — | standard | Found by reading `patterns.go` while planning the Application Knowledge work. `years_experience` needs only a duration token and `experience`, is Routine, and every Routine pattern auto-fills — so "How many years of Kubernetes experience?" typed the operator's whole career total onto a real screening question. A fabricated qualification, submitted under their name. | diff --git a/documentation/backlog_history/bugs_done_details.md b/documentation/backlog_history/bugs_done_details.md index 9f83ccb..9368d27 100644 --- a/documentation/backlog_history/bugs_done_details.md +++ b/documentation/backlog_history/bugs_done_details.md @@ -5474,3 +5474,35 @@ No findings required a further code change beyond what is described above. - `documentation/backlog/bugs.md` - `documentation/backlog_history/bugs_done_details.md` +## 557. A revalidated legacy (CAPTCHA/manual-account) handoff loses its confirmation control once the browser closes, even after a genuine submit + +**Found and closed 2026-08-20**, live, during the controlled five-application dogfood run (`documentation/backlog_history` process — application #4 of cohort #1). Not a speculative audit finding: it stopped the run. + +### Root cause + +`pkg/storage/assisted.go`'s `actionForRevalidation`, in its `"application_ready"` case (the state a job lands in once `revalidateAssistedPlan` has re-checked the live employer page against the queued role and the browser is not currently open), decided whether to render the "I saw a confirmation — Mark Applied" control with a literal `status == "AWAITING_REVIEW"` check. That check was #518's fix: before it, *no* revalidated job could be confirmed once its browser closed. #518 closed the gap for `AWAITING_REVIEW`-origin jobs only, and a test (`TestActionForRevalidation_OtherStatusesUnchanged`) was written asserting `BLOCKED_CAPTCHA` and `MANUAL_REQUIRED` must *not* get the control, reasoning that "a CAPTCHA-blocked page has no prepared form to have been submitted." + +That reasoning held for a job that had never been opened, but not for one that had. Wurl (job 1240) was a `BLOCKED_CAPTCHA`-origin legacy handoff with `assisted_attempt_count = 2` already on it — prior sessions had opened its browser and reached `needs_answers` with two real questions (visa sponsorship, salary expectation) pending. Relaunching it for the dogfood run reset `assisted_state` back to `waiting_human` (launch always does this, regardless of prior progress — a separate, pre-existing behavior this fix does not change). The operator then filled and submitted the real Greenhouse form directly in the browser without returning to click "I completed this step — Continue" first (the same shortcut had already been taken, harmlessly, on an `AWAITING_REVIEW`-origin job earlier in the same run). Closing the browser released the lease but left `assisted_state` at `waiting_human`, which falls through to `actionForRevalidation`'s `"application_ready"` case — and because the job's *original* status was `BLOCKED_CAPTCHA`, not `AWAITING_REVIEW`, `RequiresExplicitSubmit` came back `false`. The operator had a genuine Greenhouse thank-you page and no way to tell Career Agent so. + +`ConfirmAssistedSubmission` itself was never the problem — it already accepts any status in `eligibleAssistedStatuses` (`AWAITING_REVIEW`, `MANUAL_REQUIRED`, `BLOCKED_CAPTCHA`). The gap was purely that the UI-facing gate in `actionForRevalidation` was narrower than the backend gate it was supposed to front for. + +### Fix + +`actionForRevalidation`'s `"application_ready"` case now gates on `isAssistedEligibleStatus(status)` instead of the `AWAITING_REVIEW` literal, for both the instruction text and `RequiresExplicitSubmit`. This is the same predicate `ConfirmAssistedSubmission` already uses, so the confirm control can no longer be narrower than what the backend will actually accept — closing the same class of gap #518 was meant to close, for every eligible origin status rather than one. + +Considered and rejected: gating the widened affordance on `assisted_attempt_count > 0` as well, to avoid ever showing "Mark Applied" for a `BLOCKED_CAPTCHA` job that was never opened. Rejected because the `AWAITING_REVIEW` path already carries this exact same shape of risk today (it shows the control on a bare `application_ready` page match, with no attempt-count check, and always has) — adding an asymmetric extra gate only for the other two statuses would be a new, narrower behavior than the one #518 already shipped and accepted, not a fix to what broke. The project's existing position is that confirmation is a deliberate human action and the UI does not try to prevent every way the button could be clicked in error; this fix keeps that position consistent across all three eligible statuses instead of introducing a fourth, different one. + +### Verification + +- `pkg/storage/assisted_test.go`: `TestActionForRevalidation_OtherStatusesUnchanged` (which asserted the now-falsified assumption) replaced with `TestActionForRevalidation_EligibleStatusesAllowConfirmation` (all three eligible statuses get the control) and `TestActionForRevalidation_IneligibleStatusUnchanged` (an empty/unrecognized status still does not). +- `go build ./...`, `go vet ./...`, `gofmt -l ./cmd ./pkg ./internal` all clean. +- `go test ./...`: full suite passes except `TestServeKnowledgeProfile_KeepsBackupsBoundedAndPrivate`, confirmed pre-existing and unrelated on `main` before this change (a timestamp-collision flake in PII backup naming, `cmd/dashboard/knowledge_api_test.go`) — not touched by this fix. +- **Live-verified end to end**, not just by test: rebuilt and restarted the running dashboard binary (no live assisted browser was open at the time; the agent daemon was left untouched, since this fix is dashboard-only queue-projection logic it never calls). Wurl's card regained "I saw a confirmation — Mark Applied", the operator clicked it for the real prior submission, and `ConfirmAssistedSubmission` committed atomically: `assisted_state` → `completed` (`confirmation_provenance = manual_user_confirmation`), `job_funnel.status` → `APPLIED`, `dogfood_cohort_applications` ordinal 4 for job 1240. The dogfood cohort advanced from 3/5 to 4/5 with no duplicate row and no other job's state touched. + +### Files changed + +- `pkg/storage/assisted.go` +- `pkg/storage/assisted_test.go` +- `documentation/backlog/bugs.md` +- `documentation/backlog_history/bugs_done_details.md` + diff --git a/pkg/storage/assisted.go b/pkg/storage/assisted.go index 3d17fd2..9587f09 100644 --- a/pkg/storage/assisted.go +++ b/pkg/storage/assisted.go @@ -678,19 +678,25 @@ func actionForRevalidation(status, reason, state string) AssistedNextAction { case "captcha_confirmed": return actionForLegacy(status, reason) case "application_ready": - // AWAITING_REVIEW means the pipeline already filled the form and - // stopped before submitting, so the only remaining steps are the - // operator's own Submit and their confirmation that the employer - // received it. Without RequiresExplicitSubmit the dashboard never - // renders "I saw a confirmation — Mark Applied", which left a - // genuinely submitted application with no way to be confirmed from - // the UI once its browser closed (bugs.md #518). Revalidating a - // candidate is routine, so this affected every assisted application. + // Any status ConfirmAssistedSubmission itself will accept means the + // pipeline already filled the form and stopped before submitting, so + // the only remaining steps are the operator's own Submit and their + // confirmation that the employer received it. Without + // RequiresExplicitSubmit the dashboard never renders "I saw a + // confirmation — Mark Applied", which left a genuinely submitted + // application with no way to be confirmed from the UI once its + // browser closed (bugs.md #518, originally fixed for AWAITING_REVIEW + // only). Revalidating a candidate is routine, so this affected every + // assisted application regardless of origin status (bugs.md #557: a + // BLOCKED_CAPTCHA-origin job reached and submitted a real form the + // same way, and lost the same confirmation control the same way once + // its browser closed without the guided Continue step). instruction := "The current page matches this role and shows an application entry point. Review it before providing any information or submitting." - if status == "AWAITING_REVIEW" { + eligible := isAssistedEligibleStatus(status) + if eligible { instruction = "The prepared application is ready. Review it, submit it yourself on the employer's site, then mark it applied only after the employer confirms it was received." } - return AssistedNextAction{"open_verified_application", "Application ready", instruction, "Open Verified Application", true, false, status == "AWAITING_REVIEW", false} + return AssistedNextAction{"open_verified_application", "Application ready", instruction, "Open Verified Application", true, false, eligible, false} case "unavailable": return AssistedNextAction{"revalidate_current_page", "Current application is not ready", "The current page did not verify this role and an application entry point. No browser was opened.", "Check Current Page Again", false, false, false, false} case "unreachable": diff --git a/pkg/storage/assisted_test.go b/pkg/storage/assisted_test.go index 7e41285..9801d12 100644 --- a/pkg/storage/assisted_test.go +++ b/pkg/storage/assisted_test.go @@ -711,32 +711,40 @@ func TestAssistedDocumentExists_CoverLetterReadinessFollowsTheMasterLetter(t *te } } -// A revalidated AWAITING_REVIEW handoff must still expose the explicit-submit -// affordance. Without it the dashboard never renders "I saw a confirmation — -// Mark Applied", so an application the operator genuinely submitted cannot be -// confirmed from the UI once its browser closes (bugs.md #518). Revalidation -// is routine before every launch, so this affected every assisted application. -func TestActionForRevalidation_AwaitingReviewAllowsConfirmation(t *testing.T) { - action := actionForRevalidation("AWAITING_REVIEW", "", "application_ready") - if action.Code != "open_verified_application" { - t.Errorf("Code = %q, want open_verified_application", action.Code) - } - if !action.RequiresExplicitSubmit { - t.Error("RequiresExplicitSubmit = false; the Mark Applied control would not render") - } - if !action.RequiresBrowser { - t.Error("RequiresBrowser = false, want true") +// A revalidated handoff of any status ConfirmAssistedSubmission itself +// accepts must still expose the explicit-submit affordance. Without it the +// dashboard never renders "I saw a confirmation — Mark Applied", so an +// application the operator genuinely submitted cannot be confirmed from the +// UI once its browser closes (bugs.md #518, then #557). #518 fixed this for +// AWAITING_REVIEW only; #557 found a BLOCKED_CAPTCHA-origin job (Wurl, job +// 1240, live dogfood run 2026-08-20) reach and submit a real form the exact +// same way and lose the exact same control the exact same way, which +// falsified the assumption the old version of this test asserted — that a +// CAPTCHA-blocked page never has a prepared form to have been submitted. It +// can, once the CAPTCHA is solved and the browser is later closed without the +// guided Continue step. +func TestActionForRevalidation_EligibleStatusesAllowConfirmation(t *testing.T) { + for _, status := range []string{"AWAITING_REVIEW", "BLOCKED_CAPTCHA", "MANUAL_REQUIRED"} { + action := actionForRevalidation(status, "", "application_ready") + if action.Code != "open_verified_application" { + t.Errorf("status %q: Code = %q, want open_verified_application", status, action.Code) + } + if !action.RequiresExplicitSubmit { + t.Errorf("status %q: RequiresExplicitSubmit = false; the Mark Applied control would not render", status) + } + if !action.RequiresBrowser { + t.Errorf("status %q: RequiresBrowser = false, want true", status) + } } } -// Other statuses must not gain a confirmation affordance they never had: a -// CAPTCHA-blocked page has no prepared form to have been submitted. -func TestActionForRevalidation_OtherStatusesUnchanged(t *testing.T) { - for _, status := range []string{"BLOCKED_CAPTCHA", "MANUAL_REQUIRED", ""} { - action := actionForRevalidation(status, "", "application_ready") - if action.RequiresExplicitSubmit { - t.Errorf("status %q: RequiresExplicitSubmit = true, want false", status) - } +// A status ConfirmAssistedSubmission itself would refuse must not gain a +// confirmation affordance either — there is no eligible funnel row for it to +// commit against. +func TestActionForRevalidation_IneligibleStatusUnchanged(t *testing.T) { + action := actionForRevalidation("", "", "application_ready") + if action.RequiresExplicitSubmit { + t.Error("RequiresExplicitSubmit = true for an ineligible status, want false") } }