From 23e96715daf0e8dd3b2eaefdc0aed9199d72271e Mon Sep 17 00:00:00 2001 From: rabesss <26330469+rabesss@users.noreply.github.com> Date: Sat, 31 Oct 2026 18:50:18 +0530 Subject: [PATCH] fix(quiz): prevent fresh attempts during uncertain-start recovery Require continuation after an unverified start even when the attempt ID was never received. Preserve the checkpoint when no continuation exists and explain recovery without inviting another graded attempt. Add complete learner CLI workflow and partial-save recovery coverage, plus an agent guide. Include CodeRabbit's account-check ordering correction in verification, covered by a regression test. --- .secrets.baseline | 2 +- README.md | 8 +- docs/agent-quizzes.md | 167 +++++++++++++++++++++++++ lighthouse_cli/quiz_learner_session.py | 17 ++- tests/test_quiz_agent_workflow.py | 135 ++++++++++++++++++++ tests/test_quiz_learner_session.py | 64 +++++++++- 6 files changed, 380 insertions(+), 13 deletions(-) create mode 100644 docs/agent-quizzes.md create mode 100644 tests/test_quiz_agent_workflow.py diff --git a/.secrets.baseline b/.secrets.baseline index 00bf6ec..2f90ccf 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -149,7 +149,7 @@ "filename": "README.md", "hashed_secret": "45d676e7c6ab44cf4b8fa366ef2d8fccd3e6d6e6", "is_verified": false, - "line_number": 367, + "line_number": 373, "is_secret": false } ], diff --git a/README.md b/README.md index 182cae0..09556c2 100644 --- a/README.md +++ b/README.md @@ -241,6 +241,9 @@ lighthouse instructor preview reconcile COURSE_ID QUIZ_ID --confirm-no-remote-at **Learner quiz attempts** (`student attempt`) take your own graded attempt of a quiz, timed or not, a page at a time: +For an agent's discovery, opening-time, answer and recovery loop, see +[Taking a teacher's quiz with an agent](docs/agent-quizzes.md). + ```bash # Continue the attempt in progress, or start a new one (uses one of your attempts): lighthouse student attempt start COURSE_ID QUIZ_ID --yes --json @@ -277,7 +280,10 @@ reported. If one cannot be verified, `page` settles an answer save, and `start` continues the attempt on the page Brightspace has. Once `start` has opened an attempt, until it is submitted `start` continues only that attempt and never begins a new one: if it ended elsewhere, `verify` checks whether it was submitted, and -`forget` drops the CLI's record of it. `submit` is refused +`forget` drops the CLI's record of it. An unverified start also only continues: +if its attempt ID was never received and Brightspace offers no Continue, +inspect Brightspace before deliberately using `forget` to allow a fresh attempt. +`verify` cannot check a receipt without that ID. `submit` is refused before anything is saved unless the cursor is on the last page; unless `--allow-unanswered`, it then stops while any question of the quiz is unanswered, listing them under `unanswered` (only the page's own answers were diff --git a/docs/agent-quizzes.md b/docs/agent-quizzes.md new file mode 100644 index 0000000..f57f614 --- /dev/null +++ b/docs/agent-quizzes.md @@ -0,0 +1,167 @@ +# Taking a teacher's quiz with an agent + +The CLI supports this sequence: discover a quiz, wait until it opens, start or +continue the signed-in learner's attempt, read its questions, save answers, +move through its pages, submit, and verify the receipt. Learner attempts are +real, graded attempts. Choosing answers and deciding when to run commands +belong to the calling agent. + +## Discover and wait + +Run the installed `lighthouse` command. Put `--json` on each leaf command; +it is not a global option. + +```bash +lighthouse auth status --json +lighthouse courses --json +lighthouse quizzes COURSE_ID --json +lighthouse quiz COURSE_ID QUIZ_ID --json +``` + +Check exit codes and parse stdout as one JSON document. Diagnostics go to +stderr. Use course and quiz IDs from these results, rather than guessed names +or question IDs. Quiz listings contain settings and dates, not attempt +questions. Instructors' `quiz-questions` is a different permission-gated +surface, not a way to read a learner's attempt. + +The calling agent can repeat the quiz listing to discover newly visible +teacher quizzes and wait for their start dates. There is no built-in watcher +or scheduler. Recheck authentication when the quiz opens, especially after a +long wait. A working session or configured +[unattended sign-in](auth-unattended.md) is needed; attempt operations themselves +never sign in automatically or replay writes. + +Before starting, check the quiz's dates, attempt allowance and time limit. +An open attempt may resume instead of creating a new one: + +```bash +lighthouse student attempt status COURSE_ID QUIZ_ID --json +lighthouse student attempt start COURSE_ID QUIZ_ID --yes --json +``` + +`--yes` allows the remote change without a terminal prompt. If the quiz has +not opened, a refusal may include `opens_at` (UTC) and +`seconds_until_open`. Wait until then and recheck. Other refusals must be +resolved, not treated as a countdown. `start --dry-run` only describes the +command: it does not contact Brightspace, check availability, or inspect the +questions. + +A successful start returns `attempt_id`, `page`, `questions`, +`has_next_control`, `has_previous_control`, `timer`, `quiz` and +`resumed`. Its clock keeps running between commands. + +## Read, answer and move + +`start` already returns the first current page; `page` reads it again when +needed: + +```bash +lighthouse student attempt page COURSE_ID QUIZ_ID --json +lighthouse student attempt images COURSE_ID QUIZ_ID --json +``` + +Each question has `question_id`, `number`, `text`, `kind`, +`supported`, `choices`, `selected_choice_ids`, `blanks`, `images` +and `saved`. Choices have `choice_id` and text. Use these IDs exactly as +returned, even when the teacher randomizes questions or choices. + +Equations are returned as LaTeX. Image markers refer to the files listed by +`images`; inspect those files before answering image-dependent questions. +Check each image entry for an `error` even if the command exits successfully. + +The answer object maps question IDs to the following values: + +| Question kind | Answer value | +| --- | --- | +| `single-choice` (including rendered true/false radios) | One returned choice ID as a string | +| `multi-select` | A list of returned choice IDs | +| `fill-blank` | A list of texts in the order of the returned blanks | + +For example, using synthetic IDs: + +```bash +lighthouse student attempt answer COURSE_ID QUIZ_ID \ + --answers '{"101":"o2","102":["o11","o13"],"103":["four","six"]}' --yes --json +``` + +Questions omitted from the object keep their current answers. Each changed +answer is saved separately and read back before success is reported. Repeating +an answer that is already saved does not send another save. + +After reading and answering a page, use `has_next_control` to decide: + +- If true, use `next --yes --json`, or use `answer --next --yes --json` + to save and advance together. +- If false, save with `answer` without `--next`, then submit. + `answer --next` on the last page refuses before saving any of that batch. + +`previous --yes --json` works only where the quiz permits moving back. +For a forward-only quiz, finish each page before leaving it. Commands return +the verified current page after a save or page change. + +Only the three question kinds above are supported. An unsupported question +blocks every write on its page, including navigation and submission, because +Brightspace posts the whole page form. Do not try to skip it with +`--allow-unanswered`. Complete that attempt through Brightspace instead. +Password-protected quizzes and quizzes requiring a browser step such as +LockDown Browser are also refused. + +## Submit and verify + +On the last page, after checking saved answers and the time remaining: + +```bash +lighthouse student attempt submit COURSE_ID QUIZ_ID --yes --json +lighthouse student attempt verify COURSE_ID QUIZ_ID --json +``` + +Success includes `submitted: true`, the attempt identity and attempt number. +Scores are returned only when Brightspace releases them. A successful HTTP +request alone does not establish submission; the CLI checks both the +submissions list and receipt. + +By default, submission stops if any question in the quiz is unanswered, and +returns an `unanswered` list with page/question IDs and question numbers. +`--allow-unanswered` is an explicit decision to leave answers blank, not a +normal recovery option. After verified submission, stop: another `start` +can consume another allowed attempt. + +Timed replies include `timer.seconds_left`, `timer.ends_at` (UTC), +`timer.limit_seconds` and `timer.auto_submit`, or `timer: null` for an +unenforced limit. Allow time for requests, reasoning and final verification; +submit before the countdown reaches zero. With auto-submit enabled, the CLI +sends no further attempt changes after expiry. Call `verify` to check whether +Brightspace has submitted it; if processing is still underway, wait and check again. + +## Recover without replaying a write + +After an error or an interrupted process, read +`student attempt status COURSE_ID QUIZ_ID --json`. It reports the local +checkpoint, not the current remote state. It contains no answer history. + +| Local state | Next action | +| --- | --- | +| `uncertain`, operation `answer` | Run `page` to settle the save and read stored answers; then supply only the intended changes. Some earlier answers in the batch may already be saved. | +| `uncertain`, operation `next` or `previous` | Run `start --yes` to resume where Brightspace has the same attempt. Do not repeat the page-change command. | +| `uncertain`, operation `start`, known `attempt_id` | Run `start --yes` to continue only. If the attempt ended elsewhere, use `verify`. | +| `uncertain`, operation `start`, `attempt_id: null` | Run `start --yes` to continue only. If no Continue exists, inspect Brightspace before deliberate `forget`; `verify` cannot locate a receipt without the attempt ID. | +| `uncertain`, operation `submit` | Run `verify` first. If it explicitly reports still in progress, `start --yes` can continue that attempt. Do not blindly submit again. | +| `submitted` | Stop; use `verify` if a fresh receipt check is needed. | +| `active` | Use `page` to read the saved answers before continuing. | + +For session expiry, authenticate separately with `auth login` (or the +configured unattended read flow), then follow the checkpoint's recovery step. +When an auto-submit timer has expired, use `verify`; it can report that +Brightspace is still processing the attempt. Stop on account mismatches or +an unreadable checkpoint and inspect the remote attempt before recovery. + +`forget --yes` removes the local checkpoint and can allow a later +`start` to create another graded attempt. Use it only after inspecting +Brightspace and deliberately deciding that a new attempt is wanted. + +Errors currently include a human-readable `error`; some also include +structured countdown or unanswered-question data. There is no versioned error +code or recovery-action schema. Use the checkpoint's state/operation plus the +error details, and stop on an unrecognized failure. Serialize commands for the +same course/quiz: the CLI locks each operation, but does not coordinate the +intent of multiple independent agents across commands. diff --git a/lighthouse_cli/quiz_learner_session.py b/lighthouse_cli/quiz_learner_session.py index 3cf6684..87223c4 100644 --- a/lighthouse_cli/quiz_learner_session.py +++ b/lighthouse_cli/quiz_learner_session.py @@ -87,6 +87,9 @@ class LearnerWorkflowError(ValueError): "Sign in as that account and run attempt start to settle it.") _NOT_CONTINUABLE = ("The attempt the CLI was taking is no longer in progress, so no new attempt was started. " "Run attempt verify to check whether it was submitted, or attempt forget to drop the CLI's record of it.") +_START_NO_ID = ("The last quiz start could not be verified, and its attempt ID is unknown. " + "If Brightspace offers Continue, run attempt start. Otherwise check the quiz in Brightspace " + "before using attempt forget to allow a new graded attempt.") _OTHER_ATTEMPT = ("Brightspace has a different attempt in progress than the one the CLI was taking, so the CLI did " "not take it over. Run attempt verify to check whether the CLI's attempt was submitted, " "or attempt forget to drop the CLI's record of it.") @@ -419,6 +422,10 @@ def start(self) -> dict[str, Any]: # may be continued: if it ended elsewhere, a new start would use # another graded attempt. kept = own if own is not None and own["status"] != "submitted" and own["attempt_id"] is not None else None + # A lost start reply is not proof that no attempt was created. + # Keep identity matching separate: an unknown ID cannot be compared. + recovering_start = own is not None and own["status"] == "uncertain" and own["operation"] == "start" + continue_only = kept is not None or recovering_start if kept is not None and _time_up(kept): raise LearnerWorkflowError(_TIME_UP) info = quiz_info(client.get_quiz_detail(self.course_id, self.quiz_id)) @@ -426,8 +433,8 @@ def start(self) -> dict[str, Any]: # Refused whether or not the CLI kept that attempt (after forget, # or on another computer, it did not), before anything is written. refuse_while_processing(summary) - if kept is not None and not summary.can_continue: - raise LearnerWorkflowError(_NOT_CONTINUABLE) + if continue_only and not summary.can_continue: + raise LearnerWorkflowError(_START_NO_ID if kept is None else _NOT_CONTINUABLE) # The intent keeps the CLI's attempt, its limit and its browser # tab, so a start that fails midway still names them for the next # start and for verify. @@ -450,7 +457,7 @@ def seal(attempt_id: int, page: int, tab_id: str) -> None: try: started = start_learner(client, course_id=self.course_id, quiz_id=self.quiz_id, - continue_only=kept is not None, known_tab_id=kept.get("tab_id", "") if kept else "", + continue_only=continue_only, known_tab_id=kept.get("tab_id", "") if kept else "", on_identity=seal, summary=summary) except LearnerStartUnknownError as exc: # The cursor stays uncertain, naming the attempt. @@ -583,10 +590,10 @@ def verify(self) -> dict[str, Any]: state = self._load() if state is None: raise LearnerWorkflowError(_NOT_OPEN) - if state["attempt_id"] is None: - raise LearnerWorkflowError(_REOPEN) if self._actor(client) != state["actor_id"]: raise LearnerWorkflowError(_OTHER_ACCOUNT) + if state["attempt_id"] is None: + raise LearnerWorkflowError(_START_NO_ID) try: receipt = verify_learner_submission(client, course_id=self.course_id, quiz_id=self.quiz_id, attempt_id=state["attempt_id"], frame_page=state.get("frame_page")) diff --git a/tests/test_quiz_agent_workflow.py b/tests/test_quiz_agent_workflow.py new file mode 100644 index 0000000..b17dc7e --- /dev/null +++ b/tests/test_quiz_agent_workflow.py @@ -0,0 +1,135 @@ +"""Exercise an agent's separate CLI calls with synthetic HTTP responses.""" + +from __future__ import annotations + +import json +from unittest.mock import Mock, patch + +from click.testing import CliRunner + +from lighthouse_cli.cli import cli +from tests.test_quiz_learner import ( + ALL, + ANSWERED, + FINISHED, + RECEIPT, + body, + confirmation, + listing, + posted, + questions, + reply, + saved_reply, + start_client, + summary, +) +from tests.test_quiz_learner_session import QUIZ + +SESSION = "lighthouse_cli.quiz_learner_session" + + +def invoke(operation: str, *args: str, exit_code: int = 0): + result = CliRunner().invoke(cli, ["student", "attempt", operation, "10", "20", *args, "--json"]) + assert result.exit_code == exit_code, result.stdout + result.stderr + assert "SESSION_SENTINEL" not in result.stdout + result.stderr + return json.loads(result.stdout) + + +def client_for_attempt(): + client = start_client(summary()) + client.get_json = Mock(return_value={"Identifier": 7}) + client.get_quiz_detail = Mock(return_value=QUIZ) + return client + + +def test_agent_can_complete_and_verify_a_quiz_across_cli_calls(): + client = client_for_attempt() + with patch(f"{SESSION}.LighthouseClient", return_value=client): + started = invoke("start", "--yes") + assert started["resumed"] is False + assert {q["kind"] for q in started["questions"]} == { + "single-choice", + "multi-select", + "fill-blank", + } + assert started["timer"] is None + client._request.assert_called_once() + + client.get_raw.side_effect = [(body(questions()), {})] + page = invoke("page") + assert page["attempt_id"] == started["attempt_id"] + assert page["has_next_control"] is False + client._request.reset_mock() + client._request.side_effect = [ + posted(saved_reply("101")), + posted(saved_reply("102")), + posted(saved_reply("103")), + ] + client.get_raw.side_effect = [ + (body(questions()), {}), + (body(questions(sc=("o2",), saved=("True", "False", "False"))), {}), + (body(questions(sc=("o2",), ms=("o11", "o13"), saved=("True", "True", "False"))), {}), + (body(ANSWERED), {}), + ] + answered = invoke("answer", "--answers", json.dumps(ALL), "--yes") + assert all(q["saved"] is True for q in answered["questions"]) + assert client._request.call_count == 3 + + client._request.reset_mock() + client.get_raw.side_effect = [(body(ANSWERED), {})] + repeated = invoke("answer", "--answers", json.dumps(ALL), "--yes") + assert repeated["questions"] == answered["questions"] + client._request.assert_not_called() + + client.get_raw.side_effect = [ + (body(ANSWERED), {}), + (confirmation(), {}), + (listing(), {}), + (RECEIPT, {}), + ] + client._request.side_effect = [posted(FINISHED), posted(reply())] + submitted = invoke("submit", "--yes") + assert submitted["submitted"] is True + assert submitted["attempt_id"] == started["attempt_id"] + assert client._request.call_count == 2 + + client._request.reset_mock() + client.get_raw.side_effect = [(listing(), {}), (RECEIPT, {})] + assert invoke("verify") == submitted + client._request.assert_not_called() + assert invoke("status")["status"] == "submitted" + + +def test_agent_settles_a_partial_save_before_resending_remaining_answers(): + client = client_for_attempt() + partial = body(questions(sc=("o2",), saved=("True", "False", "False"))) + answers = json.dumps({"101": "o2", "102": ["o11"]}) + first, second = posted(saved_reply("101")), posted(saved_reply("102"), status=500) + with patch(f"{SESSION}.LighthouseClient", return_value=client): + invoke("start", "--yes") + client._request.reset_mock() + client._request.side_effect = [first, second] + client.get_raw.side_effect = [(body(questions()), {}), (partial, {})] + assert invoke("answer", "--answers", answers, "--yes", exit_code=1)["error"] + assert client._request.call_count == 2 + first.close.assert_called_once() + second.close.assert_called_once() + cursor = invoke("status") + assert (cursor["status"], cursor["operation"]) == ("uncertain", "answer") + + client._request.reset_mock() + client.get_raw.side_effect = [(partial, {})] + reread = invoke("page") + assert reread["questions"][0]["selected_choice_ids"] == ["o2"] + assert reread["questions"][1]["selected_choice_ids"] == [] + assert invoke("status")["status"] == "active" + client._request.assert_not_called() + + client.get_raw.side_effect = [ + (partial, {}), + (body(questions(sc=("o2",), ms=("o11",), saved=("True", "True", "False"))), {}), + ] + client._request.side_effect = [posted(saved_reply("102"))] + saved = invoke("answer", "--answers", answers, "--yes") + assert [q["saved"] for q in saved["questions"][:2]] == [True, True] + client._request.assert_called_once() diff --git a/tests/test_quiz_learner_session.py b/tests/test_quiz_learner_session.py index 7c08082..8eaab24 100644 --- a/tests/test_quiz_learner_session.py +++ b/tests/test_quiz_learner_session.py @@ -262,18 +262,67 @@ def unknown(client, *, on_identity, **kwargs): assert workflow.status() == {**ACTIVE, "status": "submitted"} -def test_an_unknown_start_without_an_identity_is_kept(remote): +def test_an_unknown_start_without_an_identity_never_starts_another_attempt(remote): workflow = LearnerWorkflow(10, 20) with patch(f"{SESSION}.start_learner", side_effect=LearnerStartUnknownError()): with pytest.raises(LearnerStartUnknownError): workflow.start() status = workflow.status() assert (status["status"], status["attempt_id"], status["page"]) == ("uncertain", None, None) - # No attempt is known to exist, so starting again may start one. + # The start may have reached Brightspace and even ended before recovery. + # No Continue is not evidence that no graded attempt was created. + before = workflow.path.read_bytes() + with patch(f"{SESSION}.start_learner") as started: + for _ in range(2): + with pytest.raises(LearnerWorkflowError, match="attempt ID is unknown"): + workflow.start() + started.assert_not_called() + assert workflow.path.read_bytes() == before + + +def test_an_unknown_start_without_an_identity_can_only_continue(remote): + workflow = LearnerWorkflow(10, 20) + with patch(f"{SESSION}.start_learner", side_effect=LearnerStartUnknownError()): + with pytest.raises(LearnerStartUnknownError): + workflow.start() + + def continued(client, *, on_identity, **kwargs): + on_identity(30, 1, TAB) + assert saved(workflow)["attempt_id"] == 30 + return opened() + + with patch(f"{SESSION}.read_learner_summary", return_value=summary_state(resumed=True)), \ + patch(f"{SESSION}.start_learner", side_effect=continued) as started: + result = workflow.start() + assert started.call_args.kwargs["continue_only"] is True + assert started.call_args.kwargs["known_tab_id"] == "" + assert result["resumed"] is True + assert workflow.status() == ACTIVE + + +def test_verifying_an_unknown_identity_preserves_the_cursor_and_explains_recovery(remote): + workflow = LearnerWorkflow(10, 20) + with patch(f"{SESSION}.start_learner", side_effect=LearnerStartUnknownError()): + with pytest.raises(LearnerStartUnknownError): + workflow.start() + before = workflow.path.read_bytes() + with patch(f"{SESSION}.verify_learner_submission") as verify: + with pytest.raises(LearnerWorkflowError, match="attempt ID is unknown"): + workflow.verify() + verify.assert_not_called() + assert workflow.path.read_bytes() == before + + +def test_forgetting_an_unknown_start_explicitly_allows_a_fresh_attempt(remote): + workflow = LearnerWorkflow(10, 20) + with patch(f"{SESSION}.start_learner", side_effect=LearnerStartUnknownError()): + with pytest.raises(LearnerStartUnknownError): + workflow.start() + assert workflow.forget()["forgotten"] is True with patch(f"{SESSION}.start_learner", return_value=opened()) as started: - workflow.start() + result = workflow.start() + assert result["resumed"] is False assert started.call_args.kwargs["continue_only"] is False - assert started.call_args.kwargs["known_tab_id"] == "" @pytest.mark.parametrize("change", [ @@ -648,8 +697,11 @@ def test_verify_reads_only_this_accounts_started_attempt(remote): with patch(f"{SESSION}.start_learner", side_effect=LearnerStartUnknownError()): with pytest.raises(LearnerStartUnknownError): workflow.start() - assert_refused(workflow, "Run attempt start", ("verify",)) - start(workflow) + state["actor"] = 8 + assert_refused(workflow, "different signed-in account", ("verify",)) + state["actor"] = 7 + assert_refused(workflow, "attempt ID is unknown", ("verify",)) + start(workflow, resumed=True) state["actor"] = 8 assert_refused(workflow, "different signed-in account", ("verify",)) state["actor"] = 7