From 55fb834728fa7b44c72277edc51c6db40a965d6f Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 26 Oct 2026 11:48:41 +0100 Subject: [PATCH 1/9] docs(plan): bound TUI guide and findings slice --- .../initiatives/WS-CLI-001/WS-CLI-001-49.md | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 .commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md new file mode 100644 index 000000000..b17b999a1 --- /dev/null +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md @@ -0,0 +1,79 @@ +# WS-CLI-001-49 — Contributor guide and findings in the terminal + +- Initiative: `WS-CLI-001` +- Durable disposition: `Planned` +- Intended merge outcome: Replace guide/findings handoffs with bounded interactive reads and explicit verified guide downloads. + +## Intent and current behavior + +Issue #688 tracks the remaining terminal experience. Main's read-only workspace +opens exact task/context/intake and Submission details but sends users back to +explicit commands for detailed findings and guide originals. The public APIs +and their validation already exist. This slice removes those handoffs without +adding product mutations or changing backend authority. + +## Design and scope + +Extend existing TUI navigation, request generations, cancellation and safe text +rendering. WorkContext supplies assignment-sensitive ordered guide metadata; +DownloadGuideDocument verifies exact same-origin content, SHA-256 and byte count. +Use an explicit destination entry and confirmation before downloading. Extract +the existing CLI private-file/no-overwrite publication operation into a shared +owner if needed; both callers must use it, not duplicate filesystem policy. +Keep binary documents out of terminal rendering; no system viewer launch or +shell execution. Metadata must never be labelled verified bytes. + +Expose typed paged findings through the existing validated SubmissionFindings +operation, updating affected explicit callers rather than adding a second +transport/decoder. Display exact run identity/current-versus-historical state, +checker/check ID, status, severity, path/line, message and suggested fix. +Continuation is explicit and bound to selected Submission/task; never infer +acceptance from checker success. Navigation/refresh invalidate stale responses; +denial clears affected observations. Every displayed untrusted value uses the +existing terminal sanitizer and bearer suppression. + +Allowed paths: cli/internal/tui/, affected api/submission_reads.go and its +callers/tests, existing guide download command and a narrowly shared download +owner, cli/tests/integration/test_tui_http.py and focused guide/findings process +tests, cli/README.md, docs/design/cli-terminal-experience.md, +docs/roadmap_status.md and affected WS-CLI-001 records/index when their next +boundary changes. No backend, dependencies, workflow or release changes. + +## Boundaries and alternatives + +Do not add claim/start/prepare/submit, polling, credential storage, local replay +journals, PDF rendering, automatic viewer launch, task examples, directory +creation, overwrite, or download-all implicit chains. Existing explicit commands +remain available. A separate raw-HTTP TUI client or duplicate download helper +was rejected because it would fork validation and custody rules. + +## Risk and review + +L1: untrusted terminal data and local filesystem publication. Plan review before +implementation; focused security, QA and reuse/architecture review of affected +owners; documentation review for capability claims. Human merge remains required. +No new authority decision is needed; if a backing contract is missing, report +the gap rather than invent it. + +## Acceptance and proof + +- Assigned task guide screen preserves exact server ordering, identities and + hashes; no documents for an unassigned task or task-example disclosure. +- Explicit download writes private temporary content; checksum/size failure or + missing bytes publishes no final file. Existing files/symlinks are not replaced. + Cancellation/navigation cannot silently publish an unobserved success. +- Findings load only for the selected Submission and task, preserve all returned + statuses and historical/current run distinctions, and follow explicit cursors. +- Denial, malformed/misbinding responses, token/control-character content, + late responses and quit remain safe. Narrow/plain terminals stay usable. +- Focused Go tests and real socket/PTY process tests exercise new screens and + failure boundaries. Existing native suite runs unchanged on all four platforms; + no skipped assertions or CI weakening. Retain existing public PostgreSQL + guide/findings authorization proof; a TUI test does not certify hosted service. +- Update affected capability ledger/README/design claims together; run markdown + links, stale wording, Go tests/vet and review before merge readiness. + +## Human review focus + +Confirm destination/confirmation usability, exact locked identity visibility, +no-overwrite and integrity failure behavior, and honest checker outcome labels. From 8d5a97b9687c1374005f0742de0d5b3848cbbbfe Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 26 Oct 2026 11:52:36 +0100 Subject: [PATCH 2/9] docs(plan): specify staged publication and TUI transitions --- .../initiatives/WS-CLI-001/WS-CLI-001-49.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md index b17b999a1..3dd3f451a 100644 --- a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md @@ -77,3 +77,43 @@ the gap rather than invent it. Confirm destination/confirmation usability, exact locked identity visibility, no-overwrite and integrity failure behavior, and honest checker outcome labels. + +## Concrete state and owner contract + +Mine detail keeps Overview, Context and Intake in their existing order, then +adds Guide. Guide freshly reads WorkContext; arrows select one ordered document. +`d` edits an existing-directory destination, Enter opens confirmation showing +task/document identity, directory and computed UUID filename, and `y` starts +staging. Esc cancels editing/confirmation without I/O. No directory is created. +Submission detail Tab alternates Overview and Findings. Findings fetches 50 +runs; `n` replaces that page using its exact cursor, `r` restarts at page one. +Back, refresh, project changes, denial and quit invalidate pending work. + +The narrow shared owner is `cli/internal/download/`. Move the bound-root and +private-publication behavior from `command/guide_reads.go` there and update +all consumers: `command/task_guide.go`, `command/guide_reads.go` and +`command/task_batch_source.go`. CLI consumers stage then publish synchronously; +do not preserve duplicate implementations. TUI registers an abortable stage +before dispatch. Async work can only write/verify/sync/close temporary bytes. +Only the current-generation receive may synchronously publish the no-overwrite +hard link and set the visible success state, before another key is processed. +Abort/Publish are idempotent and race-serialized. Invalidation/Close marks an +in-flight stage aborted; cleanup does not require delivery of its response, +including on Windows where a writer may still hold an open file. + +SubmissionFindings returns a typed one-of result containing Raw plus Page or +Run (only the mode-appropriate member populated), using its existing decoder. +CLI JSON and text continue rendering Raw unchanged; TUI consumes Page. Receive +checks every run's task and Submission against selected row/detail, as well as +request cursor, generation and selector. Preserve nullable fields, predecessor +and current-run flag. No alternative decoder or transport is introduced. + +Named proof additions: Go typed page/exact-run and unchanged raw-output tests; +TUI selection/confirmation/current-receipt tests; download stage tests for +bound/symlink directories, existing file/symlink, 0600 temporary content, +checksum/size/missing/transport failure, and delayed writer Abort cleanup. +Extend test_tui_http.py and its tui_terminal_support.py binary-response fixture +to exercise actual PTY navigation, explicit confirmation, download success, +cancel/quit before delayed completion, findings pagination/misbinding, every +finding status and hostile text. These tests remain in the native four-platform +suite. Retain prior input, denial and terminal lifecycle assertions. From ec90c8a88d134c633b2e1e9532f68c844f5bd45e Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 26 Oct 2026 12:07:11 +0100 Subject: [PATCH 3/9] feat(tui): browse findings and download verified locked guides --- .commitrail/INDEX.md | 2 +- .../initiatives/WS-CLI-001/OVERVIEW.md | 11 +- cli/README.md | 24 +- cli/internal/api/submission_reads.go | 48 ++- cli/internal/api/submission_reads_test.go | 66 +++++ cli/internal/command/guide_reads.go | 73 +---- cli/internal/command/submission.go | 6 +- cli/internal/command/task_batch_source.go | 18 +- cli/internal/command/task_guide.go | 41 +-- cli/internal/download/download.go | 142 +++++++++ cli/internal/download/download_test.go | 130 +++++++++ cli/internal/tui/guide_findings_test.go | 198 +++++++++++++ cli/internal/tui/handoff.go | 5 +- cli/internal/tui/model.go | 120 +++++++- cli/internal/tui/model_test.go | 22 ++ cli/internal/tui/requests.go | 158 +++++++++- cli/internal/tui/view.go | 127 +++++++- cli/tests/integration/test_tui_http.py | 273 +++++++++++++++++- cli/tests/integration/tui_terminal_support.py | 14 +- docs/design/cli-terminal-experience.md | 30 +- docs/roadmap_status.md | 15 +- 21 files changed, 1370 insertions(+), 153 deletions(-) create mode 100644 cli/internal/api/submission_reads_test.go create mode 100644 cli/internal/download/download.go create mode 100644 cli/internal/download/download_test.go create mode 100644 cli/internal/tui/guide_findings_test.go diff --git a/.commitrail/INDEX.md b/.commitrail/INDEX.md index 9a8d920fd..6609615a6 100644 --- a/.commitrail/INDEX.md +++ b/.commitrail/INDEX.md @@ -11,7 +11,7 @@ for current product capability. | [WS-AGENT-001](initiatives/WS-AGENT-001/OVERVIEW.md) | Planned | Neutral document tools plus OpenAI Agents SDK and OmniCoreAgent original-guide qualifications are delivered through the same public API, durable worker, parser and MinIO path. Retain the exact external provider/account and selected-image AWS evidence, then prove the broader hosted Flow contribution journey before deployment claims. | | [WS-DB-002](initiatives/WS-DB-002/OVERVIEW.md) | Complete | Shared UUIDv7 record generation, native-UUID relationships and fresh v0.1 baseline; natural-owner retry custody and aligned CI/local setup | | [WS-MCP-002](initiatives/WS-MCP-002/OVERVIEW.md) | Planned | 108 tools through WS-MCP-002-12 cover access, guide setup, contributor workspace, contribution policies, checker/artifact operations and task execution/submissions; task operations/imports and assembled release proof remain. | -| [WS-CLI-001](initiatives/WS-CLI-001/OVERVIEW.md) | Planned | Public Go CLI setup, contributor/manager workflows, administrative and operational commands, Finance policies and contribution exports are detailed in the overview; -43 proves the connected local unpaid automated-acceptance journey. -44 prepares four native release archives and a gated manual publisher; -47 hardens its administrator-bypass guard. Protected release-environment and immutable-tag ruleset configuration/verification, first publication and public-asset audit remain. -46 adds the separate system-reader project catalogue; -48 adds the opt-in read-only contributor TUI. Detailed findings/Evidence remain manual CLI handoffs; TUI writes/management, public human-review controls and hosted proof are separate. Current revision context never substitutes for historical evidence. | +| [WS-CLI-001](initiatives/WS-CLI-001/OVERVIEW.md) | Planned | Public Go CLI setup, contributor/manager workflows, administrative and operational commands, Finance policies and contribution exports are detailed in the overview; -43 proves the connected local unpaid automated-acceptance journey. -44 prepares four native release archives and a gated manual publisher; -47 hardens its administrator-bypass guard. First publication and public-asset audit remain operational work. -46 adds the separate system-reader project catalogue; -48 adds contributor TUI navigation and -49 adds findings pages and confirmed verified guide downloads. Evidence remains a manual CLI handoff; TUI product writes/management, public human-review controls and hosted proof are separate. Current revision context never substitutes for historical evidence. | | [WS-ARCH-001](initiatives/WS-ARCH-001/OVERVIEW.md) | Planned | Public admission-backed initial/remediation Submission entry and supported false-policy guide activation are implemented over selected production delivery. New false/pass effects require LIVE REV; a known pre-effect non-live refusal has bounded retry, with exhausted delivery dead-lettered. The [bounded local public/worker drill](initiatives/WS-ARCH-001/planning/PLAN.md#first-complete-contributor-milestone) starts from public guide inputs with recorded inference; hosted operation and live human review remain. | | [WS-ART-001](initiatives/WS-ART-001/OVERVIEW.md) | Planned | Verified input/output custody, exact-cause replacement and public admission-backed ZIP/Submission entry are delivered. The bounded local public/worker drill passed; hosted operation and setup-fault handling remain separate. | | [WS-AUTH-001](initiatives/WS-AUTH-001/OVERVIEW.md) | Planned | Exact fixed-router custody, checker remediation and checker retry are delivered. The ART Operator backend prerequisite adds exact system-Operator artifact reads and independently guarded verification retry; WS-CLI-001-43 consumes all nine public ART operations, while hosted S3 proof remains separate. Public entry and supported false-guide activation use current exact AUTH/SQL custody; human decision authority remains later work. | diff --git a/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md b/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md index e7bdb289b..c599ef76c 100644 --- a/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md +++ b/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md @@ -60,8 +60,10 @@ It does not claim a tag, published binary or signed/notarized artifact. [WS-CLI-001-48](WS-CLI-001-48.md) adds the opt-in read-only contributor workbench: exact-grant projects, one-page Ready/active Mine/own Submissions, -fresh details and explicit Mine context/intake reads. Detailed findings and -contribution evidence remain inert CLI handoffs. All mutations, management +fresh details and explicit Mine context/intake reads. +[WS-CLI-001-49](WS-CLI-001-49.md) adds exact-Submission findings pages and +ordered locked-guide selection with explicit verified single-document download. +Contribution evidence remains an inert CLI handoff. Product mutations, management screens and operation-specific recovery remain separate TUI boundaries; this interface does not depend on first binary publication or system-wide discovery. @@ -422,8 +424,9 @@ CLIs. Keep the package independent of backend and MCP runtime dependencies. journey through contribution export, with negative intake and authority checks. Hosted and human-review journey proof remain separate. Unfinished human review/revision is not current CLI implementation scope. -26. **Optional TUI:** -48 supplies read-only contributor navigation through - existing typed reads. In-TUI findings/Evidence, writes and management remain +26. **Optional TUI:** -48 supplies contributor navigation through existing typed + reads; -49 adds findings pages and confirmed verified guide downloads. + In-TUI Evidence, product writes and management remain separate boundaries; Evidence first needs a shared typed single-page client port without changing complete CLI exports. Never require a TUI for agents or scripts. diff --git a/cli/README.md b/cli/README.md index 386ff0c37..e45178fa6 100644 --- a/cli/README.md +++ b/cli/README.md @@ -26,8 +26,8 @@ non-terminal input/output and JSON mode before making an API request. Start with your exact-grant projects, then browse **Ready**, **Mine** (active assignments) and **Submissions**. Enter opens fresh contributor-safe task or -owned Submission detail. Active Mine details also offer explicit Context and -Intake reads. Each list fetches one page; filtering searches only that loaded +owned Submission detail. Active Mine details offer Context, Intake and Guide +sections. Submission detail also offers paged Findings. Each list fetches one page; filtering searches only that loaded page. Project membership, terms and action hints are observations, not authority for a later operation. Workstream rechecks each public read. @@ -39,15 +39,21 @@ for a later operation. Workstream rechecks each public read. | `n` / `r` | Fetch the next page / refresh the current view | | `/` | Filter loaded names/titles locally; Enter finishes and Esc cancels | | `c` / `?` | Show inert CLI arguments / keyboard help | +| `d` in Guide | Enter an existing download directory; Enter reviews the target, `y` confirms | | `q` / Ctrl+C | Quit and cancel outstanding client reads | -This first workspace is read-only: it does not claim/start work, prepare or -submit ZIPs, download documents, export records, poll, acquire tokens or save -local history. Guide metadata is not proof of downloaded-byte integrity. -Detailed findings and contribution evidence are clearly labelled manual CLI -handoffs, not populated TUI screens; displayed arguments never execute or copy -automatically. Use `submission findings SUBMISSION_ID` and -`contributions --project PROJECT_ID` explicitly when needed. +The workspace makes no product mutation requests: it does not claim/start work, +prepare or submit ZIPs, export records, poll, acquire tokens or save local +history. Guide metadata is not proof of downloaded-byte integrity. In Guide, +select one document, enter an existing real directory and confirm the exact +target. Verified bytes are published under the document's UUID filename without +overwriting an existing file. Cancelled or failed downloads are not published. +PDF/Office documents are downloaded, not rendered or opened automatically. +Findings display each returned run's identity, current flag and checker evidence; +they do not imply acceptance. `n` fetches one next findings page. +Contribution evidence remains a manual CLI handoff: use +`contributions --project PROJECT_ID`. Displayed arguments never execute or copy +automatically. A denial clears affected content; expired authentication clears the session's work data. Network failure is shown as unavailable, never as an empty queue or diff --git a/cli/internal/api/submission_reads.go b/cli/internal/api/submission_reads.go index 4b6dbaef9..0903c56a9 100644 --- a/cli/internal/api/submission_reads.go +++ b/cli/internal/api/submission_reads.go @@ -87,6 +87,13 @@ type SubmissionCheckerPage struct { Cursor *string `json:"next_cursor"` } +// SubmissionFindingsResult is one typed response shape. Exactly one member is +// populated according to whether the caller selected a page or an exact run. +type SubmissionFindingsResult struct { + Page *SubmissionCheckerPage + Run *SubmissionCheckerRun +} + func decodeSubmission(raw json.RawMessage, task, submission string) (SubmissionHistory, error) { var value SubmissionHistory bad := &Failure{Code: "invalid_api_response"} @@ -247,9 +254,10 @@ func decodeSubmissionChecker(raw json.RawMessage, submission, run string) (Submi return value, nil } -func (c *Client) SubmissionFindings(ctx context.Context, submission, run string, limit int, cursor *string) (json.RawMessage, error) { +func (c *Client) SubmissionFindings(ctx context.Context, submission, run string, limit int, cursor *string) (Result[SubmissionFindingsResult], error) { + var result Result[SubmissionFindingsResult] if !submissionSelector(submission) || (run != "" && !submissionSelector(run)) { - return nil, errors.New("submission and run must be UUIDs") + return result, errors.New("submission and run must be UUIDs") } path := "/api/v1/submissions/" + url.PathEscape(submission) + "/checker-runs" query := "" @@ -259,43 +267,57 @@ func (c *Client) SubmissionFindings(ctx context.Context, submission, run string, var err error query, err = submissionPageQuery(limit, cursor) if err != nil { - return nil, err + return result, err } } raw, err := c.requestWithResponseLimit(ctx, http.MethodGet, path, query, nil, "", http.StatusOK, maxSubmissionReadBytes) if err != nil { - return nil, err + return result, err } bad := &Failure{Code: "invalid_api_response"} if !c.safeMutationResponse(raw) { - return nil, bad + return result, bad + } + value, err := decodeSubmissionFindings(raw, submission, run, limit) + if err != nil { + return result, err } + return Result[SubmissionFindingsResult]{Raw: raw, Value: value}, nil +} + +func decodeSubmissionFindings(raw json.RawMessage, submission, run string, limit int) (SubmissionFindingsResult, error) { + var result SubmissionFindingsResult + bad := &Failure{Code: "invalid_api_response"} if run != "" { - if _, err := decodeSubmissionChecker(raw, submission, run); err != nil { - return nil, err + value, err := decodeSubmissionChecker(raw, submission, run) + if err != nil { + return result, err } - return raw, nil + result.Run = &value + return result, nil } var page SubmissionCheckerPage fields, err := contextObject(raw, &page, []string{"items"}, nil) if err != nil || !requiredNullable(fields, "next_cursor") || page.Items == nil || len(page.Items) > limit || (page.Cursor != nil && (!validSubmissionCursor(*page.Cursor) || len(page.Items) == 0)) { - return nil, bad + return result, bad } var items []json.RawMessage if json.Unmarshal(fields["items"], &items) != nil { - return nil, bad + return result, bad } seen := map[[16]byte]bool{} var task string - for _, item := range items { + for index, item := range items { value, err := decodeSubmissionChecker(item, submission, "") id, _ := uuidIdentity(value.ID) if err != nil || seen[id] || (task != "" && !sameUUID(task, value.Task)) { - return nil, bad + return result, bad } seen[id] = true task = value.Task + page.Items[index] = value } - return raw, nil + result.Page = &page + return result, nil } diff --git a/cli/internal/api/submission_reads_test.go b/cli/internal/api/submission_reads_test.go new file mode 100644 index 000000000..d6e4ea5d7 --- /dev/null +++ b/cli/internal/api/submission_reads_test.go @@ -0,0 +1,66 @@ +package api + +import ( + "encoding/json" + "testing" +) + +const ( + testSubmission = "019b9ba2-4100-7000-8000-000000000001" + testTask = "019b9ba2-4100-7000-8000-000000000002" + testRun = "019b9ba2-4100-7000-8000-000000000003" + testFinding = "019b9ba2-4100-7000-8000-000000000004" +) + +func checkerRunJSON() map[string]any { + return map[string]any{ + "id": testRun, "task_id": testTask, "submission_id": testSubmission, + "submission_version": 2, "status": "completed", "attempt_number": 1, + "supersedes_checker_run_id": nil, "is_current_for_submission": true, + "created_at": "2026-10-26T10:00:00Z", "completed_at": "2026-10-26T10:01:00Z", + "results": []any{map[string]any{ + "id": testFinding, "checker_name": "lint", "check_id": "lint.safe", + "path": "answer.md", "line": 4, "status": "warning", "severity": "medium", + "worker_message": "explain this", "worker_suggested_fix": "add detail", + }}, + } +} + +func TestDecodeSubmissionFindingsTypedPageAndRun(t *testing.T) { + runRaw, err := json.Marshal(checkerRunJSON()) + if err != nil { + t.Fatal(err) + } + exactBytes := string(runRaw) + exact, err := decodeSubmissionFindings(runRaw, testSubmission, testRun, 50) + if err != nil || exact.Run == nil || exact.Page != nil || exact.Run.ID != testRun || len(exact.Run.Results) != 1 { + t.Fatal("exact run was not typed", exact, err) + } + if string(runRaw) != exactBytes { + t.Fatal("exact raw response changed") + } + pageRaw, err := json.Marshal(map[string]any{"items": []any{checkerRunJSON()}, "next_cursor": "opaque-next"}) + if err != nil { + t.Fatal(err) + } + pageBytes := string(pageRaw) + page, err := decodeSubmissionFindings(pageRaw, testSubmission, "", 50) + if err != nil || page.Page == nil || page.Run != nil || len(page.Page.Items) != 1 || page.Page.Cursor == nil || *page.Page.Cursor != "opaque-next" { + t.Fatal("page was not typed", page, err) + } + if got := page.Page.Items[0]; got.Task != testTask || got.Results[0].Fix == nil || *got.Results[0].Fix != "add detail" { + t.Fatal("typed fields were not preserved", got) + } + if string(pageRaw) != pageBytes { + t.Fatal("page raw response changed") + } +} + +func TestDecodeSubmissionFindingsRejectsMisbinding(t *testing.T) { + run := checkerRunJSON() + run["submission_id"] = testTask + raw, _ := json.Marshal(map[string]any{"items": []any{run}, "next_cursor": nil}) + if _, err := decodeSubmissionFindings(raw, testSubmission, "", 50); err == nil { + t.Fatal("foreign Submission accepted") + } +} diff --git a/cli/internal/command/guide_reads.go b/cli/internal/command/guide_reads.go index 570558432..bcb784d73 100644 --- a/cli/internal/command/guide_reads.go +++ b/cli/internal/command/guide_reads.go @@ -1,15 +1,13 @@ package command import ( - "crypto/rand" "encoding/json" "errors" "fmt" "io" - "os" - "path/filepath" "github.com/Flow-Research/workstream/cli/internal/api" + "github.com/Flow-Research/workstream/cli/internal/download" "github.com/spf13/cobra" ) @@ -94,7 +92,7 @@ func addGuideReads(guide *cobra.Command, client func() (*api.Client, error), out if !api.ValidUUID(args[2]) { return errors.New("DOCUMENT_ID must be a UUID") } - root, err := openBoundDownloadRoot(directory) + root, err := download.OpenRoot(directory) if err != nil { return err } @@ -120,9 +118,18 @@ func addGuideReads(guide *cobra.Command, client func() (*api.Client, error), out return &api.Failure{Code: "guide_document_not_uploaded"} } name := api.TaskGuideDocument{DocumentID: selected.DocumentID, MediaType: selected.MediaType}.Filename() - if err := publishVerifiedDownload(root, name, func(target io.Writer) error { - return apiClient.DownloadProjectGuideDocument(cmd.Context(), args[0], args[1], *selected, target) - }); err != nil { + stage, err := root.Stage(name) + if err != nil { + return err + } + defer stage.Abort() + if err := apiClient.DownloadProjectGuideDocument(cmd.Context(), args[0], args[1], *selected, stage); err != nil { + return err + } + if err := stage.Seal(); err != nil { + return err + } + if err := stage.Publish(); err != nil { return err } if *output == "json" { @@ -139,55 +146,3 @@ func addGuideReads(guide *cobra.Command, client func() (*api.Client, error), out document.Flags().StringVar(&directory, "download", "", "Required existing directory for the verified original (never overwrite)") guide.AddCommand(document) } - -// openBoundDownloadRoot pins the checked directory before any remote read. -// Opening by pathname may follow a symlink swapped in after Lstat; comparing -// the opened root's inode to the checked one prevents that redirection. -func openBoundDownloadRoot(directory string) (*os.Root, error) { - if directory == "" { - return nil, errors.New("--download must be an existing real directory") - } - directory = filepath.Clean(directory) - checked, err := os.Lstat(directory) - if err != nil || !checked.IsDir() || checked.Mode()&os.ModeSymlink != 0 { - return nil, errors.New("--download must be an existing real directory") - } - root, err := os.OpenRoot(directory) - if err != nil { - return nil, errors.New("cannot open download directory") - } - opened, err := root.Stat(".") - if err != nil || !os.SameFile(checked, opened) { - _ = root.Close() - return nil, errors.New("download directory changed during open") - } - return root, nil -} - -// publishVerifiedDownload writes privately, then links the name only after -// write reports every byte verified; it never replaces an existing file. -func publishVerifiedDownload(root *os.Root, name string, write func(io.Writer) error) error { - if _, err := root.Lstat(name); !os.IsNotExist(err) { - return errors.New("download target already exists or is inaccessible") - } - temporary := ".workstream-guide-" + rand.Text() - file, err := root.OpenFile(temporary, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600) - if err != nil { - return errors.New("cannot create private guide download") - } - defer root.Remove(temporary) - defer file.Close() - if err := write(file); err != nil { - return err - } - if err := file.Sync(); err != nil { - return errors.New("cannot persist guide download") - } - if err := file.Close(); err != nil { - return errors.New("cannot close guide download") - } - if err := root.Link(temporary, name); err != nil { - return errors.New("cannot publish guide download without overwrite") - } - return nil -} diff --git a/cli/internal/command/submission.go b/cli/internal/command/submission.go index b981848d3..166638542 100644 --- a/cli/internal/command/submission.go +++ b/cli/internal/command/submission.go @@ -164,14 +164,14 @@ func addSubmissionCommands(root, task *cobra.Command, client func() (*api.Client if err != nil { return err } - raw, err := c.SubmissionFindings(cmd.Context(), args[0], run, limit, cursor) + result, err := c.SubmissionFindings(cmd.Context(), args[0], run, limit, cursor) if err != nil { return err } if *output == "json" { - return writeSafeJSON(stdout, raw) + return writeSafeJSON(stdout, result.Raw) } - return writeTaskContextFields(stdout, []contextField{{"Checker evidence (not acceptance)", raw}}) + return writeTaskContextFields(stdout, []contextField{{"Checker evidence (not acceptance)", result.Raw}}) } submission.AddCommand(findings) root.AddCommand(submission) diff --git a/cli/internal/command/task_batch_source.go b/cli/internal/command/task_batch_source.go index 3636188f3..3182ea3da 100644 --- a/cli/internal/command/task_batch_source.go +++ b/cli/internal/command/task_batch_source.go @@ -6,6 +6,7 @@ import ( "io" "github.com/Flow-Research/workstream/cli/internal/api" + "github.com/Flow-Research/workstream/cli/internal/download" "github.com/Flow-Research/workstream/cli/internal/taskimport" "github.com/spf13/cobra" ) @@ -75,7 +76,7 @@ func addTaskBatchSources(batch *cobra.Command, client func() (*api.Client, error if !api.ValidUUID(args[0]) || !api.ValidUUID(args[1]) { return commandError{"invalid_arguments", "PROJECT_ID and SOURCE_ID must be UUIDs"} } - root, err := openBoundDownloadRoot(directory) + root, err := download.OpenRoot(directory) if err != nil { return err } @@ -92,9 +93,18 @@ func addTaskBatchSources(batch *cobra.Command, client func() (*api.Client, error return &api.Failure{Code: "task_import_source_not_verified"} } name := result.Value.Filename() - if err := publishVerifiedDownload(root, name, func(target io.Writer) error { - return apiClient.DownloadTaskImportSource(cmd.Context(), args[0], args[1], result.Value, target) - }); err != nil { + stage, err := root.Stage(name) + if err != nil { + return err + } + defer stage.Abort() + if err := apiClient.DownloadTaskImportSource(cmd.Context(), args[0], args[1], result.Value, stage); err != nil { + return err + } + if err := stage.Seal(); err != nil { + return err + } + if err := stage.Publish(); err != nil { return err } if *output == "json" { diff --git a/cli/internal/command/task_guide.go b/cli/internal/command/task_guide.go index fa5e3158c..f0f03a331 100644 --- a/cli/internal/command/task_guide.go +++ b/cli/internal/command/task_guide.go @@ -1,13 +1,11 @@ package command import ( - "crypto/rand" "encoding/json" - "errors" "io" - "os" "github.com/Flow-Research/workstream/cli/internal/api" + "github.com/Flow-Research/workstream/cli/internal/download" "github.com/spf13/cobra" ) @@ -28,13 +26,9 @@ func addTaskGuide(task *cobra.Command, client func() (*api.Client, error), outpu return &api.Failure{Code: "task_assignment_required"} } if directory != "" { - info, err := os.Lstat(directory) - if err != nil || !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { - return errors.New("download directory must be an existing real directory") - } - root, err := os.OpenRoot(directory) + root, err := download.OpenRoot(directory) if err != nil { - return errors.New("cannot open download directory") + return err } defer root.Close() for _, document := range result.Value.GuideDocuments { @@ -57,31 +51,18 @@ func addTaskGuide(task *cobra.Command, client func() (*api.Client, error), outpu task.AddCommand(cmd) } -func downloadGuide(cmd *cobra.Command, client *api.Client, root *os.Root, taskID string, document api.TaskGuideDocument) error { +func downloadGuide(cmd *cobra.Command, client *api.Client, root *download.Root, taskID string, document api.TaskGuideDocument) error { name := document.Filename() - if _, err := root.Lstat(name); !os.IsNotExist(err) { - return errors.New("download target already exists or is inaccessible") - } - temporary := ".workstream-guide-" + rand.Text() - file, err := root.OpenFile(temporary, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600) + stage, err := root.Stage(name) if err != nil { - return errors.New("cannot create private guide download") - } - defer root.Remove(temporary) - defer file.Close() - if err := client.DownloadGuideDocument(cmd.Context(), taskID, document, file); err != nil { return err } - if err := file.Sync(); err != nil { - return errors.New("cannot persist guide download") - } - if err := file.Close(); err != nil { - return errors.New("cannot close guide download") + defer stage.Abort() + if err := client.DownloadGuideDocument(cmd.Context(), taskID, document, stage); err != nil { + return err } - // A hard link publishes atomically without replacing an existing file or - // following a symlink. Root confines both names even if the directory moves. - if err := root.Link(temporary, name); err != nil { - return errors.New("cannot publish guide download without overwrite") + if err := stage.Seal(); err != nil { + return err } - return nil + return stage.Publish() } diff --git a/cli/internal/download/download.go b/cli/internal/download/download.go new file mode 100644 index 000000000..753325af4 --- /dev/null +++ b/cli/internal/download/download.go @@ -0,0 +1,142 @@ +// Package download owns private staging and atomic no-overwrite publication. +package download + +import ( + "crypto/rand" + "errors" + "io" + "os" + "path/filepath" + "strings" + "sync" +) + +// Root is an opened, inode-bound destination directory. +type Root struct { + root *os.Root +} + +// OpenRoot opens an existing real directory and refuses a path swapped while +// it is being opened. +func OpenRoot(directory string) (*Root, error) { + if directory == "" { + return nil, errors.New("download directory must be an existing real directory") + } + directory = filepath.Clean(directory) + checked, err := os.Lstat(directory) + if err != nil || !checked.IsDir() || checked.Mode()&os.ModeSymlink != 0 { + return nil, errors.New("download directory must be an existing real directory") + } + root, err := os.OpenRoot(directory) + if err != nil { + return nil, errors.New("cannot open download directory") + } + opened, err := root.Stat(".") + if err != nil || !os.SameFile(checked, opened) { + _ = root.Close() + return nil, errors.New("download directory changed during open") + } + return &Root{root: root}, nil +} + +func (r *Root) Close() error { return r.root.Close() } + +// Stage creates a private temporary file for one final leaf name. Abort, +// Seal, and Publish are serialized so cancellation can safely race a writer. +type Stage struct { + mu sync.Mutex + root *Root + file *os.File + temporary string + final string + sealed bool + finished bool + published bool +} + +func (r *Root) Stage(name string) (*Stage, error) { + if name == "" || name == "." || filepath.Base(name) != name || strings.ContainsAny(name, "/\\\x00") { + return nil, errors.New("invalid download target name") + } + if _, err := r.root.Lstat(name); !os.IsNotExist(err) { + return nil, errors.New("download target already exists or is inaccessible") + } + temporary := ".workstream-download-" + rand.Text() + file, err := r.root.OpenFile(temporary, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600) + if err != nil { + return nil, errors.New("cannot create private download") + } + return &Stage{root: r, file: file, temporary: temporary, final: name}, nil +} + +func (s *Stage) Write(value []byte) (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.finished || s.sealed || s.file == nil { + return 0, errors.New("download stage is closed") + } + return s.file.Write(value) +} + +// Seal persists and closes verified staged bytes without publishing them. +func (s *Stage) Seal() error { + s.mu.Lock() + defer s.mu.Unlock() + if s.finished { + return errors.New("download stage was aborted") + } + if s.sealed { + return nil + } + if err := s.file.Sync(); err != nil { + return errors.New("cannot persist download") + } + if err := s.file.Close(); err != nil { + return errors.New("cannot close download") + } + s.file = nil + s.sealed = true + return nil +} + +// Publish exposes sealed bytes with a hard link, never replacing an existing +// file or following a target symlink. +func (s *Stage) Publish() error { + s.mu.Lock() + defer s.mu.Unlock() + if s.published { + return nil + } + if s.finished || !s.sealed { + return errors.New("download stage is not publishable") + } + if err := s.root.root.Link(s.temporary, s.final); err != nil { + _ = s.root.root.Remove(s.temporary) + s.finished = true + return errors.New("cannot publish download without overwrite") + } + s.finished = true + s.published = true + // Once linked, publication succeeded. A private sibling cleanup failure + // cannot be reported as a failed download because the final file exists. + _ = s.root.root.Remove(s.temporary) + return nil +} + +// Abort is idempotent and cleans an unpublished stage. Closing the handle +// first also permits removal on Windows while a cancelled writer unwinds. +func (s *Stage) Abort() { + s.mu.Lock() + defer s.mu.Unlock() + if s.finished { + return + } + s.finished = true + if s.file != nil { + _ = s.file.Close() + s.file = nil + } + _ = s.root.root.Remove(s.temporary) +} + +var _ io.Writer = (*Stage)(nil) diff --git a/cli/internal/download/download_test.go b/cli/internal/download/download_test.go new file mode 100644 index 000000000..71ff6d225 --- /dev/null +++ b/cli/internal/download/download_test.go @@ -0,0 +1,130 @@ +package download + +import ( + "os" + "path/filepath" + "runtime" + "sync" + "testing" +) + +func TestStagePublishesPrivateFileWithoutOverwrite(t *testing.T) { + directory := t.TempDir() + root, err := OpenRoot(directory) + if err != nil { + t.Fatal(err) + } + defer root.Close() + stage, err := root.Stage("guide.pdf") + if err != nil { + t.Fatal(err) + } + defer stage.Abort() + if _, err := stage.Write([]byte("verified bytes")); err != nil { + t.Fatal(err) + } + entries, err := os.ReadDir(directory) + if err != nil || len(entries) != 1 || entries[0].Name() == "guide.pdf" { + t.Fatal("bytes were not privately staged", entries, err) + } + info, err := entries[0].Info() + if err != nil || info.Mode().Perm() != 0600 { + t.Fatal("stage is not private", info, err) + } + if err := stage.Seal(); err != nil { + t.Fatal(err) + } + if err := stage.Publish(); err != nil { + t.Fatal(err) + } + if err := stage.Publish(); err != nil { + t.Fatal("successful publication is not idempotent", err) + } + body, err := os.ReadFile(filepath.Join(directory, "guide.pdf")) + if err != nil || string(body) != "verified bytes" { + t.Fatal(string(body), err) + } + entries, _ = os.ReadDir(directory) + if len(entries) != 1 || entries[0].Name() != "guide.pdf" { + t.Fatal("private stage retained", entries) + } + stage.Abort() +} + +func TestStageRefusesExistingFileAndSymlink(t *testing.T) { + directory := t.TempDir() + root, err := OpenRoot(directory) + if err != nil { + t.Fatal(err) + } + defer root.Close() + if err := os.WriteFile(filepath.Join(directory, "existing"), []byte("untouched"), 0600); err != nil { + t.Fatal(err) + } + if _, err := root.Stage("existing"); err == nil { + t.Fatal("existing file accepted") + } + if runtime.GOOS != "windows" { + if err := os.Symlink(filepath.Join(directory, "existing"), filepath.Join(directory, "link")); err != nil { + t.Fatal(err) + } + if _, err := root.Stage("link"); err == nil { + t.Fatal("existing symlink accepted") + } + } + if body, _ := os.ReadFile(filepath.Join(directory, "existing")); string(body) != "untouched" { + t.Fatal("existing target changed") + } +} + +func TestOpenRootRejectsDirectorySymlink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("ordinary test user cannot reliably create a directory symlink") + } + directory := t.TempDir() + link := filepath.Join(t.TempDir(), "destination") + if err := os.Symlink(directory, link); err != nil { + t.Fatal(err) + } + if _, err := OpenRoot(link); err == nil { + t.Fatal("directory symlink accepted") + } +} + +func TestAbortRacesWriterAndCleansWithoutResponseDelivery(t *testing.T) { + directory := t.TempDir() + root, err := OpenRoot(directory) + if err != nil { + t.Fatal(err) + } + defer root.Close() + stage, err := root.Stage("never-published") + if err != nil { + t.Fatal(err) + } + started := make(chan struct{}) + var once sync.Once + done := make(chan error, 1) + go func() { + for { + once.Do(func() { close(started) }) + if _, err := stage.Write(make([]byte, 32*1024)); err != nil { + done <- err + return + } + } + }() + <-started + stage.Abort() + if err := <-done; err == nil { + t.Fatal("aborted writer did not stop") + } + stage.Abort() + entries, err := os.ReadDir(directory) + if err != nil || len(entries) != 0 { + t.Fatal("aborted stage retained bytes", entries, err) + } + if err := stage.Seal(); err == nil { + t.Fatal("aborted stage sealed", err) + } +} diff --git a/cli/internal/tui/guide_findings_test.go b/cli/internal/tui/guide_findings_test.go new file mode 100644 index 000000000..c928bbd4f --- /dev/null +++ b/cli/internal/tui/guide_findings_test.go @@ -0,0 +1,198 @@ +package tui + +import ( + "context" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Flow-Research/workstream/cli/internal/api" +) + +func guideContext(documents ...api.TaskGuideDocument) api.WorkContext { + return api.WorkContext{ + Task: taskDetail(), Project: api.ContextProject{ID: projectID}, + Guide: api.ContextGuide{ID: actorID, ProjectID: projectID, Version: "locked-v1"}, + ContributionPolicyVersionID: policyID, + Lifecycle: api.ContextLifecycle{AssignedToCurrentActor: true}, + GuideDocuments: documents, + } +} + +func openMineGuide(t *testing.T, f *fakeClient) *Model { + t.Helper() + m := boot(t, f) + press(t, m, "enter") + press(t, m, "tab") + press(t, m, "enter") + press(t, m, "tab") + press(t, m, "tab") + press(t, m, "tab") + if m.section != 3 || m.context == nil { + t.Fatal("Guide did not load", f.calls) + } + return m +} + +func TestGuidePreservesOrderAndPublishesOnlyAfterConfirmation(t *testing.T) { + doc1 := api.TaskGuideDocument{DocumentID: actorID, Order: 2, Label: "First", MediaType: "text/markdown", ByteCount: 5, SHA256: "sha256:" + strings.Repeat("a", 64)} + doc2 := api.TaskGuideDocument{DocumentID: policyID, Order: 7, Label: "Second", MediaType: "application/pdf", ByteCount: 5, SHA256: "sha256:" + strings.Repeat("b", 64)} + f := &fakeClient{context: func(context.Context, string) (api.Result[api.WorkContext], error) { + return api.Result[api.WorkContext]{Value: guideContext(doc1, doc2)}, nil + }} + m := openMineGuide(t, f) + text := m.detailText() + if strings.Index(text, "First") > strings.Index(text, "Second") || !strings.Contains(text, "bytes not downloaded") { + t.Fatal(text) + } + directory := filepath.Join(t.TempDir(), "q?gr") + if err := os.Mkdir(directory, 0700); err != nil { + t.Fatal(err) + } + press(t, m, "down") + press(t, m, "d") + for _, character := range directory { + press(t, m, string(character)) + } + if m.closed { + t.Fatal("destination text triggered a global key") + } + if !strings.Contains(m.detailText(), "Download directory: "+directory) { + t.Fatal(m.detailText()) + } + before := len(f.calls) + press(t, m, "enter") + if !strings.Contains(m.detailText(), "Confirm download") || len(f.calls) != before || m.loading { + t.Fatal(m.detailText()) + } + if entries, _ := os.ReadDir(directory); len(entries) != 0 { + t.Fatal("confirmation performed I/O", entries) + } + press(t, m, "y") + target := filepath.Join(directory, doc2.Filename()) + if body, err := os.ReadFile(target); err != nil || string(body) != "guide" || !strings.Contains(m.detailText(), "Downloaded: "+doc2.Filename()) { + t.Fatal(string(body), err, m.detailText()) + } +} + +func TestGuideNavigationAbortsStageAndLateResponseCannotPublish(t *testing.T) { + document := api.TaskGuideDocument{DocumentID: actorID, Label: "Guide", MediaType: "text/markdown", ByteCount: 5, SHA256: "sha256:" + strings.Repeat("a", 64)} + f := &fakeClient{context: func(context.Context, string) (api.Result[api.WorkContext], error) { + return api.Result[api.WorkContext]{Value: guideContext(document)}, nil + }} + m := openMineGuide(t, f) + directory := t.TempDir() + press(t, m, "d") + press(t, m, directory) + press(t, m, "enter") + _, cmd := m.Update(keyMessage("y")) + if cmd == nil || m.download == nil { + t.Fatal("download was not staged") + } + press(t, m, "esc") + late := cmd() + m.Update(late) + if _, err := os.Stat(filepath.Join(directory, document.Filename())); !os.IsNotExist(err) { + t.Fatal("cancelled response published", err) + } + if entries, _ := os.ReadDir(directory); len(entries) != 0 { + t.Fatal("cancelled stage retained", entries) + } +} + +func TestGuideRefreshAbortsDownloadAndReloadsFreshContext(t *testing.T) { + document := api.TaskGuideDocument{DocumentID: actorID, Label: "Guide", MediaType: "text/markdown", ByteCount: 5, SHA256: "sha256:" + strings.Repeat("a", 64)} + contextReads := 0 + f := &fakeClient{context: func(context.Context, string) (api.Result[api.WorkContext], error) { + contextReads++ + return api.Result[api.WorkContext]{Value: guideContext(document)}, nil + }} + m := openMineGuide(t, f) + directory := t.TempDir() + press(t, m, "d") + press(t, m, directory) + press(t, m, "enter") + _, downloadCmd := m.Update(keyMessage("y")) + _, refreshCmd := m.Update(keyMessage("r")) + if refreshCmd == nil || m.request.kind != contextRead { + t.Fatal("refresh did not replace pending download with Guide context read") + } + apply(t, m, refreshCmd) + late := downloadCmd() + m.Update(late) + if contextReads < 3 || m.context == nil || m.section != 3 { + t.Fatal("Guide context was not freshly restored", contextReads, m.section) + } + if entries, _ := os.ReadDir(directory); len(entries) != 0 { + t.Fatal("refreshed download published or retained bytes", entries) + } +} + +func TestFindingsNavigationPaginationAndMisbinding(t *testing.T) { + cursor := "next-page" + requests := 0 + f := &fakeClient{findings: func(_ context.Context, submission, run string, limit int, next *string) (api.Result[api.SubmissionFindingsResult], error) { + requests++ + if submission != submissionID || run != "" || limit != 50 { + t.Fatal(submission, run, limit) + } + path, line, check, message, fix := "answer.md", int64(8), "lint.safe", "hostile\x1b]52;c;token\a", "repair" + task := taskID + if requests == 3 { + task = actorID + } + page := api.SubmissionCheckerPage{Items: []api.SubmissionCheckerRun{{ + ID: runID, Task: task, Submission: submissionID, Version: nil, Status: "completed", Attempt: nil, + Current: requests != 2, Results: []api.ContributorFinding{{ID: findingID, Checker: "lint", CheckID: &check, Path: &path, Line: &line, Status: "coverage_incomplete", Severity: "medium", Message: &message, Fix: &fix}}, + }}} + if next == nil { + page.Cursor = &cursor + } else if *next != cursor { + t.Fatal(*next) + } + return api.Result[api.SubmissionFindingsResult]{Value: api.SubmissionFindingsResult{Page: &page}}, nil + }} + m := boot(t, f) + press(t, m, "enter") + press(t, m, "tab") + press(t, m, "tab") + press(t, m, "enter") + press(t, m, "tab") + text := m.detailText() + for _, want := range []string{"Run: " + runID, "Current: true", "Checker: lint", "Check ID: lint.safe", "Finding status: coverage_incomplete", "Severity: medium", "Path: answer.md", "Line: 8", "Suggested fix: repair", "not acceptance"} { + if !strings.Contains(text, want) { + t.Fatal(want, text) + } + } + if strings.Contains(m.View().Content, "\x1b]52") { + t.Fatal("terminal control reached view", m.View().Content) + } + press(t, m, "n") + if requests != 2 || m.findingsPage != 2 || !strings.Contains(m.detailText(), "Current: false") { + t.Fatal(requests, m.findingsPage, m.detailText()) + } + press(t, m, "r") + if requests != 3 || m.failure == "" || m.findings != nil || m.submission != nil { + t.Fatal("foreign task finding retained", requests, m.failure) + } +} + +func TestDownloadPanicAbortsPrivateStage(t *testing.T) { + document := api.TaskGuideDocument{DocumentID: actorID, Label: "Guide", MediaType: "text/markdown", ByteCount: 5, SHA256: "sha256:" + strings.Repeat("a", 64)} + f := &fakeClient{context: func(context.Context, string) (api.Result[api.WorkContext], error) { + return api.Result[api.WorkContext]{Value: guideContext(document)}, nil + }, download: func(context.Context, string, api.TaskGuideDocument, io.Writer) error { + panic("secret-token download panic") + }} + m := openMineGuide(t, f) + directory := t.TempDir() + press(t, m, "d") + press(t, m, directory) + press(t, m, "enter") + press(t, m, "y") + if entries, _ := os.ReadDir(directory); len(entries) != 0 || strings.Contains(m.View().Content, "secret-token") { + t.Fatal(entries, m.View().Content) + } +} diff --git a/cli/internal/tui/handoff.go b/cli/internal/tui/handoff.go index 53851feba..bd8a7cf79 100644 --- a/cli/internal/tui/handoff.go +++ b/cli/internal/tui/handoff.go @@ -18,6 +18,9 @@ func (m *Model) handoffText() string { if m.section == 2 { verb = "requirements" } + if m.section == 3 { + verb = "guide" + } args = append(args, "task", verb, selected.task) } } @@ -54,7 +57,7 @@ func (m *Model) handoffText() string { out.WriteString(" argument: " + m.clean(arg) + "\n") } } - if m.detail && m.tab == submissions { + if m.detail && m.tab == submissions && m.section == 0 { if selected, ok := m.selectedRow(); ok { out.WriteString("\nDetailed findings: external manual read only.\n") for _, arg := range []string{"workstream", "submission", "findings", selected.id} { diff --git a/cli/internal/tui/model.go b/cli/internal/tui/model.go index a80434b14..9309524ac 100644 --- a/cli/internal/tui/model.go +++ b/cli/internal/tui/model.go @@ -26,6 +26,14 @@ type row struct { id, task, project, title, status string } +type guideMode int + +const ( + guideBrowse guideMode = iota + guideDestination + guideConfirm +) + // Model owns one fetched page and one exact detail, never an authority cache. type Model struct { client ReadClient @@ -53,6 +61,13 @@ type Model struct { context *api.WorkContext intake *api.SubmissionRequirements submission *api.SubmissionHistory + findings *api.SubmissionCheckerPage + findingsCursor, findingsPageCursor string + findingsPage int + guideSelected int + guideMode guideMode + guideDirectory, guideNotice string + download *guideDownload viewport viewport.Model } @@ -95,13 +110,19 @@ func (m *Model) invalidate() { m.cancel() m.cancel = nil } + if m.download != nil { + m.download.abort() + m.download = nil + } m.generation++ m.loading = false } func (m *Model) clearDetail() { m.detail, m.section = false, 0 - m.task, m.context, m.intake, m.submission = nil, nil, nil, nil + m.task, m.context, m.intake, m.submission, m.findings = nil, nil, nil, nil, nil + m.findingsCursor, m.findingsPageCursor, m.findingsPage = "", "", 1 + m.guideSelected, m.guideMode, m.guideDirectory, m.guideNotice = 0, guideBrowse, "", "" m.handoff = false m.viewport.GotoTop() } @@ -139,7 +160,7 @@ func (m *Model) selectedRow() (row, bool) { func (m *Model) key(key tea.KeyPressMsg) tea.Cmd { k := key.String() - if k == "ctrl+c" || (!m.filtering && k == "q") { + if k == "ctrl+c" || (!m.filtering && m.guideMode != guideDestination && k == "q") { m.closed = true m.invalidate() m.stop() @@ -164,6 +185,36 @@ func (m *Model) key(key tea.KeyPressMsg) tea.Cmd { m.selected = 0 return nil } + if m.guideMode == guideDestination { + switch k { + case "enter": + if m.guideDirectory != "" { + m.guideMode = guideConfirm + } + case "esc": + m.guideMode, m.guideDirectory = guideBrowse, "" + case "backspace": + _, size := utf8.DecodeLastRuneInString(m.guideDirectory) + if size > 0 { + m.guideDirectory = m.guideDirectory[:len(m.guideDirectory)-size] + } + default: + if key.Text != "" && len(m.guideDirectory)+len(key.Text) <= 4096 { + m.guideDirectory += key.Text + } + } + return nil + } + if m.guideMode == guideConfirm { + if k == "esc" { + m.guideMode, m.guideDirectory = guideBrowse, "" + return nil + } + if k == "y" { + return m.beginGuideDownload() + } + return nil + } if k == "?" { m.help, m.handoff = !m.help, false m.viewport.GotoTop() @@ -195,6 +246,13 @@ func (m *Model) key(key tea.KeyPressMsg) tea.Cmd { return m.scroll(k) } if k == "esc" && m.detail { + if m.tab == submissions && m.section == 1 { + m.invalidate() + m.section, m.findings = 0, nil + m.findingsCursor, m.findingsPageCursor, m.findingsPage = "", "", 1 + m.viewport.GotoTop() + return nil + } m.invalidate() m.clearDetail() m.failure = "" @@ -208,11 +266,19 @@ func (m *Model) key(key tea.KeyPressMsg) tea.Cmd { } if k == "r" { if m.loading { + if m.detail && m.tab == mine && m.section == 3 && m.request.kind == guideDownloadRead { + m.context, m.guideMode, m.guideNotice = nil, guideBrowse, "" + return m.begin(contextRead) + } return nil } if m.detail { + m.guideNotice = "" + if m.tab == submissions && m.section == 1 { + m.findings, m.findingsCursor, m.findingsPageCursor, m.findingsPage = nil, "", "", 1 + return m.begin(findingsRead) + } m.context, m.intake, m.task, m.submission = nil, nil, nil, nil - m.section = 0 return m.begin(m.detailKind()) } m.clearPage() @@ -229,14 +295,51 @@ func (m *Model) key(key tea.KeyPressMsg) tea.Cmd { } if m.detail { if k == "tab" && m.tab == mine { - m.section = (m.section + 1) % 3 + m.section = (m.section + 1) % 4 m.viewport.GotoTop() switch m.section { case 1: return m.begin(contextRead) case 2: return m.begin(intakeRead) + case 3: + return m.begin(contextRead) + } + } + if k == "tab" && m.tab == submissions { + m.invalidate() + m.section = (m.section + 1) % 2 + m.viewport.GotoTop() + if m.section == 1 { + m.findings, m.findingsCursor, m.findingsPageCursor, m.findingsPage = nil, "", "", 1 + return m.begin(findingsRead) + } + m.findings = nil + return nil + } + if m.tab == mine && m.section == 3 && m.context != nil { + switch k { + case "up": + m.guideSelected = max(0, m.guideSelected-1) + case "down": + m.guideSelected = min(max(0, len(m.context.GuideDocuments)-1), m.guideSelected+1) + case "d": + if len(m.context.GuideDocuments) > 0 { + m.guideMode, m.guideDirectory, m.guideNotice = guideDestination, "", "" + } + default: + return m.scroll(k) } + return nil + } + if m.tab == submissions && m.section == 1 { + if k == "n" && m.findingsCursor != "" { + cursor := m.findingsCursor + m.findings, m.findingsCursor = nil, "" + m.findingsPage++ + return m.beginCursor(findingsRead, cursor) + } + return m.scroll(k) } return m.scroll(k) } @@ -277,8 +380,17 @@ func (m *Model) key(key tea.KeyPressMsg) tea.Cmd { func (m *Model) detailKind() readKind { if m.tab == submissions { + if m.section == 1 { + return findingsRead + } return submissionRead } + switch m.section { + case 1, 3: + return contextRead + case 2: + return intakeRead + } return taskRead } diff --git a/cli/internal/tui/model_test.go b/cli/internal/tui/model_test.go index d5e039270..3eb75787c 100644 --- a/cli/internal/tui/model_test.go +++ b/cli/internal/tui/model_test.go @@ -3,6 +3,7 @@ package tui import ( "context" "errors" + "io" "strings" "testing" @@ -16,6 +17,8 @@ const ( taskID = "30000000-0000-7000-8000-000000000001" submissionID = "40000000-0000-7000-8000-000000000001" policyID = "50000000-0000-7000-8000-000000000001" + runID = "60000000-0000-7000-8000-000000000001" + findingID = "70000000-0000-7000-8000-000000000001" ) type fakeClient struct { @@ -26,6 +29,8 @@ type fakeClient struct { task func(context.Context, string) (api.Result[api.ContributorTaskDetail], error) context func(context.Context, string) (api.Result[api.WorkContext], error) intake func(context.Context, string) (api.Result[api.SubmissionRequirements], error) + findings func(context.Context, string, string, int, *string) (api.Result[api.SubmissionFindingsResult], error) + download func(context.Context, string, api.TaskGuideDocument, io.Writer) error assignmentQuery, submissionQuery api.PageQuery } @@ -88,6 +93,23 @@ func (f *fakeClient) Submission(context.Context, string) (api.Result[api.Submiss f.calls = append(f.calls, "submission") return api.Result[api.SubmissionHistory]{Value: api.SubmissionHistory{ID: submissionID, Task: taskID, Status: "review_pending", Summary: "Stored summary"}}, nil } +func (f *fakeClient) SubmissionFindings(ctx context.Context, submission, run string, limit int, cursor *string) (api.Result[api.SubmissionFindingsResult], error) { + f.calls = append(f.calls, "findings") + if f.findings != nil { + return f.findings(ctx, submission, run, limit, cursor) + } + page := api.SubmissionCheckerPage{Items: []api.SubmissionCheckerRun{{ID: runID, Task: taskID, Submission: submission, Status: "completed", Current: true, + Results: []api.ContributorFinding{{ID: findingID, Checker: "lint", Status: "passed", Severity: "info"}}}}} + return api.Result[api.SubmissionFindingsResult]{Value: api.SubmissionFindingsResult{Page: &page}}, nil +} +func (f *fakeClient) DownloadGuideDocument(ctx context.Context, task string, document api.TaskGuideDocument, target io.Writer) error { + f.calls = append(f.calls, "download") + if f.download != nil { + return f.download(ctx, task, document, target) + } + _, err := target.Write([]byte("guide")) + return err +} func apply(t *testing.T, m *Model, cmd tea.Cmd) { t.Helper() diff --git a/cli/internal/tui/requests.go b/cli/internal/tui/requests.go index 38cb1c368..0af460616 100644 --- a/cli/internal/tui/requests.go +++ b/cli/internal/tui/requests.go @@ -4,11 +4,14 @@ import ( "context" "errors" "fmt" + "io" "strings" + "sync" "time" tea "charm.land/bubbletea/v2" "github.com/Flow-Research/workstream/cli/internal/api" + "github.com/Flow-Research/workstream/cli/internal/download" ) // ReadClient is the bounded public read surface, satisfied directly by api.Client. @@ -22,6 +25,8 @@ type ReadClient interface { WorkContext(context.Context, string) (api.Result[api.WorkContext], error) SubmissionRequirements(context.Context, string) (api.Result[api.SubmissionRequirements], error) Submission(context.Context, string) (api.Result[api.SubmissionHistory], error) + SubmissionFindings(context.Context, string, string, int, *string) (api.Result[api.SubmissionFindingsResult], error) + DownloadGuideDocument(context.Context, string, api.TaskGuideDocument, io.Writer) error } var _ ReadClient = (*api.Client)(nil) @@ -36,21 +41,48 @@ const ( submissionRead contextRead intakeRead + findingsRead + guideDownloadRead ) // request is an immutable identity carried through a non-cooperative response. type request struct { - generation uint64 - actor, project, resource, task, cursor string - tab tab - kind readKind - page, section int + generation uint64 + actor, project, resource, task, cursor, document, destination string + tab tab + kind readKind + page, section int } type response struct { - request request - value any - err error + request request + value any + err error + download *guideDownload +} + +type guideDownload struct { + stage *download.Stage + root *download.Root + once sync.Once + name string +} + +func (d *guideDownload) abort() { + if d == nil { + return + } + d.stage.Abort() + d.once.Do(func() { _ = d.root.Close() }) +} + +func (d *guideDownload) publish() error { + if err := d.stage.Publish(); err != nil { + d.abort() + return err + } + d.once.Do(func() { _ = d.root.Close() }) + return nil } func (m *Model) begin(kind readKind) tea.Cmd { return m.beginCursor(kind, "") } @@ -60,6 +92,9 @@ func (m *Model) beginCursor(kind readKind, cursor string) tea.Cmd { if kind == projectsRead || kind == pageRead { m.pageCursor = cursor } + if kind == findingsRead { + m.findingsPageCursor = cursor + } m.failure, m.loading = "", true m.observed = time.Time{} r := request{generation: m.generation, actor: m.actor, project: m.project, tab: m.tab, @@ -76,7 +111,7 @@ func (m *Model) beginCursor(kind readKind, cursor string) tea.Cmd { // renderer's diagnostic logger or process-wide panic machinery. defer func() { if recover() != nil { - message = response{r, nil, &api.Failure{Code: "request_failed"}} + message = response{request: r, err: &api.Failure{Code: "request_failed"}} } }() var value any @@ -129,17 +164,80 @@ func (m *Model) beginCursor(kind readKind, cursor string) tea.Cmd { var result api.Result[api.SubmissionRequirements] result, err = client.SubmissionRequirements(ctx, r.task) value = result.Value + case findingsRead: + var result api.Result[api.SubmissionFindingsResult] + result, err = client.SubmissionFindings(ctx, r.resource, "", 50, continuation) + if result.Value.Page != nil { + value = *result.Value.Page + } + } + return response{request: r, value: value, err: err} + } +} + +func (m *Model) beginGuideDownload() tea.Cmd { + if m.context == nil || m.guideSelected < 0 || m.guideSelected >= len(m.context.GuideDocuments) { + return nil + } + document := m.context.GuideDocuments[m.guideSelected] + m.invalidate() + root, err := download.OpenRoot(m.guideDirectory) + if err != nil { + m.guideMode, m.guideNotice = guideBrowse, "Unavailable: "+m.clean(err.Error()) + return nil + } + stage, err := root.Stage(document.Filename()) + if err != nil { + _ = root.Close() + m.guideMode, m.guideNotice = guideBrowse, "Unavailable: "+m.clean(err.Error()) + return nil + } + work := &guideDownload{stage: stage, root: root, name: document.Filename()} + m.download = work + m.guideMode, m.guideNotice, m.failure, m.loading = guideBrowse, "", "", true + m.observed = time.Time{} + r := request{generation: m.generation, actor: m.actor, project: m.project, tab: m.tab, + kind: guideDownloadRead, resource: m.selectedResource(), task: m.context.Task.TaskID, + document: document.DocumentID, destination: m.guideDirectory, page: m.page, section: m.section} + m.request = r + ctx, cancel := context.WithCancel(m.ctx) + m.cancel = cancel + client := m.client + return func() (message tea.Msg) { + defer func() { + if recover() != nil { + work.abort() + message = response{request: r, err: &api.Failure{Code: "request_failed"}, download: work} + } + }() + err := client.DownloadGuideDocument(ctx, r.task, document, stage) + if err == nil { + err = stage.Seal() } - return response{r, value, err} + if err != nil || ctx.Err() != nil { + work.abort() + if err == nil { + err = ctx.Err() + } + } + return response{request: r, err: err, download: work} } } +func (m *Model) selectedResource() string { + selected, _ := m.selectedRow() + return selected.id +} + func sameID(a, b string) bool { return strings.EqualFold(strings.ReplaceAll(a, "-", ""), strings.ReplaceAll(b, "-", "")) } func (m *Model) receive(msg response) tea.Cmd { if msg.request != m.request || msg.request.generation != m.generation || !m.loading { + if msg.download != nil { + msg.download.abort() + } return nil } m.loading = false @@ -148,11 +246,37 @@ func (m *Model) receive(msg response) tea.Cmd { m.cancel = nil } if msg.err != nil { + if msg.download != nil { + msg.download.abort() + if m.download == msg.download { + m.download = nil + } + } m.failed(msg.err, msg.request.kind == profileRead) return nil } bad := func() tea.Cmd { m.failed(&api.Failure{Code: "invalid_api_response"}, false); return nil } r := msg.request + if r.kind == guideDownloadRead { + if msg.download == nil || m.download != msg.download || r.tab != mine || r.section != 3 || m.context == nil || + !sameID(r.resource, m.selectedResource()) || !sameID(r.task, m.context.Task.TaskID) || + m.guideSelected < 0 || m.guideSelected >= len(m.context.GuideDocuments) || + !sameID(r.document, m.context.GuideDocuments[m.guideSelected].DocumentID) { + if msg.download != nil { + msg.download.abort() + } + return bad() + } + if err := msg.download.publish(); err != nil { + m.download = nil + m.guideNotice = "Unavailable: " + m.clean(err.Error()) + return nil + } + m.download = nil + m.guideNotice = "Downloaded: " + m.clean(msg.download.name) + m.observed = time.Now() + return nil + } switch value := msg.value.(type) { case api.Profile: if r.kind != profileRead || value.ActorProfileID == "" { @@ -225,6 +349,20 @@ func (m *Model) receive(msg response) tea.Cmd { return bad() } m.intake = &value + case api.SubmissionCheckerPage: + if r.kind != findingsRead || r.tab != submissions || r.section != 1 || !sameID(r.resource, m.selectedResource()) { + return bad() + } + for _, run := range value.Items { + if !sameID(run.Submission, r.resource) || !sameID(run.Task, r.task) { + return bad() + } + } + m.findings = &value + m.findingsCursor = "" + if value.Cursor != nil { + m.findingsCursor = *value.Cursor + } default: return bad() } diff --git a/cli/internal/tui/view.go b/cli/internal/tui/view.go index 917f760b3..724e253b9 100644 --- a/cli/internal/tui/view.go +++ b/cli/internal/tui/view.go @@ -18,9 +18,12 @@ func (m *Model) heading() string { return m.tab.String() } if m.tab == submissions { + if m.section == 1 { + return "FINDINGS" + } return "SUBMISSION" } - return [...]string{"TASK", "CONTEXT", "INTAKE"}[m.section] + return [...]string{"TASK", "CONTEXT", "INTAKE", "GUIDE"}[m.section] } // fitText bounds a display label, never an exact identity or request selector. @@ -42,13 +45,39 @@ func (m *Model) headerText() string { } func (m *Model) footerText() string { + if m.guideMode == guideDestination { + return "enter confirm | esc cancel | ctrl+c quit" + } + if m.guideMode == guideConfirm { + return "y download | esc cancel | q quit" + } if m.height < 16 { return "? keys | q quit" } + if m.detail && m.tab == mine && m.section == 3 { + return "arrows select | d download | tab section | esc back\nr refresh | c CLI | ? help | q quit" + } + if m.detail && m.tab == submissions && m.section == 1 { + return "arrows scroll | n next | tab overview | esc back\nr refresh | c CLI | ? help | q quit" + } return "arrows select/scroll | enter inspect | tab section | esc back\ng project | n next | r refresh | / local filter | c CLI | ? help | q quit" } func (m *Model) statusText() string { + if m.detail && m.tab == submissions && m.section == 1 { + count := 0 + if m.findings != nil { + count = len(m.findings.Items) + } + status := fmt.Sprintf("page %d | fetched %d runs", m.findingsPage, count) + if m.findingsCursor != "" { + status += " | more available" + } + if !m.observed.IsZero() { + status += " observed " + m.observed.Format("15:04:05") + } + return fitText(status, m.width) + } status := fmt.Sprintf("page %d | fetched %d", m.page, len(m.rows)) if m.width < 70 { status = fmt.Sprintf("page %d rows %d", m.page, len(m.rows)) @@ -107,7 +136,7 @@ func (m *Model) reflow() { func (m *Model) body() string { if m.help { - return "KEYBOARD HELP\nArrows select a row or scroll detail. Enter inspects one exact resource.\nTab: Ready/Mine/Submissions in lists; Overview/Context/Intake in active Mine detail.\nEsc returns without a write. g fetches a fresh project picker.\nn fetches exactly one next page. r resets the current read; no polling.\n/ filters the fetched page only; enter finishes, esc cancels.\nc shows inert CLI arguments. q or Ctrl+C quits.\nNo actions, downloads or exports run in this workbench.\nEvidence and detailed findings are manual CLI handoffs.\nA read is not a reservation or authority.\n? or Esc closes help." + return "KEYBOARD HELP\nArrows select a row/document or scroll detail. Enter inspects one exact resource.\nTab: Ready/Mine/Submissions in lists; Overview/Context/Intake/Guide in active Mine detail; Overview/Findings in Submission detail.\nEsc returns or cancels destination/confirmation without a download. g fetches a fresh project picker.\nn fetches exactly one next findings/list page. r resets the current read; no polling.\n/ filters the fetched list page only; enter finishes, esc cancels.\nd enters a destination for one selected guide original; y confirms.\nc shows inert CLI arguments. q or Ctrl+C quits.\nChecker evidence is not acceptance. A read is not a reservation or authority.\n? or Esc closes help." } if m.handoff { return m.handoffText() @@ -184,6 +213,9 @@ func (m *Model) detailText() string { line(label, *value) } } + if m.tab == submissions && m.section == 1 && m.findings != nil { + return m.findingsText() + } if m.submission != nil { v := m.submission line("Submission", v.ID) @@ -204,9 +236,12 @@ func (m *Model) detailText() string { line("Type", e.Type) line("Size", number(e.Size)) } - out.WriteString("Detailed findings: manual CLI handoff only (c).\nStored observations do not predict acceptance or fulfillment.") + out.WriteString("Tab opens contributor-visible checker findings.\nStored observations do not predict acceptance or fulfillment.") return out.String() } + if m.section == 3 && m.context != nil { + return m.guideText() + } if m.section == 1 && m.context != nil { v := m.context line("Task", v.Task.TaskID) @@ -317,3 +352,89 @@ func (m *Model) detailText() string { } return out.String() } + +func (m *Model) guideText() string { + var out strings.Builder + v := m.context + line := func(label, value string) { out.WriteString(label + ": " + m.clean(value) + "\n") } + line("Task", v.Task.TaskID) + line("Guide", v.Guide.ID) + line("Locked guide version", v.Guide.Version) + if len(v.GuideDocuments) == 0 { + out.WriteString("No assigned guide documents.\n") + } + for index, d := range v.GuideDocuments { + mark := " " + if index == m.guideSelected { + mark = "> " + } + out.WriteString(mark + "Document " + fmt.Sprint(index+1) + "\n") + line("Label", d.Label) + line("Document ID", d.DocumentID) + line("Order", fmt.Sprint(d.Order)) + line("Media type", d.MediaType) + line("Bytes (server metadata; bytes not downloaded)", fmt.Sprint(d.ByteCount)) + line("SHA-256 (server metadata; bytes not downloaded)", d.SHA256) + } + if m.guideMode == guideDestination { + line("Download directory", m.guideDirectory) + out.WriteString("Enter confirms this existing directory; no directory is created.\n") + } else if m.guideMode == guideConfirm && len(v.GuideDocuments) > 0 { + d := v.GuideDocuments[m.guideSelected] + out.WriteString("Confirm download\n") + line("Task", v.Task.TaskID) + line("Document", d.DocumentID) + line("Download directory", m.guideDirectory) + line("Filename", d.Filename()) + out.WriteString("y downloads verified bytes | esc cancels\n") + } else if m.guideNotice != "" { + out.WriteString(m.guideNotice + "\n") + } + out.WriteString("Guide metadata is not verified downloaded bytes.") + return out.String() +} + +func (m *Model) findingsText() string { + var out strings.Builder + line := func(label, value string) { out.WriteString(label + ": " + m.clean(value) + "\n") } + optional := func(label string, value *string) { + if value == nil { + line(label, "not supplied") + } else { + line(label, *value) + } + } + if len(m.findings.Items) == 0 { + out.WriteString("No checker runs on this fetched page.\n") + } + for _, run := range m.findings.Items { + line("Run", run.ID) + line("Submission", run.Submission) + line("Task", run.Task) + line("Submission version", number(run.Version)) + line("Run status", run.Status) + line("Attempt", number(run.Attempt)) + line("Current", fmt.Sprint(run.Current)) + optional("Predecessor run", run.Predecessor) + line("Created", run.CreatedAt) + optional("Completed", run.CompletedAt) + for _, finding := range run.Results { + line("Finding", finding.ID) + line("Checker", finding.Checker) + optional("Check ID", finding.CheckID) + line("Finding status", finding.Status) + line("Severity", finding.Severity) + optional("Path", finding.Path) + if finding.Line == nil { + line("Line", "not supplied") + } else { + line("Line", fmt.Sprint(*finding.Line)) + } + optional("Message", finding.Message) + optional("Suggested fix", finding.Fix) + } + out.WriteString("\n") + } + out.WriteString("Checker findings are evidence, not acceptance.") + return out.String() +} diff --git a/cli/tests/integration/test_tui_http.py b/cli/tests/integration/test_tui_http.py index b37cfba00..37e05bfc7 100644 --- a/cli/tests/integration/test_tui_http.py +++ b/cli/tests/integration/test_tui_http.py @@ -1,6 +1,7 @@ """Built TUI -> native PTY/ConPTY -> adversarial socket; no runtime substitute.""" from copy import deepcopy +import hashlib import os import re import subprocess @@ -12,7 +13,8 @@ from test_contributor_reads_http import ASSIGNMENT_ROW, PROJECT_ROW, SUBMISSION_ROW from test_contributor_task_http import CONTRIBUTOR, READY from test_http_boundary import ACTOR, PROFILE, PROJECT, TOKEN -from test_submission_http import HISTORY, SUBMISSION +from test_submission_http import CHECKER as SUBMISSION_CHECKER +from test_submission_http import HISTORY, RUN, SUBMISSION from test_task_context_http import CONTEXT, REQUIREMENTS from test_task_http_boundary import TASK from tui_terminal_support import Terminal, environment, executable, socket_peer @@ -20,6 +22,18 @@ OTHER_PROJECT = "018f0ebc-7966-7e8d-bc4d-1cae1e000099" OTHER_TASK = "018f0ebc-7966-7e8d-bc4d-1cae1e000098" CURSOR = "opaque+/=&?#next" +CHECKER = deepcopy(SUBMISSION_CHECKER) +CHECKER["task_id"] = TASK +GUIDE_BYTES = b"# Locked guide\nDeliver the requested evidence.\n" +GUIDE_DOCUMENT = { + "document_id": ACTOR, + "order": 0, + "label": "Locked instructions", + "media_type": "text/markdown", + "byte_count": len(GUIDE_BYTES), + "sha256": "sha256:" + hashlib.sha256(GUIDE_BYTES).hexdigest(), + "read_reference": f"/api/v1/tasks/{TASK}/guide/documents/{ACTOR}/content", +} READY_PATH = f"/api/v1/projects/{PROJECT}/tasks/ready" TASK_PATH = f"/api/v1/tasks/{TASK}" SECRET = "EXACT_PRIVATE_INSTRUCTIONS" @@ -117,6 +131,263 @@ def denial(status=404): } +def open_guide(terminal): + open_ready(terminal) + terminal.send("\t") + terminal.wait("Assigned terminal task") + terminal.send("\r") + terminal.wait(SECRET) + terminal.send("\t") + terminal.wait("CONTEXT") + terminal.send("\t") + terminal.wait("INTAKE") + terminal.send("\t") + terminal.wait("GUIDE") + terminal.wait("Locked instructions") + + +def guide_reads(): + reads = Reads() + reads.context["lifecycle"]["assigned_to_current_actor"] = True + reads.context["guide_documents"] = [deepcopy(GUIDE_DOCUMENT)] + reads.overrides[GUIDE_DOCUMENT["read_reference"]] = lambda _: ( + 200, + GUIDE_BYTES, + "text/markdown", + ) + return reads + + +def confirm_download(terminal, directory): + terminal.send("d") + terminal.wait("Download directory") + terminal.send(str(directory)) + terminal.send("\r") + terminal.wait("Confirm download") + terminal.send("y") + + +def open_findings(terminal): + open_ready(terminal) + terminal.send("\t") + terminal.wait("Assigned terminal task") + terminal.send("\t") + terminal.wait("Owned terminal attempt") + terminal.send("\r") + terminal.wait("Exact owned submission summary") + terminal.send("\t") + + +def test_guide_download_requires_confirmation_and_verifies_bytes(tmp_path): + reads = guide_reads() + with ( + socket_peer(reads) as (origin, requests), + Terminal(origin, TOKEN, "--plain") as terminal, + ): + open_guide(terminal) + assert not list(tmp_path.iterdir()) + assert not any( + target == GUIDE_DOCUMENT["read_reference"] for _, target, _ in requests + ) + terminal.send("d") + terminal.wait("Download directory") + terminal.send(str(tmp_path)) + terminal.send("\r") + terminal.wait("Confirm download") + assert not list(tmp_path.iterdir()) + assert not any( + target == GUIDE_DOCUMENT["read_reference"] for _, target, _ in requests + ) + terminal.send("y") + terminal.wait("Downloaded") + saved = tmp_path / f"{ACTOR}.md" + assert saved.read_bytes() == GUIDE_BYTES + assert ( + "sha256:" + hashlib.sha256(saved.read_bytes()).hexdigest() + == GUIDE_DOCUMENT["sha256"] + ) + assert list(tmp_path.iterdir()) == [saved] + terminal.quit() + + +@pytest.mark.parametrize("corrupt", [b"x" * len(GUIDE_BYTES), GUIDE_BYTES[:-1]]) +def test_guide_integrity_failure_never_publishes(tmp_path, corrupt): + reads = guide_reads() + reads.overrides[GUIDE_DOCUMENT["read_reference"]] = lambda _: ( + 200, + corrupt, + "text/markdown", + ) + with ( + socket_peer(reads) as (origin, _), + Terminal(origin, TOKEN, "--plain") as terminal, + ): + open_guide(terminal) + confirm_download(terminal, tmp_path) + terminal.wait("guide_document_integrity_mismatch") + assert not list(tmp_path.iterdir()) + assert "Downloaded" not in terminal.screen + terminal.quit() + + +def test_guide_download_never_overwrites_existing_file(tmp_path): + reads = guide_reads() + saved = tmp_path / f"{ACTOR}.md" + saved.write_bytes(b"existing private content") + with ( + socket_peer(reads) as (origin, requests), + Terminal(origin, TOKEN, "--plain") as terminal, + ): + open_guide(terminal) + confirm_download(terminal, tmp_path) + terminal.wait("Unavailable") + assert saved.read_bytes() == b"existing private content" + assert list(tmp_path.iterdir()) == [saved] + assert not any( + target == GUIDE_DOCUMENT["read_reference"] for _, target, _ in requests + ) + terminal.quit() + + +def test_guide_refusal_clears_locked_metadata_and_publishes_nothing(tmp_path): + reads = guide_reads() + reads.overrides[GUIDE_DOCUMENT["read_reference"]] = lambda _: denial() + with ( + socket_peer(reads) as (origin, _), + Terminal(origin, TOKEN, "--plain") as terminal, + ): + open_guide(terminal) + confirm_download(terminal, tmp_path) + terminal.wait("Denied") + assert "Locked instructions" not in terminal.screen + assert not list(tmp_path.iterdir()) + assert TOKEN.encode() not in terminal.raw + terminal.quit() + + +@pytest.mark.parametrize("cancel", ["\x1b", "q"]) +def test_cancel_delayed_guide_never_publishes(tmp_path, cancel): + reads = guide_reads() + started, release, returned = Event(), Event(), Event() + + def delayed(_): + started.set() + assert release.wait(10), "test failed to release delayed download" + returned.set() + return 200, GUIDE_BYTES, "text/markdown" + + reads.overrides[GUIDE_DOCUMENT["read_reference"]] = delayed + with ( + socket_peer(reads) as (origin, _), + Terminal(origin, TOKEN, "--plain") as terminal, + ): + try: + open_guide(terminal) + confirm_download(terminal, tmp_path) + assert started.wait(5) + terminal.send(cancel) + if cancel == "q": + terminal.wait_exit() + else: + terminal.pump(0.3) + release.set() + assert returned.wait(5) + terminal.pump(0.3) + assert not list(tmp_path.iterdir()) + if cancel != "q": + assert "Downloaded" not in terminal.screen + terminal.quit() + finally: + release.set() + + +def test_findings_explicit_pagination_preserves_run_identity(): + reads = Reads() + calls = [] + second = deepcopy(CHECKER) + second["id"] = OTHER_TASK + second["is_current_for_submission"] = False + second["results"][0]["status"] = "coverage_incomplete" + second["results"][0]["worker_message"] = "Coverage gap on historical run" + + def findings(query): + calls.append(query) + if "cursor" not in query: + assert query == {"limit": ["50"]} + return 200, page(CHECKER, CURSOR) + assert query == {"limit": ["50"], "cursor": [CURSOR]} + return 200, page(second) + + reads.overrides[f"/api/v1/submissions/{SUBMISSION}/checker-runs"] = findings + with ( + socket_peer(reads) as (origin, _), + Terminal(origin, TOKEN, "--plain", rows=60) as terminal, + ): + open_findings(terminal) + terminal.wait(RUN) + terminal.wait("quality.output") + assert len(calls) == 1 + assert "src/out.txt" in terminal.screen + terminal.send("n") + terminal.wait("Coverage gap on historical run") + assert len(calls) == 2 + assert ( + OTHER_TASK in terminal.screen and "coverage_incomplete" in terminal.screen + ) + assert RUN not in terminal.screen + assert TOKEN.encode() not in terminal.raw + assert b"\x1b]52;" not in terminal.raw + terminal.quit() + + +@pytest.mark.parametrize("field", ["task_id", "submission_id"]) +def test_findings_refuse_wrong_task_or_submission(field): + reads = Reads() + run = deepcopy(CHECKER) + run[field] = OTHER_TASK + run["results"][0]["worker_message"] = "FOREIGN_FINDING_MUST_NOT_RENDER" + reads.overrides[f"/api/v1/submissions/{SUBMISSION}/checker-runs"] = lambda _: ( + 200, + page(run), + ) + with ( + socket_peer(reads) as (origin, _), + Terminal(origin, TOKEN, "--plain") as terminal, + ): + open_findings(terminal) + terminal.wait("invalid_api_response") + assert "FOREIGN_FINDING_MUST_NOT_RENDER" not in terminal.screen + assert "Exact owned submission summary" not in terminal.screen + terminal.quit() + + +@pytest.mark.parametrize( + "status", ["passed", "warning", "failed", "coverage_incomplete"] +) +def test_findings_preserve_status_and_sanitize_details(status): + reads = Reads() + run = deepcopy(CHECKER) + run["results"][0].update(status=status, worker_message="Finding " + HOSTILE) + reads.overrides[f"/api/v1/submissions/{SUBMISSION}/checker-runs"] = lambda _: ( + 200, + page(run), + ) + with ( + socket_peer(reads) as (origin, _), + Terminal(origin, TOKEN, "--plain", rows=60) as terminal, + ): + open_findings(terminal) + terminal.wait("Finding status: " + status) + assert "Severity: error" in terminal.screen + assert "Check ID: quality.output" in terminal.screen + assert "Path: src/out.txt" in terminal.screen + assert "Line: 4" in terminal.screen + assert TOKEN.encode() not in terminal.raw + assert b"\x1b]52;" not in terminal.raw and b"\x1b]8;" not in terminal.raw + assert "\u202e".encode() not in terminal.raw + terminal.quit() + + def test_keyboard_journey_uses_only_nine_fresh_public_reads(): reads = Reads() with ( diff --git a/cli/tests/integration/tui_terminal_support.py b/cli/tests/integration/tui_terminal_support.py index 153a8aa64..dbe2cd6b8 100644 --- a/cli/tests/integration/tui_terminal_support.py +++ b/cli/tests/integration/tui_terminal_support.py @@ -836,13 +836,19 @@ def do_GET(self): # noqa: N802 - HTTP handler interface request = (self.command, self.path, self.headers.get("Authorization")) requests.append(request) try: - status, body = respond(*request) + result = respond(*request) + status, body = result[:2] + media_type = result[2] if len(result) == 3 else "application/json" except Exception as error: # fixture errors must reach the test thread failures.append(error) - status, body = 500, {"unexpected": True} - encoded = json.dumps(body, ensure_ascii=False).encode() + status, body, media_type = 500, {"unexpected": True}, "application/json" + encoded = ( + body + if isinstance(body, bytes) + else json.dumps(body, ensure_ascii=False).encode() + ) self.send_response(status) - self.send_header("Content-Type", "application/json") + self.send_header("Content-Type", media_type) self.send_header("Content-Length", str(len(encoded))) self.end_headers() try: diff --git a/docs/design/cli-terminal-experience.md b/docs/design/cli-terminal-experience.md index 570f56115..c5434c92f 100644 --- a/docs/design/cli-terminal-experience.md +++ b/docs/design/cli-terminal-experience.md @@ -1,6 +1,7 @@ # Workstream terminal experience — implemented first slice and proposed extensions -Status: WS-CLI-001-48 implements the bounded read-only contributor slice below; +Status: WS-CLI-001-48 implements the bounded contributor read slice; +WS-CLI-001-49 extends it with findings and verified guide downloads below; the broader workspace and mutation experience remains proposed. This is not a new product/API contract. The opt-in `workstream tui` shares the existing Go binary and public client. Explicit commands in [the CLI reference](../../cli/README.md) @@ -383,7 +384,7 @@ Tea 2.0.10, Bubbles 2.2.1 and Lip Gloss 2.0.6, with resolved modules in the CLI lockfiles. Native terminal verification remains required on all four supported targets; accessibility certification is not implied. -### Implemented first slice +### Original first slice (WS-CLI-001-48) The opt-in **read-only contributor workbench** in the existing Go binary provides: launch/terminal guard → actor binding → `me projects` page → selected project's @@ -433,11 +434,36 @@ Proof must include a built TUI/public-API journey and unchanged explicit CLI text/JSON/non-TTY behavior; render snapshots alone are not runtime proof. This proposal is not authorization to begin that next change automatically. +### Guide and findings extension (WS-CLI-001-49) + +[The bounded extension](../../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md) +adds Guide after Overview/Context/Intake in Mine detail. Opening Guide fetches +fresh assignment-sensitive WorkContext. Arrows select a document in server +order; `d` enters an existing destination directory, Enter shows exact target +confirmation, and `y` starts the verified download. Metadata is not a verified +byte claim. Private staged bytes become a final UUID-named file only after +verification and receipt by the still-current screen; abort/quit cleanup does +not rely on a late response being rendered. Existing files are never replaced. +No automatic viewer, markdown execution or task examples are added. + +Submission detail Tab opens Findings using the existing validated checker-run +read. One page is loaded; `n` requests its explicit continuation and `r` resets +the findings read. Every run retains its identity/current flag and every finding +its status, severity, check ID, path/line, message and suggested fix when supplied. +Successful checks are not acceptance. Contribution Evidence remains a CLI +handoff. There are still no product writes, polling or operation journals. + +The shared API now returns the already-validated findings page/exact-run shape +alongside Raw; existing CLI output keeps Raw unchanged. The shared download +owner serves both explicit commands and TUI, preserving directory binding, +private staging, verified bytes and atomic no-overwrite publication. + ### Dependency map to the end-to-end experience | Boundary | Dependency and next usable result | |---|---| | Contributor read-only workbench | Nine existing typed reads and merged backend guards → project/Ready/active Mine/Submissions browse/inspect with explicit pagination; detailed findings and Evidence are manual CLI handoffs; no API-client edit or catalogue dependency | +| Guide/findings extension | Existing WorkContext and verified document reader plus typed existing checker-run result → ordered guide selection, confirmed download and findings pagination; no backend changes or product writes | | Later in-TUI contribution Evidence | Narrow shared typed single-page contribution port and preserved complete-export regressions → explicit per-page evidence browsing; not a first-slice prerequisite | | Claim/start | Read workbench + reviewed per-operation private receipt and exact-key recovery → separate claim and start confirmations | | Prepare/submit | Claim/start + ZIP source integrity + distinct preparation/admission replay receipts → separate intake and immutable Submission, then stored evaluation/outcome/contribution evidence | diff --git a/docs/roadmap_status.md b/docs/roadmap_status.md index c43265073..2e409529b 100644 --- a/docs/roadmap_status.md +++ b/docs/roadmap_status.md @@ -46,11 +46,14 @@ atomic retry evidence. The CLI consumes all nine existing routes through hosted S3 operation remains a separate proof boundary. The [CLI distribution path](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-44.md) prepares native four-platform GitHub Releases proof and a protected manual -publisher. No approved release tag or public binary has been published yet. +publisher. Publication and public-installation verification are tracked in +[#562](https://github.com/Flow-Research/workstream/issues/562); a tag or passing +build alone is not public distribution. The opt-in [contributor terminal workbench](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-48.md) adds local read-only project/task/Submission navigation through existing typed -public reads. Detailed findings and contribution evidence remain explicit CLI -handoffs; TUI mutations and management are separate work. Neither this interface +public reads. WS-CLI-001-49 adds paged detailed findings and explicit verified +locked-guide downloads. Contribution evidence remains an explicit CLI +handoff; TUI product mutations and management are separate work. Neither this interface nor release automation establishes hosted operation or a published binary. ## Product Goal @@ -439,8 +442,10 @@ projects, Ready tasks, active own assignments and own Submissions, with fresh task/attempt details and explicit Mine context/intake reads. It uses the existing API client, one page per deliberate continuation and no local authority rules. Denial removes affected content; late responses cannot replace the current -view. Detailed findings/Evidence are inert CLI handoffs, not automatic export. -All writes, downloads, polling, management forms and operation-specific recovery +view. WS-CLI-001-49 adds Guide selection and confirmed single-document downloads +through shared private staging and no-overwrite publication, plus exact-Submission +paged Findings. Evidence remains an inert CLI handoff, not automatic export. +Product writes, polling, management forms and operation-specific recovery remain outside this bounded workbench. Explicit text/JSON commands stay usable without a terminal. The [Operator checker CLI](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-29.md) From 314218422fe9899dfef2ebe7fd66e43477134667 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 26 Oct 2026 12:07:24 +0100 Subject: [PATCH 4/9] docs(plan): use canonical intent heading --- .commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md index 3dd3f451a..253a5ce28 100644 --- a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md @@ -4,7 +4,7 @@ - Durable disposition: `Planned` - Intended merge outcome: Replace guide/findings handoffs with bounded interactive reads and explicit verified guide downloads. -## Intent and current behavior +## Intent Issue #688 tracks the remaining terminal experience. Main's read-only workspace opens exact task/context/intake and Submission details but sends users back to From 74931344ad0e24571de220af6905e26dab93445c Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 26 Oct 2026 12:08:48 +0100 Subject: [PATCH 5/9] docs(plan): align evidence and bounded-change sections --- .../initiatives/WS-CLI-001/WS-CLI-001-49.md | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md index 253a5ce28..537176bb2 100644 --- a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md @@ -12,7 +12,7 @@ explicit commands for detailed findings and guide originals. The public APIs and their validation already exist. This slice removes those handoffs without adding product mutations or changing backend authority. -## Design and scope +## Bounded change Extend existing TUI navigation, request generations, cancellation and safe text rendering. WorkContext supplies assignment-sensitive ordered guide metadata; @@ -47,7 +47,7 @@ creation, overwrite, or download-all implicit chains. Existing explicit commands remain available. A separate raw-HTTP TUI client or duplicate download helper was rejected because it would fork validation and custody rules. -## Risk and review +## Risk and review routing L1: untrusted terminal data and local filesystem publication. Plan review before implementation; focused security, QA and reuse/architecture review of affected @@ -55,7 +55,7 @@ owners; documentation review for capability claims. Human merge remains required No new authority decision is needed; if a backing contract is missing, report the gap rather than invent it. -## Acceptance and proof +## Acceptance criteria - Assigned task guide screen preserves exact server ordering, identities and hashes; no documents for an unassigned task or task-example disclosure. @@ -78,6 +78,23 @@ the gap rather than invent it. Confirm destination/confirmation usability, exact locked identity visibility, no-overwrite and integrity failure behavior, and honest checker outcome labels. +## Evidence + +Plan review required explicit staged publication, typed findings with preserved +Raw output, and reachable keyboard transitions; the concrete contract below +incorporates those corrections. Runtime proof is the focused Go owner tests and +built-process native terminal tests, plus existing explicit guide/source and +submission regressions. Hosted native and public-API checks remain required +before merge readiness; current results belong in the PR trust bundle. + +This cohesive slice exceeds the preferred 500-line L1 guideline: new staging +and cancellation tests, native interaction regressions, and removal of both +existing publication implementations account for much of the change. Splitting +the helper from its consumers would leave duplicated custody rules; the two +read surfaces share the bounded navigation extension. No backend, CI, dependency +or policy boundary changes are included. Focused reviewers inspect filesystem +security, state/navigation correctness and owner reuse separately. + ## Concrete state and owner contract Mine detail keeps Overview, Context and Intake in their existing order, then From 337ba793f51a5328ba81ec90b220f71e9a2a07d9 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 26 Oct 2026 12:35:57 +0100 Subject: [PATCH 6/9] [WS-CLI-001-49] Reconcile review findings and native private staging --- .../initiatives/WS-CLI-001/OVERVIEW.md | 7 +- .../initiatives/WS-CLI-001/WS-CLI-001-49.md | 20 +++- cli/README.md | 2 +- cli/go.mod | 2 +- cli/internal/download/download.go | 2 +- cli/internal/download/download_test.go | 21 ++-- cli/internal/download/private_other.go | 9 ++ cli/internal/download/private_other_test.go | 16 ++++ cli/internal/download/private_windows.go | 95 +++++++++++++++++++ cli/internal/download/private_windows_test.go | 43 +++++++++ cli/internal/tui/guide_findings_test.go | 65 +++++++++++-- cli/internal/tui/handoff.go | 17 ++-- cli/internal/tui/handoff_test.go | 20 +++- cli/internal/tui/model.go | 7 -- cli/internal/tui/requests_test.go | 53 +++++++++++ cli/tests/integration/test_tui_http.py | 83 +++++++++++++++- docs/design/cli-terminal-experience.md | 2 +- docs/roadmap_status.md | 14 ++- 18 files changed, 422 insertions(+), 56 deletions(-) create mode 100644 cli/internal/download/private_other.go create mode 100644 cli/internal/download/private_other_test.go create mode 100644 cli/internal/download/private_windows.go create mode 100644 cli/internal/download/private_windows_test.go diff --git a/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md b/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md index c599ef76c..4f86d7114 100644 --- a/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md +++ b/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md @@ -75,7 +75,7 @@ and separate guide-bound review/revision policy configuration. The commands the existing public writes with caller-owned replay keys and policy `If-Match`. The backend owns manager authorization for new writes, policy semantics, replay and activation; editing/configuring a guide does not approve or activate it. Other public API -client gaps and the broader TUI beyond -48 remain separate work. Binary distribution's +client gaps and the broader TUI beyond -49 remain separate work. Binary distribution's workflow is prepared by -44; a protected release run and public-asset audit remain operational work. @@ -326,13 +326,14 @@ that human's credential, not a registered agent identity. Make every operation complete without a terminal: stable JSON output, safe errors, exit status, and no required prompts. Add optional terminal assistance only where it improves an actual workflow. The opt-in `workstream tui` browses -the bounded contributor reads in -48 using the same client operations and never +the bounded contributor reads in -48/-49 using the same client operations, including +confirmed verified guide downloads and paged checker findings, and never becomes a second state machine. Further screens require stable public contracts and their own proof. Prefer purposeful commands over generating a flag mirror of every OpenAPI route. Go is the chosen implementation language. Cobra owns command parsing/help and -completion. The bounded -48 workbench uses Bubble Tea, Bubbles and Lip Gloss; +completion. The bounded -48/-49 workbench uses Bubble Tea, Bubbles and Lip Gloss; noninteractive commands retain their existing behavior. Do not add Python, TypeScript, or Rust duplicate CLIs. Keep the package independent of backend and MCP runtime dependencies. diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md index 537176bb2..609c75640 100644 --- a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md @@ -37,7 +37,10 @@ callers/tests, existing guide download command and a narrowly shared download owner, cli/tests/integration/test_tui_http.py and focused guide/findings process tests, cli/README.md, docs/design/cli-terminal-experience.md, docs/roadmap_status.md and affected WS-CLI-001 records/index when their next -boundary changes. No backend, dependencies, workflow or release changes. +boundary changes. The shared owner may use platform-specific permission files +and tests; cli/go.mod may promote the already-pinned x/sys dependency from +indirect to direct for Windows file-handle ACL operations. No new dependency or +version, backend, workflow or release changes. ## Boundaries and alternatives @@ -91,8 +94,8 @@ This cohesive slice exceeds the preferred 500-line L1 guideline: new staging and cancellation tests, native interaction regressions, and removal of both existing publication implementations account for much of the change. Splitting the helper from its consumers would leave duplicated custody rules; the two -read surfaces share the bounded navigation extension. No backend, CI, dependency -or policy boundary changes are included. Focused reviewers inspect filesystem +read surfaces share the bounded navigation extension. No backend, CI, new dependency +or product-policy boundary changes are included. Focused reviewers inspect filesystem security, state/navigation correctness and owner reuse separately. ## Concrete state and owner contract @@ -118,6 +121,15 @@ Abort/Publish are idempotent and race-serialized. Invalidation/Close marks an in-flight stage aborted; cleanup does not require delivery of its response, including on Windows where a writer may still hold an open file. +Windows mode bits do not enforce private access. Create the stage atomically +with a protected current-user-only DACL relative to the already-bound directory +handle, with exclusive-create/non-directory semantics; failure returns no stage. +Do not tighten inherited permissions after creation: a foreign reader could +retain a handle acquired before tightening. POSIX uses mode 0600. +Native tests inspect the actual platform permission boundary and exercise +symlink refusal without skipped assertions. This replaces the incorrect +cross-platform assumption that 0600 alone provides private staging. + SubmissionFindings returns a typed one-of result containing Raw plus Page or Run (only the mode-appropriate member populated), using its existing decoder. CLI JSON and text continue rendering Raw unchanged; TUI consumes Page. Receive @@ -127,7 +139,7 @@ and current-run flag. No alternative decoder or transport is introduced. Named proof additions: Go typed page/exact-run and unchanged raw-output tests; TUI selection/confirmation/current-receipt tests; download stage tests for -bound/symlink directories, existing file/symlink, 0600 temporary content, +bound/symlink directories, existing file/symlink, native private temporary content, checksum/size/missing/transport failure, and delayed writer Abort cleanup. Extend test_tui_http.py and its tui_terminal_support.py binary-response fixture to exercise actual PTY navigation, explicit confirmation, download success, diff --git a/cli/README.md b/cli/README.md index e45178fa6..16c1d7f04 100644 --- a/cli/README.md +++ b/cli/README.md @@ -40,7 +40,7 @@ for a later operation. Workstream rechecks each public read. | `/` | Filter loaded names/titles locally; Enter finishes and Esc cancels | | `c` / `?` | Show inert CLI arguments / keyboard help | | `d` in Guide | Enter an existing download directory; Enter reviews the target, `y` confirms | -| `q` / Ctrl+C | Quit and cancel outstanding client reads | +| `q` / Ctrl+C | Quit and cancel outstanding client reads; `q` is text during text entry, Ctrl+C always quits | The workspace makes no product mutation requests: it does not claim/start work, prepare or submit ZIPs, export records, poll, acquire tokens or save local diff --git a/cli/go.mod b/cli/go.mod index ae75b7d41..f10177f58 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -8,6 +8,7 @@ require ( charm.land/lipgloss/v2 v2.0.6 github.com/charmbracelet/x/term v0.2.2 github.com/spf13/cobra v1.10.2 + golang.org/x/sys v0.47.0 ) require ( @@ -26,5 +27,4 @@ require ( github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect golang.org/x/sync v0.22.0 // indirect - golang.org/x/sys v0.47.0 // indirect ) diff --git a/cli/internal/download/download.go b/cli/internal/download/download.go index 753325af4..fd641419c 100644 --- a/cli/internal/download/download.go +++ b/cli/internal/download/download.go @@ -62,7 +62,7 @@ func (r *Root) Stage(name string) (*Stage, error) { return nil, errors.New("download target already exists or is inaccessible") } temporary := ".workstream-download-" + rand.Text() - file, err := r.root.OpenFile(temporary, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600) + file, err := createPrivateFile(r.root, temporary) if err != nil { return nil, errors.New("cannot create private download") } diff --git a/cli/internal/download/download_test.go b/cli/internal/download/download_test.go index 71ff6d225..3951d0041 100644 --- a/cli/internal/download/download_test.go +++ b/cli/internal/download/download_test.go @@ -3,7 +3,6 @@ package download import ( "os" "path/filepath" - "runtime" "sync" "testing" ) @@ -20,6 +19,7 @@ func TestStagePublishesPrivateFileWithoutOverwrite(t *testing.T) { t.Fatal(err) } defer stage.Abort() + assertPrivateStage(t, stage.file) if _, err := stage.Write([]byte("verified bytes")); err != nil { t.Fatal(err) } @@ -27,10 +27,6 @@ func TestStagePublishesPrivateFileWithoutOverwrite(t *testing.T) { if err != nil || len(entries) != 1 || entries[0].Name() == "guide.pdf" { t.Fatal("bytes were not privately staged", entries, err) } - info, err := entries[0].Info() - if err != nil || info.Mode().Perm() != 0600 { - t.Fatal("stage is not private", info, err) - } if err := stage.Seal(); err != nil { t.Fatal(err) } @@ -64,13 +60,11 @@ func TestStageRefusesExistingFileAndSymlink(t *testing.T) { if _, err := root.Stage("existing"); err == nil { t.Fatal("existing file accepted") } - if runtime.GOOS != "windows" { - if err := os.Symlink(filepath.Join(directory, "existing"), filepath.Join(directory, "link")); err != nil { - t.Fatal(err) - } - if _, err := root.Stage("link"); err == nil { - t.Fatal("existing symlink accepted") - } + if err := os.Symlink(filepath.Join(directory, "existing"), filepath.Join(directory, "link")); err != nil { + t.Fatal(err) + } + if _, err := root.Stage("link"); err == nil { + t.Fatal("existing symlink accepted") } if body, _ := os.ReadFile(filepath.Join(directory, "existing")); string(body) != "untouched" { t.Fatal("existing target changed") @@ -78,9 +72,6 @@ func TestStageRefusesExistingFileAndSymlink(t *testing.T) { } func TestOpenRootRejectsDirectorySymlink(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("ordinary test user cannot reliably create a directory symlink") - } directory := t.TempDir() link := filepath.Join(t.TempDir(), "destination") if err := os.Symlink(directory, link); err != nil { diff --git a/cli/internal/download/private_other.go b/cli/internal/download/private_other.go new file mode 100644 index 000000000..696ade0e2 --- /dev/null +++ b/cli/internal/download/private_other.go @@ -0,0 +1,9 @@ +//go:build !windows + +package download + +import "os" + +func createPrivateFile(root *os.Root, name string) (*os.File, error) { + return root.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600) +} diff --git a/cli/internal/download/private_other_test.go b/cli/internal/download/private_other_test.go new file mode 100644 index 000000000..6eeacb9f2 --- /dev/null +++ b/cli/internal/download/private_other_test.go @@ -0,0 +1,16 @@ +//go:build !windows + +package download + +import ( + "os" + "testing" +) + +func assertPrivateStage(t *testing.T, file *os.File) { + t.Helper() + info, err := file.Stat() + if err != nil || info.Mode().Perm() != 0600 { + t.Fatal("stage is not owner-only", info, err) + } +} diff --git a/cli/internal/download/private_windows.go b/cli/internal/download/private_windows.go new file mode 100644 index 000000000..0899bb9d2 --- /dev/null +++ b/cli/internal/download/private_windows.go @@ -0,0 +1,95 @@ +//go:build windows + +package download + +import ( + "os" + "runtime" + "unsafe" + + "golang.org/x/sys/windows" +) + +const fileAllAccess = windows.STANDARD_RIGHTS_REQUIRED | windows.SYNCHRONIZE | 0x1ff + +// createPrivateFile atomically creates a current-user-only file relative to +// the already-bound os.Root directory. The protected DACL is present at object +// creation; there is deliberately no post-creation ACL fallback. +func createPrivateFile(root *os.Root, name string) (*os.File, error) { + directory, err := root.Open(".") + if err != nil { + return nil, err + } + defer directory.Close() + + user, err := windows.GetCurrentProcessToken().GetTokenUser() + if err != nil { + return nil, err + } + var pinner runtime.Pinner + pinner.Pin(user.User.Sid) + defer pinner.Unpin() + access := []windows.EXPLICIT_ACCESS{{ + AccessPermissions: fileAllAccess, + AccessMode: windows.GRANT_ACCESS, + Inheritance: windows.NO_INHERITANCE, + Trustee: windows.TRUSTEE{ + TrusteeForm: windows.TRUSTEE_IS_SID, + TrusteeType: windows.TRUSTEE_IS_USER, + TrusteeValue: windows.TrusteeValueFromSID(user.User.Sid), + }, + }} + descriptor, err := windows.BuildSecurityDescriptor(nil, nil, access, nil, nil) + if err != nil { + return nil, err + } + absolute, err := descriptor.ToAbsolute() + if err != nil { + return nil, err + } + if err := absolute.SetControl(windows.SE_DACL_PROTECTED, windows.SE_DACL_PROTECTED); err != nil { + return nil, err + } + descriptor, err = absolute.ToSelfRelative() + if err != nil { + return nil, err + } + objectName, err := windows.NewNTUnicodeString(name) + if err != nil { + return nil, err + } + attributes := &windows.OBJECT_ATTRIBUTES{ + RootDirectory: windows.Handle(directory.Fd()), + ObjectName: objectName, + Attributes: windows.OBJ_CASE_INSENSITIVE | windows.OBJ_DONT_REPARSE, + SecurityDescriptor: descriptor, + } + attributes.Length = uint32(unsafe.Sizeof(*attributes)) + var handle windows.Handle + err = windows.NtCreateFile( + &handle, + windows.FILE_GENERIC_WRITE|windows.STANDARD_RIGHTS_READ|windows.FILE_READ_ATTRIBUTES|windows.FILE_READ_EA|windows.SYNCHRONIZE, + attributes, + &windows.IO_STATUS_BLOCK{}, + nil, + windows.FILE_ATTRIBUTE_NORMAL, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + windows.FILE_CREATE, + windows.FILE_NON_DIRECTORY_FILE|windows.FILE_OPEN_REPARSE_POINT|windows.FILE_SYNCHRONOUS_IO_NONALERT, + 0, + 0, + ) + runtime.KeepAlive(objectName) + runtime.KeepAlive(descriptor) + runtime.KeepAlive(user) + if err != nil { + return nil, err + } + file := os.NewFile(uintptr(handle), name) + if file == nil { + _ = windows.CloseHandle(handle) + _ = root.Remove(name) + return nil, windows.ERROR_INVALID_HANDLE + } + return file, nil +} diff --git a/cli/internal/download/private_windows_test.go b/cli/internal/download/private_windows_test.go new file mode 100644 index 000000000..d495aa773 --- /dev/null +++ b/cli/internal/download/private_windows_test.go @@ -0,0 +1,43 @@ +//go:build windows + +package download + +import ( + "os" + "runtime" + "testing" + "unsafe" + + "golang.org/x/sys/windows" +) + +func assertPrivateStage(t *testing.T, file *os.File) { + t.Helper() + sd, err := windows.GetSecurityInfo(windows.Handle(file.Fd()), windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION) + if err != nil { + t.Fatal(err) + } + control, _, err := sd.Control() + if err != nil || control&windows.SE_DACL_PROTECTED == 0 { + t.Fatal("stage DACL inherits access", control, err) + } + acl, _, err := sd.DACL() + if err != nil || acl == nil || acl.AceCount != 1 { + t.Fatal("stage does not have exactly one owner ACE", acl, err) + } + var ace *windows.ACCESS_ALLOWED_ACE + if err := windows.GetAce(acl, 0, &ace); err != nil { + t.Fatal(err) + } + user, err := windows.GetCurrentProcessToken().GetTokenUser() + if err != nil { + t.Fatal(err) + } + aceSID := (*windows.SID)(unsafe.Pointer(&ace.SidStart)) + if ace.Header.AceType != windows.ACCESS_ALLOWED_ACE_TYPE || ace.Header.AceFlags != 0 || + ace.Mask != fileAllAccess || !aceSID.Equals(user.User.Sid) { + t.Fatal("stage DACL is not current-user full access", ace.Header, ace.Mask, aceSID) + } + runtime.KeepAlive(sd) + runtime.KeepAlive(user) +} diff --git a/cli/internal/tui/guide_findings_test.go b/cli/internal/tui/guide_findings_test.go index c928bbd4f..066227ed4 100644 --- a/cli/internal/tui/guide_findings_test.go +++ b/cli/internal/tui/guide_findings_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" "github.com/Flow-Research/workstream/cli/internal/api" ) @@ -44,15 +46,25 @@ func TestGuidePreservesOrderAndPublishesOnlyAfterConfirmation(t *testing.T) { }} m := openMineGuide(t, f) text := m.detailText() - if strings.Index(text, "First") > strings.Index(text, "Second") || !strings.Contains(text, "bytes not downloaded") { + for _, want := range []string{"Label: First", "Order: 2", doc1.SHA256, "Label: Second", "Order: 7", doc2.SHA256, "bytes not downloaded"} { + if !strings.Contains(text, want) { + t.Fatal(want, text) + } + } + if strings.Index(text, "Label: First") > strings.Index(text, "Label: Second") { t.Fatal(text) } - directory := filepath.Join(t.TempDir(), "q?gr") + directory := filepath.Join(t.TempDir(), "qgr") if err := os.Mkdir(directory, 0700); err != nil { t.Fatal(err) } press(t, m, "down") press(t, m, "d") + press(t, m, "?") + if m.guideDirectory != "?" { + t.Fatal("question mark triggered a global key", m.guideDirectory) + } + press(t, m, "backspace") for _, character := range directory { press(t, m, string(character)) } @@ -64,8 +76,14 @@ func TestGuidePreservesOrderAndPublishesOnlyAfterConfirmation(t *testing.T) { } before := len(f.calls) press(t, m, "enter") - if !strings.Contains(m.detailText(), "Confirm download") || len(f.calls) != before || m.loading { - t.Fatal(m.detailText()) + confirmation := m.detailText() + for _, want := range []string{"Confirm download", "Task: " + taskID, "Document: " + doc2.DocumentID, "Filename: " + doc2.Filename()} { + if !strings.Contains(confirmation, want) { + t.Fatal(want, confirmation) + } + } + if len(f.calls) != before || m.loading { + t.Fatal(confirmation) } if entries, _ := os.ReadDir(directory); len(entries) != 0 { t.Fatal("confirmation performed I/O", entries) @@ -143,9 +161,13 @@ func TestFindingsNavigationPaginationAndMisbinding(t *testing.T) { if requests == 3 { task = actorID } + predecessor := actorID page := api.SubmissionCheckerPage{Items: []api.SubmissionCheckerRun{{ ID: runID, Task: task, Submission: submissionID, Version: nil, Status: "completed", Attempt: nil, - Current: requests != 2, Results: []api.ContributorFinding{{ID: findingID, Checker: "lint", CheckID: &check, Path: &path, Line: &line, Status: "coverage_incomplete", Severity: "medium", Message: &message, Fix: &fix}}, + Current: requests != 2, Predecessor: &predecessor, Results: []api.ContributorFinding{ + {ID: findingID, Checker: "lint", CheckID: &check, Path: &path, Line: &line, Status: "coverage_incomplete", Severity: "medium", Message: &message, Fix: &fix}, + {ID: policyID, Checker: "format", Status: "passed", Severity: "info"}, + }, }}} if next == nil { page.Cursor = &cursor @@ -161,7 +183,7 @@ func TestFindingsNavigationPaginationAndMisbinding(t *testing.T) { press(t, m, "enter") press(t, m, "tab") text := m.detailText() - for _, want := range []string{"Run: " + runID, "Current: true", "Checker: lint", "Check ID: lint.safe", "Finding status: coverage_incomplete", "Severity: medium", "Path: answer.md", "Line: 8", "Suggested fix: repair", "not acceptance"} { + for _, want := range []string{"Run: " + runID, "Current: true", "Predecessor run: " + actorID, "Checker: lint", "Check ID: lint.safe", "Finding status: coverage_incomplete", "Severity: medium", "Path: answer.md", "Line: 8", "Suggested fix: repair", "Checker: format\nCheck ID: not supplied\nFinding status: passed\nSeverity: info\nPath: not supplied\nLine: not supplied\nMessage: not supplied\nSuggested fix: not supplied", "not acceptance"} { if !strings.Contains(text, want) { t.Fatal(want, text) } @@ -179,6 +201,37 @@ func TestFindingsNavigationPaginationAndMisbinding(t *testing.T) { } } +func TestGuideAndFindingsViewsFitMinimumTerminal(t *testing.T) { + document := api.TaskGuideDocument{DocumentID: actorID, Order: 4, Label: strings.Repeat("wide guide label ", 20), MediaType: "text/markdown", ByteCount: 5, SHA256: "sha256:" + strings.Repeat("a", 64)} + guideClient := &fakeClient{context: func(context.Context, string) (api.Result[api.WorkContext], error) { + return api.Result[api.WorkContext]{Value: guideContext(document)}, nil + }} + guide := openMineGuide(t, guideClient) + guide.Update(tea.WindowSizeMsg{Width: 40, Height: 10}) + assertBoundedView(t, guide.View().Content, 40, 10, "GUIDE") + + findings := boot(t, &fakeClient{}) + press(t, findings, "enter") + press(t, findings, "tab") + press(t, findings, "tab") + press(t, findings, "enter") + press(t, findings, "tab") + findings.Update(tea.WindowSizeMsg{Width: 40, Height: 10}) + assertBoundedView(t, findings.View().Content, 40, 10, "FINDINGS") +} + +func assertBoundedView(t *testing.T, content string, width, height int, heading string) { + t.Helper() + if !strings.Contains(content, heading) || lipgloss.Height(content) > height { + t.Fatal("view is not usable at minimum size", content) + } + for _, line := range strings.Split(content, "\n") { + if lipgloss.Width(line) > width { + t.Fatalf("line exceeds %d cells: %q", width, line) + } + } +} + func TestDownloadPanicAbortsPrivateStage(t *testing.T) { document := api.TaskGuideDocument{DocumentID: actorID, Label: "Guide", MediaType: "text/markdown", ByteCount: 5, SHA256: "sha256:" + strings.Repeat("a", 64)} f := &fakeClient{context: func(context.Context, string) (api.Result[api.WorkContext], error) { diff --git a/cli/internal/tui/handoff.go b/cli/internal/tui/handoff.go index bd8a7cf79..8bd762b7e 100644 --- a/cli/internal/tui/handoff.go +++ b/cli/internal/tui/handoff.go @@ -9,7 +9,14 @@ func (m *Model) handoffText() string { } else if m.detail { if selected, ok := m.selectedRow(); ok { if m.tab == submissions { - args = append(args, "submission", "show", selected.id) + if m.section == 1 { + args = append(args, "submission", "findings", selected.id, "--limit", "50") + if m.findingsPageCursor != "" { + args = append(args, "--cursor", m.findingsPageCursor) + } + } else { + args = append(args, "submission", "show", selected.id) + } } else { verb := "show" if m.section == 1 { @@ -57,13 +64,5 @@ func (m *Model) handoffText() string { out.WriteString(" argument: " + m.clean(arg) + "\n") } } - if m.detail && m.tab == submissions && m.section == 0 { - if selected, ok := m.selectedRow(); ok { - out.WriteString("\nDetailed findings: external manual read only.\n") - for _, arg := range []string{"workstream", "submission", "findings", selected.id} { - out.WriteString(" argument: " + m.clean(arg) + "\n") - } - } - } return out.String() } diff --git a/cli/internal/tui/handoff_test.go b/cli/internal/tui/handoff_test.go index 4bcefec68..234067e16 100644 --- a/cli/internal/tui/handoff_test.go +++ b/cli/internal/tui/handoff_test.go @@ -27,9 +27,27 @@ func TestHandoffUsesInertArgumentsAndNoExportReads(t *testing.T) { before = len(f.calls) press(t, m, "c") text = m.handoffText() - if !strings.Contains(text, "argument: submission\n argument: findings\n argument: "+submissionID) || len(f.calls) != before { + if !strings.Contains(text, "argument: submission\n argument: show\n argument: "+submissionID) || strings.Contains(text, "external manual") || len(f.calls) != before { t.Fatal("handoff dispatched a read", text) } + press(t, m, "esc") + press(t, m, "tab") + m.findingsPageCursor = "opaque findings cursor" + before = len(f.calls) + press(t, m, "c") + text = m.handoffText() + for _, want := range []string{ + "argument: submission\n argument: findings\n argument: " + submissionID, + "argument: --limit\n argument: 50", + "argument: --cursor\n argument: opaque findings cursor", + } { + if !strings.Contains(text, want) { + t.Fatal(text) + } + } + if len(f.calls) != before { + t.Fatal("Findings handoff dispatched a read", f.calls) + } } func TestHandoffPreservesPageCursorAsSeparateArgument(t *testing.T) { diff --git a/cli/internal/tui/model.go b/cli/internal/tui/model.go index 9309524ac..106ee58f9 100644 --- a/cli/internal/tui/model.go +++ b/cli/internal/tui/model.go @@ -265,13 +265,6 @@ func (m *Model) key(key tea.KeyPressMsg) tea.Cmd { return m.begin(pageRead) } if k == "r" { - if m.loading { - if m.detail && m.tab == mine && m.section == 3 && m.request.kind == guideDownloadRead { - m.context, m.guideMode, m.guideNotice = nil, guideBrowse, "" - return m.begin(contextRead) - } - return nil - } if m.detail { m.guideNotice = "" if m.tab == submissions && m.section == 1 { diff --git a/cli/internal/tui/requests_test.go b/cli/internal/tui/requests_test.go index e3d4acd7b..364074881 100644 --- a/cli/internal/tui/requests_test.go +++ b/cli/internal/tui/requests_test.go @@ -150,3 +150,56 @@ func TestTransientProfileFailureRetriesBindingBeforeProjects(t *testing.T) { }) } } + +func TestRefreshReplacesDelayedFirstFindingsRead(t *testing.T) { + calls := 0 + f := &fakeClient{findings: func(context.Context, string, string, int, *string) (api.Result[api.SubmissionFindingsResult], error) { + calls++ + page := api.SubmissionCheckerPage{Items: []api.SubmissionCheckerRun{{ID: runID, Task: taskID, Submission: submissionID, Current: calls == 1}}} + return api.Result[api.SubmissionFindingsResult]{Value: api.SubmissionFindingsResult{Page: &page}}, nil + }} + m := boot(t, f) + press(t, m, "enter") + press(t, m, "tab") + press(t, m, "tab") + press(t, m, "enter") + _, delayed := m.Update(keyMessage("tab")) + _, refreshed := m.Update(keyMessage("r")) + if delayed == nil || refreshed == nil { + t.Fatal("pending first findings read was not replaceable") + } + apply(t, m, refreshed) + m.Update(delayed()) + if calls != 2 || m.findings == nil || !m.findings.Items[0].Current || m.loading || m.findingsPage != 1 { + t.Fatal("late first findings response replaced refresh", calls, m.findings, m.findingsPage) + } +} + +func TestRefreshReplacesDelayedNextFindingsPageWithFirstPage(t *testing.T) { + cursor := "next-page" + calls := 0 + f := &fakeClient{findings: func(_ context.Context, _ string, _ string, _ int, next *string) (api.Result[api.SubmissionFindingsResult], error) { + calls++ + page := api.SubmissionCheckerPage{Items: []api.SubmissionCheckerRun{{ID: runID, Task: taskID, Submission: submissionID, Current: next == nil}}} + if next == nil { + page.Cursor = &cursor + } + return api.Result[api.SubmissionFindingsResult]{Value: api.SubmissionFindingsResult{Page: &page}}, nil + }} + m := boot(t, f) + press(t, m, "enter") + press(t, m, "tab") + press(t, m, "tab") + press(t, m, "enter") + press(t, m, "tab") + _, delayedNext := m.Update(keyMessage("n")) + _, refreshed := m.Update(keyMessage("r")) + if delayedNext == nil || refreshed == nil { + t.Fatal("pending continuation was not replaceable") + } + apply(t, m, refreshed) + m.Update(delayedNext()) + if calls != 3 || m.findings == nil || !m.findings.Items[0].Current || m.findingsPage != 1 || m.findingsPageCursor != "" { + t.Fatal("late continuation replaced refreshed first page", calls, m.findings, m.findingsPage, m.findingsPageCursor) + } +} diff --git a/cli/tests/integration/test_tui_http.py b/cli/tests/integration/test_tui_http.py index 37e05bfc7..a94c75372 100644 --- a/cli/tests/integration/test_tui_http.py +++ b/cli/tests/integration/test_tui_http.py @@ -139,8 +139,10 @@ def open_guide(terminal): terminal.wait(SECRET) terminal.send("\t") terminal.wait("CONTEXT") + terminal.wait("Locked guide version: guide-1") terminal.send("\t") terminal.wait("INTAKE") + terminal.wait("answer.md") terminal.send("\t") terminal.wait("GUIDE") terminal.wait("Locked instructions") @@ -340,6 +342,83 @@ def findings(query): terminal.quit() +@pytest.mark.parametrize("delayed_page", ["first", "next"]) +def test_refresh_pending_findings_restarts_first_page_and_discards_late_result( + delayed_page, +): + reads = Reads() + calls = [] + started, release, returned = Event(), Event(), Event() + stale = deepcopy(CHECKER) + stale["results"][0]["worker_message"] = "STALE_FINDING_AFTER_REFRESH" + fresh = deepcopy(CHECKER) + fresh["results"][0]["worker_message"] = "Fresh findings after refresh" + + def findings(query): + calls.append(query) + should_delay = len(calls) == (1 if delayed_page == "first" else 2) + if should_delay: + started.set() + assert release.wait(10) + returned.set() + return 200, page(stale) + if delayed_page == "next" and len(calls) == 1: + return 200, page(CHECKER, CURSOR) + assert query == {"limit": ["50"]} + return 200, page(fresh) + + reads.overrides[f"/api/v1/submissions/{SUBMISSION}/checker-runs"] = findings + with ( + socket_peer(reads) as (origin, _), + Terminal(origin, TOKEN, "--plain", rows=60) as terminal, + ): + try: + open_findings(terminal) + if delayed_page == "next": + terminal.wait("quality.output") + terminal.send("n") + assert started.wait(5) + terminal.send("r") + terminal.wait("Fresh findings after refresh") + assert len(calls) == (2 if delayed_page == "first" else 3) + release.set() + assert returned.wait(5) + terminal.pump(0.3) + assert "STALE_FINDING_AFTER_REFRESH" not in terminal.screen + assert "Fresh findings after refresh" in terminal.screen + terminal.quit() + finally: + release.set() + + +@pytest.mark.parametrize("surface", ["guide", "findings"]) +def test_guide_and_findings_remain_navigable_in_narrow_terminal(surface): + reads = guide_reads() + reads.overrides[f"/api/v1/submissions/{SUBMISSION}/checker-runs"] = lambda _: ( + 200, + page(CHECKER), + ) + with ( + socket_peer(reads) as (origin, _), + Terminal(origin, TOKEN, "--plain", columns=60, rows=24) as terminal, + ): + if surface == "guide": + open_guide(terminal) + terminal.send("d") + terminal.wait("Download directory") + terminal.send("\x1b") + terminal.wait("Locked instructions") + else: + open_findings(terminal) + terminal.wait("FINDINGS") + terminal.wait(RUN) + terminal.send("c") + terminal.wait("argument: findings") + assert "argument: show" not in terminal.screen + assert TOKEN.encode() not in terminal.raw + terminal.quit() + + @pytest.mark.parametrize("field", ["task_id", "submission_id"]) def test_findings_refuse_wrong_task_or_submission(field): reads = Reads() @@ -433,8 +512,8 @@ def test_keyboard_journey_uses_only_nine_fresh_public_reads(): terminal.wait("Exact owned submission summary") assert SUBMISSION in terminal.screen and TASK in terminal.screen terminal.send("c") - terminal.wait("findings") - assert "manual" in terminal.screen.lower() + terminal.wait("argument: show") + assert "external manual read only" not in terminal.screen.lower() terminal.quit() assert [urlsplit(target).path for _, target, _ in requests] == [ "/api/v1/actors/me", diff --git a/docs/design/cli-terminal-experience.md b/docs/design/cli-terminal-experience.md index c5434c92f..c27ddc633 100644 --- a/docs/design/cli-terminal-experience.md +++ b/docs/design/cli-terminal-experience.md @@ -462,7 +462,7 @@ private staging, verified bytes and atomic no-overwrite publication. | Boundary | Dependency and next usable result | |---|---| -| Contributor read-only workbench | Nine existing typed reads and merged backend guards → project/Ready/active Mine/Submissions browse/inspect with explicit pagination; detailed findings and Evidence are manual CLI handoffs; no API-client edit or catalogue dependency | +| Contributor read-only workbench foundation (-48) | Nine existing typed reads and merged backend guards → project/Ready/active Mine/Submissions browse/inspect with explicit pagination; extended by the guide/findings slice below; no catalogue dependency | | Guide/findings extension | Existing WorkContext and verified document reader plus typed existing checker-run result → ordered guide selection, confirmed download and findings pagination; no backend changes or product writes | | Later in-TUI contribution Evidence | Narrow shared typed single-page contribution port and preserved complete-export regressions → explicit per-page evidence browsing; not a first-slice prerequisite | | Claim/start | Read workbench + reviewed per-operation private receipt and exact-key recovery → separate claim and start confirmations | diff --git a/docs/roadmap_status.md b/docs/roadmap_status.md index 2e409529b..944daf767 100644 --- a/docs/roadmap_status.md +++ b/docs/roadmap_status.md @@ -580,7 +580,7 @@ cannot be reused as post-submission review-gate evidence. See the | Review decision and revision | **Checker revision implemented; human decisions planned** | Review/revision policy identities and mutation authority, with `requires_second_review` fixed false through typed and database boundaries; REV-04A immutable Review storage; REV-04B source storage; REV-04C hidden source-neutral FinalAcceptance/TASK/CON participant; ARCH-04E2-B source AUTH and complete database/audit/outbox closure for hidden automated outcomes. PILOT-08 supplies required project-policy N>=0, assignment-scoped attempts, immediate exhaustion/release, atomic complete-context rebase and acknowledgement, setup-blocked clock pause/resume, PM attention/repair, and durable timeout rejection/reclaim. The CLI now makes the existing exact manager repair POST available with caller-owned key and source, preserving backend eligibility and replay authority | Genuine human decision authority/composition and revision integration remain #554 work under the same intended policy; hosted operation remains unproven | | Contribution and compensation truth | **Hidden shared participant plus public policy administration** | ContributionPolicyVersion persistence; lifecycle-audit participant; adapter bindings and project-points units (internal Finance commands); public Finance policy administration; REV-04B shared acceptance source storage; CON-03C immutable ContributionRecord/CompensationAward storage; obsolete guide-keyed payment storage and task-local payment fields removed; CON-07 source-neutral submitter participant and complete frozen award sets; REV-04C composes it with FinalAcceptance/TASK effects in the caller transaction; 04F2 checker remediation and terminal infrastructure retry create no contribution or award. The bounded local public drill produced exactly one false-policy FinalAcceptance, submitter ContributionRecord and project-points award, with no Review | ARCH-04E2-B supplies exact routing authority/evidence, complete-set database closure and audit/outbox under REV-12A4A. Selected production request/completion delivery, 04F3 exact-cause replacement and the 04F4 owner with supported public terminal recovery are implemented; supported false-guide activation is implemented. Only actual Reviews create reviewer records. Obligation/root/ordinal custody remains deferred until fulfillment activation; no public recognition or fulfillment route is live | | Contribution evidence reads | **Public fixed-membership REST and complete CLI export** | PILOT-10A exposes current human self-read and one-project JSON/CSV export for covering Project Managers or exact-project Finance Authorities. Each page has fresh authority and one repeatable-read response transaction; selections through 100,000 contributions retain page-one membership behind signed scope/filter/format/limit-bound cursors, with complete zero/one/two immutable awards, exact decimals, recorded-time filters and formula-neutralized CSV. The CLI follows JSON or CSV continuation internally with unchanged filters, validates cross-page order/identity, and publishes only the complete JSON result or one private no-overwrite CSV | No recognition mutation, fulfillment, payout, deployed-service or published-binary claim | -| CLI and terminal interface | **Public CLI clients and opt-in read-only contributor workbench** | Existing explicit commands retain text/JSON/noninteractive behavior. [WS-CLI-001-48](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-48.md) adds exact-grant project, Ready/active Mine/own Submission browsing and fresh details/context/intake through nine existing typed reads, with explicit pagination and fresh-denial handling | Detailed findings/Evidence remain manual CLI handoffs; a shared typed single-page contribution port precedes in-TUI Evidence. TUI writes/recovery and management remain separate. Protected first binary publication and hosted journey proof remain outstanding | +| CLI and terminal interface | **Public CLI clients and opt-in contributor read workbench** | Existing explicit commands retain text/JSON/noninteractive behavior. [WS-CLI-001-48](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-48.md) adds exact-grant project, Ready/active Mine/own Submission browsing and fresh details/context/intake with explicit pagination and fresh-denial handling. [WS-CLI-001-49](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md) adds locked-guide metadata, confirmed verified local downloads and paged checker findings through the shared client | Contribution Evidence remains a manual CLI handoff; a shared typed single-page contribution port precedes in-TUI Contribution Evidence. TUI product writes/recovery and management remain separate. Protected first binary publication and hosted journey proof remain outstanding | | Fulfillment, reconciliation, and audit | **Planned** | Shared audit foundations, provider-neutral adapter convention, AUTH-OUTBOX-02 live dispatcher authority, retained phase audit decisions, Celery delivery/recovery scans and CON-02B custody | Feature-specific handlers and authority, conditional award fulfillment, callbacks, idempotent recovery, reconciliation, bounded operational reads, and release controls | | Runtime diagnostics | **Implemented foundation; release configuration prepared** | Closed structured logging, explicit API/Celery tracing, bounded metrics, safe correlation and optional typed OTLP export; hosted composition accepts only an optional HTTPS collector origin and keeps exporter outage outside product correctness | Restrict broker publishers; configure and secure collector/log access, egress, encryption and finite deletion; prove privacy, correlation and collector outage during the deployed release drill; no deployed monitoring claim | | Frontend and pilot | **Local runtime foundation; release configuration prepared; project inputs prepared** | An original [Terminal Systems guide, example and five task briefs](../pilots/terminal_systems/README.md) provide inputs for fresh project onboarding. The fixed five-task sample, its checker, private runtime continuation and sample-only proof workflow are retired. Checkout-isolated Docker Compose API, prefork Celery, Beat, PostgreSQL, Redis and MinIO remain available, alongside digest-only hosted release composition and the generic external image service. The generic image command tests project samples and publishes a verified digest to ECR without registering or activating it. The opt-in [local agent-loop overlay](engineering/local-agent-loop.md) adds that service, a separately built networkless proof checker and operator scripts; Docker Desktop (Apple Silicon) proof covers registration, checker execution and lifecycle activation, not this project’s requirements. Setup supports inference and explicitly manager-authored rules through the same governed proposal path. Inference is limited to PRE/POST specifications, with strict JSON and bounded correction; PM policy choices remain separate. The retained real Terminal Systems proposal preserves null project limits on the unchanged guide; its independent review predates the PRE/POST-only scope, with ten explicit implementation/runtime gaps and no bindings in that retained proposal. The [project-owned PRE image](../pilots/terminal_systems/checkers/README.md) now implements layout and metadata intake with local rule and networkless container samples; it has no task-name allowlist or shared-runner project code. The independent offline gVisor feasibility spike remains historical mechanism evidence | Obtain a fresh PRE/POST-only correction of the retained proposal, then reconcile the implemented PRE image, implement the remaining POST checks and resolve their runtime/material gaps; register the required images, obtain corrected policy bindings and separate approvals, then activate, import, screen and release tasks. Then prove the real contribution journey, runtime limits and interruption recovery on the intended local/hosted stack. Guide assets, image builds and former sample results are not live-project evidence. Hosted identity/AWS configuration, isolation, diagnostics, backup/restore and broader macOS runtime proof remain; the agent-loop overlay still needs approved guide bindings and a persistent-stack FinalAcceptance drill | @@ -920,8 +920,10 @@ cannot be reused as post-submission review-gate evidence. See the `project list`, without changing exact-grant `me projects`. The [read-only contributor TUI](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-48.md) uses those existing typed reads for one-page navigation and fresh details, - without changing product authority or adding writes. Detailed findings and - contribution evidence remain manual CLI handoffs; broader TUI work remains + without changing product authority or adding product writes. + [WS-CLI-001-49](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-49.md) + adds confirmed verified locked-guide downloads and paged checker findings. + Contribution Evidence remains a manual CLI handoff; broader TUI work remains separate. Further public commands require their own usable API contracts; binary-release automation is prepared, while protected first publication remains outstanding. CLI reads do not complete unfinished product lifecycle work. @@ -1353,7 +1355,9 @@ Delivered foundations (bounded local journey proven; no deployment claim) WS-CLI-001-44 native CLI release proof + gated manual publisher (not yet published) WS-CLI-001-48 existing typed public reads -> opt-in read-only contributor TUI exact-grant projects -> Ready / active Mine / own Submissions -> fresh detail - explicit Mine context/intake; findings/Evidence -> manual CLI handoff + explicit Mine context/intake + WS-CLI-001-49 -> locked-guide metadata + confirmed verified local download + own Submission -> paged checker findings; Contribution Evidence -> manual CLI handoff later Evidence needs typed single-page port; writes need per-operation recovery task/assignment/Submission lineage + public ZIP intake/admission-backed creation ARCH-03D exact approved historical guide through durable intake handoff @@ -1486,7 +1490,7 @@ and award history never changes. ## Remaining Release Gates Client scope remains distinct from product completion: the local read-only TUI -does not complete human review, payment or hosted proof. In-TUI evidence and +does not complete human review, payment or hosted proof. In-TUI Contribution Evidence and mutations are separate optional interface boundaries, not new backend lifecycle prerequisites. CLI binary distribution still requires protected publication and independent public-asset/install verification. From 1cd6d3aaa55ffb0a1c4127190fb4ada88aa5dc5b Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 26 Oct 2026 12:46:39 +0100 Subject: [PATCH 7/9] [WS-CLI-001-49] Expose native staging proof failures and synchronize resize proof --- cli/internal/download/private_windows.go | 17 +++++++++-------- cli/internal/download/private_windows_test.go | 17 +++++++++++++++++ cli/internal/tui/guide_findings_test.go | 3 +++ cli/tests/integration/test_tui_http.py | 1 + 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/cli/internal/download/private_windows.go b/cli/internal/download/private_windows.go index 0899bb9d2..434e3c22d 100644 --- a/cli/internal/download/private_windows.go +++ b/cli/internal/download/private_windows.go @@ -3,6 +3,7 @@ package download import ( + "fmt" "os" "runtime" "unsafe" @@ -18,13 +19,13 @@ const fileAllAccess = windows.STANDARD_RIGHTS_REQUIRED | windows.SYNCHRONIZE | 0 func createPrivateFile(root *os.Root, name string) (*os.File, error) { directory, err := root.Open(".") if err != nil { - return nil, err + return nil, fmt.Errorf("open bound directory handle: %w", err) } defer directory.Close() user, err := windows.GetCurrentProcessToken().GetTokenUser() if err != nil { - return nil, err + return nil, fmt.Errorf("read token user: %w", err) } var pinner runtime.Pinner pinner.Pin(user.User.Sid) @@ -41,22 +42,22 @@ func createPrivateFile(root *os.Root, name string) (*os.File, error) { }} descriptor, err := windows.BuildSecurityDescriptor(nil, nil, access, nil, nil) if err != nil { - return nil, err + return nil, fmt.Errorf("build current-user descriptor: %w", err) } absolute, err := descriptor.ToAbsolute() if err != nil { - return nil, err + return nil, fmt.Errorf("make descriptor absolute: %w", err) } if err := absolute.SetControl(windows.SE_DACL_PROTECTED, windows.SE_DACL_PROTECTED); err != nil { - return nil, err + return nil, fmt.Errorf("protect descriptor DACL: %w", err) } descriptor, err = absolute.ToSelfRelative() if err != nil { - return nil, err + return nil, fmt.Errorf("make descriptor self-relative: %w", err) } objectName, err := windows.NewNTUnicodeString(name) if err != nil { - return nil, err + return nil, fmt.Errorf("encode private leaf: %w", err) } attributes := &windows.OBJECT_ATTRIBUTES{ RootDirectory: windows.Handle(directory.Fd()), @@ -83,7 +84,7 @@ func createPrivateFile(root *os.Root, name string) (*os.File, error) { runtime.KeepAlive(descriptor) runtime.KeepAlive(user) if err != nil { - return nil, err + return nil, fmt.Errorf("create protected private file: %w", err) } file := os.NewFile(uintptr(handle), name) if file == nil { diff --git a/cli/internal/download/private_windows_test.go b/cli/internal/download/private_windows_test.go index d495aa773..70d9a9598 100644 --- a/cli/internal/download/private_windows_test.go +++ b/cli/internal/download/private_windows_test.go @@ -11,6 +11,23 @@ import ( "golang.org/x/sys/windows" ) +func TestAtomicPrivateFileCreation(t *testing.T) { + directory := t.TempDir() + root, err := os.OpenRoot(directory) + if err != nil { + t.Fatal(err) + } + defer root.Close() + const name = ".workstream-download-diagnostic" + file, err := createPrivateFile(root, name) + if err != nil { + t.Fatalf("createPrivateFile raw Windows error: %T: %v", err, err) + } + defer root.Remove(name) + defer file.Close() + assertPrivateStage(t, file) +} + func assertPrivateStage(t *testing.T, file *os.File) { t.Helper() sd, err := windows.GetSecurityInfo(windows.Handle(file.Fd()), windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION) diff --git a/cli/internal/tui/guide_findings_test.go b/cli/internal/tui/guide_findings_test.go index 066227ed4..0164eb966 100644 --- a/cli/internal/tui/guide_findings_test.go +++ b/cli/internal/tui/guide_findings_test.go @@ -133,6 +133,9 @@ func TestGuideRefreshAbortsDownloadAndReloadsFreshContext(t *testing.T) { press(t, m, directory) press(t, m, "enter") _, downloadCmd := m.Update(keyMessage("y")) + if downloadCmd == nil { + t.Fatal("download was not staged") + } _, refreshCmd := m.Update(keyMessage("r")) if refreshCmd == nil || m.request.kind != contextRead { t.Fatal("refresh did not replace pending download with Guide context read") diff --git a/cli/tests/integration/test_tui_http.py b/cli/tests/integration/test_tui_http.py index a94c75372..5c96ab159 100644 --- a/cli/tests/integration/test_tui_http.py +++ b/cli/tests/integration/test_tui_http.py @@ -770,6 +770,7 @@ def test_hostile_text_resizing_plain_and_ctrl_c_restore_terminal( terminal.wait(SECRET) before = len(requests) terminal.resize(80, 24) + terminal.wait(TASK) terminal.wait(SECRET) assert TASK in terminal.screen assert len(requests) == before From e5f5e3e259be45f3d83ea03fef79bc583638e8fb Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 26 Oct 2026 12:59:41 +0100 Subject: [PATCH 8/9] [WS-CLI-001-49] Create DACL-only Windows stages and correct terminal observer --- cli/internal/download/private_windows.go | 60 +++++++++++-------- cli/internal/download/private_windows_test.go | 44 +++++++++++--- cli/tests/integration/test_tui_http.py | 16 ++++- cli/tests/integration/tui_terminal_support.py | 7 ++- 4 files changed, 90 insertions(+), 37 deletions(-) diff --git a/cli/internal/download/private_windows.go b/cli/internal/download/private_windows.go index 434e3c22d..d307c5f33 100644 --- a/cli/internal/download/private_windows.go +++ b/cli/internal/download/private_windows.go @@ -27,33 +27,9 @@ func createPrivateFile(root *os.Root, name string) (*os.File, error) { if err != nil { return nil, fmt.Errorf("read token user: %w", err) } - var pinner runtime.Pinner - pinner.Pin(user.User.Sid) - defer pinner.Unpin() - access := []windows.EXPLICIT_ACCESS{{ - AccessPermissions: fileAllAccess, - AccessMode: windows.GRANT_ACCESS, - Inheritance: windows.NO_INHERITANCE, - Trustee: windows.TRUSTEE{ - TrusteeForm: windows.TRUSTEE_IS_SID, - TrusteeType: windows.TRUSTEE_IS_USER, - TrusteeValue: windows.TrusteeValueFromSID(user.User.Sid), - }, - }} - descriptor, err := windows.BuildSecurityDescriptor(nil, nil, access, nil, nil) - if err != nil { - return nil, fmt.Errorf("build current-user descriptor: %w", err) - } - absolute, err := descriptor.ToAbsolute() + descriptor, acl, err := currentUserDescriptor(user) if err != nil { - return nil, fmt.Errorf("make descriptor absolute: %w", err) - } - if err := absolute.SetControl(windows.SE_DACL_PROTECTED, windows.SE_DACL_PROTECTED); err != nil { - return nil, fmt.Errorf("protect descriptor DACL: %w", err) - } - descriptor, err = absolute.ToSelfRelative() - if err != nil { - return nil, fmt.Errorf("make descriptor self-relative: %w", err) + return nil, err } objectName, err := windows.NewNTUnicodeString(name) if err != nil { @@ -82,6 +58,7 @@ func createPrivateFile(root *os.Root, name string) (*os.File, error) { ) runtime.KeepAlive(objectName) runtime.KeepAlive(descriptor) + runtime.KeepAlive(acl) runtime.KeepAlive(user) if err != nil { return nil, fmt.Errorf("create protected private file: %w", err) @@ -94,3 +71,34 @@ func createPrivateFile(root *os.Root, name string) (*os.File, error) { } return file, nil } + +func currentUserDescriptor(user *windows.Tokenuser) (*windows.SECURITY_DESCRIPTOR, *windows.ACL, error) { + var pinner runtime.Pinner + pinner.Pin(user.User.Sid) + defer pinner.Unpin() + access := []windows.EXPLICIT_ACCESS{{ + AccessPermissions: fileAllAccess, + AccessMode: windows.GRANT_ACCESS, + Inheritance: windows.NO_INHERITANCE, + Trustee: windows.TRUSTEE{ + TrusteeForm: windows.TRUSTEE_IS_SID, + TrusteeType: windows.TRUSTEE_IS_USER, + TrusteeValue: windows.TrusteeValueFromSID(user.User.Sid), + }, + }} + acl, err := windows.ACLFromEntries(access, nil) + if err != nil { + return nil, nil, fmt.Errorf("build current-user DACL: %w", err) + } + descriptor, err := windows.NewSecurityDescriptor() + if err != nil { + return nil, nil, fmt.Errorf("initialize current-user descriptor: %w", err) + } + if err := descriptor.SetDACL(acl, true, false); err != nil { + return nil, nil, fmt.Errorf("attach current-user DACL: %w", err) + } + if err := descriptor.SetControl(windows.SE_DACL_PROTECTED, windows.SE_DACL_PROTECTED); err != nil { + return nil, nil, fmt.Errorf("protect descriptor DACL: %w", err) + } + return descriptor, acl, nil +} diff --git a/cli/internal/download/private_windows_test.go b/cli/internal/download/private_windows_test.go index 70d9a9598..b3efd1001 100644 --- a/cli/internal/download/private_windows_test.go +++ b/cli/internal/download/private_windows_test.go @@ -12,6 +12,25 @@ import ( ) func TestAtomicPrivateFileCreation(t *testing.T) { + user, err := windows.GetCurrentProcessToken().GetTokenUser() + if err != nil { + t.Fatal(err) + } + descriptor, acl, err := currentUserDescriptor(user) + if err != nil { + t.Fatal(err) + } + if !descriptor.IsValid() { + t.Fatal("creation descriptor is invalid") + } + control, _, err := descriptor.Control() + if err != nil || control&windows.SE_SELF_RELATIVE != 0 || control&windows.SE_DACL_PRESENT == 0 || + control&windows.SE_DACL_PROTECTED == 0 || control&windows.SE_SACL_PRESENT != 0 { + t.Fatal("creation descriptor has unexpected control flags", control, err) + } + assertCurrentUserDACL(t, descriptor, user) + runtime.KeepAlive(acl) + directory := t.TempDir() root, err := os.OpenRoot(directory) if err != nil { @@ -35,26 +54,33 @@ func assertPrivateStage(t *testing.T, file *os.File) { t.Fatal(err) } control, _, err := sd.Control() - if err != nil || control&windows.SE_DACL_PROTECTED == 0 { + if err != nil || control&windows.SE_DACL_PRESENT == 0 || control&windows.SE_DACL_PROTECTED == 0 || control&windows.SE_SACL_PRESENT != 0 { t.Fatal("stage DACL inherits access", control, err) } - acl, _, err := sd.DACL() + user, err := windows.GetCurrentProcessToken().GetTokenUser() + if err != nil { + t.Fatal(err) + } + assertCurrentUserDACL(t, sd, user) + runtime.KeepAlive(sd) + runtime.KeepAlive(user) +} + +func assertCurrentUserDACL(t *testing.T, descriptor *windows.SECURITY_DESCRIPTOR, user *windows.Tokenuser) { + t.Helper() + acl, _, err := descriptor.DACL() if err != nil || acl == nil || acl.AceCount != 1 { - t.Fatal("stage does not have exactly one owner ACE", acl, err) + t.Fatal("descriptor does not have exactly one current-user ACE", acl, err) } var ace *windows.ACCESS_ALLOWED_ACE if err := windows.GetAce(acl, 0, &ace); err != nil { t.Fatal(err) } - user, err := windows.GetCurrentProcessToken().GetTokenUser() - if err != nil { - t.Fatal(err) - } aceSID := (*windows.SID)(unsafe.Pointer(&ace.SidStart)) if ace.Header.AceType != windows.ACCESS_ALLOWED_ACE_TYPE || ace.Header.AceFlags != 0 || ace.Mask != fileAllAccess || !aceSID.Equals(user.User.Sid) { - t.Fatal("stage DACL is not current-user full access", ace.Header, ace.Mask, aceSID) + t.Fatal("DACL is not current-user full access", ace.Header, ace.Mask, aceSID) } - runtime.KeepAlive(sd) + runtime.KeepAlive(descriptor) runtime.KeepAlive(user) } diff --git a/cli/tests/integration/test_tui_http.py b/cli/tests/integration/test_tui_http.py index 5c96ab159..c6d39f12e 100644 --- a/cli/tests/integration/test_tui_http.py +++ b/cli/tests/integration/test_tui_http.py @@ -557,6 +557,20 @@ def profile(_): terminal.quit() +def test_terminal_observer_applies_character_deletion_and_reports_actual_cursor(): + terminal = object.__new__(Terminal) + terminal._screen_setup(80, 24) + replies = [] + terminal._write = replies.append + terminal._render("Filter fetched page only: nothing-matches\rlocal filter:\x1b[12P") + assert terminal.screen.splitlines()[0] == "local filter: nothing-matches" + terminal._render("\x1b[7;9H\x1b[6n") + assert replies == [b"\x1b[7;9R"] + terminal._render("\x1b[1;79Hxy\x1b[6n\r\x1b[79G\x1b[999P") + assert replies[-1] == b"\x1b[1;80R" + assert terminal.screen.splitlines()[0] == "local filter: nothing-matches" + + def test_explicit_pagination_refresh_and_local_filter_do_not_walk_pages(): reads = Reads() @@ -578,7 +592,7 @@ def ready(query): terminal.pump(0.2) assert len(requests) == 3 terminal.send("/nothing-matches\r") - terminal.pump(0.2) + terminal.wait("local filter: nothing-matches") assert "Ready terminal task" not in terminal.screen assert "local" in terminal.screen.lower() assert len(requests) == 3 diff --git a/cli/tests/integration/tui_terminal_support.py b/cli/tests/integration/tui_terminal_support.py index dbe2cd6b8..53eebf2a5 100644 --- a/cli/tests/integration/tui_terminal_support.py +++ b/cli/tests/integration/tui_terminal_support.py @@ -136,6 +136,10 @@ def _render(self, text): self.x = max(0, self.x - n) elif command == "G": self.x = min(self.columns - 1, n - 1) + elif command == "P": + deleted = min(n, self.columns - self.x) + row = self.cells[self.y] + row[self.x :] = row[self.x + deleted :] + [" "] * deleted elif command == "J": if values[0] in {2, 3}: self.cells = [ @@ -155,7 +159,8 @@ def _render(self, text): ) self.cells[self.y][start:end] = [" "] * (end - start) elif command == "n" and params == "6": - self._write(b"\x1b[1;1R") + column = min(self.x + 1, self.columns) + self._write(f"\x1b[{self.y + 1};{column}R".encode()) continue if self.pending.startswith("\x1b]"): match = re.match(r"\x1b\].*?(?:\x07|\x1b\\)", self.pending, re.S) From 13b886bb2289c480fea231f20fdec28d4d67ef2b Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Mon, 26 Oct 2026 14:35:10 +0100 Subject: [PATCH 9/9] fix(cli): normalize guide selection after context reload --- cli/internal/tui/guide_findings_test.go | 47 +++++++++++++++++++++++++ cli/internal/tui/requests.go | 1 + 2 files changed, 48 insertions(+) diff --git a/cli/internal/tui/guide_findings_test.go b/cli/internal/tui/guide_findings_test.go index 0164eb966..c6256194c 100644 --- a/cli/internal/tui/guide_findings_test.go +++ b/cli/internal/tui/guide_findings_test.go @@ -151,6 +151,53 @@ func TestGuideRefreshAbortsDownloadAndReloadsFreshContext(t *testing.T) { } } +func TestGuideReloadClampsSelectionBeforeConfirmation(t *testing.T) { + for _, reload := range []string{"refresh", "tab"} { + for _, empty := range []bool{false, true} { + name := reload + "/shorter" + if empty { + name = reload + "/empty" + } + t.Run(name, func(t *testing.T) { + documents := []api.TaskGuideDocument{ + {DocumentID: actorID, Label: "Remaining", MediaType: "text/markdown"}, + {DocumentID: policyID, Label: "Removed", MediaType: "text/markdown"}, + } + f := &fakeClient{context: func(context.Context, string) (api.Result[api.WorkContext], error) { + return api.Result[api.WorkContext]{Value: guideContext(documents...)}, nil + }} + m := openMineGuide(t, f) + press(t, m, "down") + documents = documents[:1] + if empty { + documents = nil + } + if reload == "refresh" { + press(t, m, "r") + } else { + for range 4 { + press(t, m, "tab") + } + } + before := len(f.calls) + press(t, m, "d") + if empty { + if m.guideMode != guideBrowse || len(f.calls) != before { + t.Fatal("empty guide opened a download", m.guideMode, f.calls) + } + return + } + press(t, m, t.TempDir()) + press(t, m, "enter") + confirmation := m.detailText() + if !strings.Contains(confirmation, "Document: "+actorID) || strings.Contains(confirmation, "Document: "+policyID) || len(f.calls) != before { + t.Fatal("confirmation did not bind the refreshed document without I/O", confirmation, f.calls) + } + }) + } + } +} + func TestFindingsNavigationPaginationAndMisbinding(t *testing.T) { cursor := "next-page" requests := 0 diff --git a/cli/internal/tui/requests.go b/cli/internal/tui/requests.go index 0af460616..ec0f5bcde 100644 --- a/cli/internal/tui/requests.go +++ b/cli/internal/tui/requests.go @@ -343,6 +343,7 @@ func (m *Model) receive(msg response) tea.Cmd { return bad() } m.context = &value + m.guideSelected = max(0, min(m.guideSelected, len(value.GuideDocuments)-1)) case api.SubmissionRequirements: if r.kind != intakeRead || !sameID(value.TaskID, r.task) || !sameID(value.ProjectID, r.project) || (m.context != nil && value.GuideVersion != m.context.Guide.Version) {