fix(control): close the captcha governance loop - #65
Open
1012839419a-alt wants to merge 1 commit into
Open
Conversation
PR 2233admin#63 added error_type='captcha_challenge' classification but nothing consumed it: is_retryable() treated it as permanent (unknown types default to permanent), so a Doubao captcha wall permanently failed the task with no recovery path, while the control layer's pause/review machinery (actuator, paused_until, review_required) had no scheduler consumer at all. - error_taxonomy: add CAPTCHA_CHALLENGE + is_captcha() — a third category (needs human), neither retryable nor permanent - error_kinds: map captcha_challenge -> ErrorKind.CAPTCHA for the control vocabulary - actuator: add pause_source_for_captcha() — pause + require_review in one call, keeping the actuator the only mutator of DataSource - pipeline: on captcha_challenge collect failure, pause the source (TTL from control_pause_ttl_seconds) + flag review instead of failing permanently; best-effort so actuator/DB errors never mask the original collect error - scheduler: _get_enabled_schedules now also skips sources with review_required=True, so a paused/flagged source is never dispatched until a human clears it (control loop writes the state, scheduler honors it) Tests: taxonomy (is_captcha/retryability), error_kinds mapping, actuator pause+review (incl. idempotent re-pause), pipeline wiring (captcha pauses, ordinary failures don't), scheduler review/disabled gating.
|
✅ Health of changed files: 7.2 → 7.3 (+0.1) 📋 At a glance Files & modules (2)
✅ Health gate: passed 📌 Before you merge
🔎 More signals (2)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (5 with dependents)"]
f_backend_control_actuator_py["backend/control/actuator.py 🔥"]:::changed
f_backend_control_error_kinds_py["backend/control/error_kinds.py 🔥"]:::changed
f_backend_pipeline_pipeline_py["backend/pipeline/pipeline.py 🔥"]:::changed
f_backend_pipeline_error_taxonomy_py["backend/pipeline/error_taxonomy.py"]:::changed
f_backend_scheduler_py["backend/scheduler.py"]:::changed
end
f_backend_control_cycle_py["backend/control/cycle.py"]
f_backend_control_actuator_py --> f_backend_control_cycle_py
f_backend_control_recorder_py["backend/control/recorder.py"]
f_backend_control_error_kinds_py --> f_backend_control_recorder_py
f_backend_api_v1___init___py[".../v1/__init__.py"]
f_backend_pipeline_pipeline_py --> f_backend_api_v1___init___py
f_backend_channels_rss_channel_py["backend/channels/rss_channel.py"]
f_backend_pipeline_pipeline_py --> f_backend_channels_rss_channel_py
f_backend_channels_skill_channel_py["backend/channels/skill_channel.py"]
f_backend_pipeline_pipeline_py --> f_backend_channels_skill_channel_py
f_backend_config_py["backend/config.py"]
f_backend_pipeline_pipeline_py --> f_backend_config_py
f_backend_channels_api_channel_py["backend/channels/api_channel.py"]
f_backend_pipeline_error_taxonomy_py --> f_backend_channels_api_channel_py
f_backend_pipeline_error_taxonomy_py --> f_backend_channels_rss_channel_py
f_backend_channels_web_scraper_channel_py["backend/channels/web_scraper_channel.py"]
f_backend_pipeline_error_taxonomy_py --> f_backend_channels_web_scraper_channel_py
f_backend_pipeline_odp_client_py["backend/pipeline/odp_client.py"]
f_backend_pipeline_error_taxonomy_py --> f_backend_pipeline_odp_client_py
f_backend_main_py["backend/main.py"]
f_backend_scheduler_py --> f_backend_main_py
more(["+2 more dependents"])
PR --> more
t_tests_unit_channels_test_cli_channel_execution_py(["✅ .../channels/test_cli_channel_execution.py"]):::guard
t_tests_unit_channels_test_cli_channel_execution_py -.-> f_backend_control_error_kinds_py
t_tests_unit_pipeline_test_legacy_db_sink_py(["✅ .../pipeline/test_legacy_db_sink.py"]):::guard
t_tests_unit_pipeline_test_legacy_db_sink_py -.-> f_backend_pipeline_pipeline_py
t_tests_unit_channels_test_cli_channel_allowlist_py(["✅ .../channels/test_cli_channel_allowlist.py"]):::guard
t_tests_unit_channels_test_cli_channel_allowlist_py -.-> f_backend_pipeline_error_taxonomy_py
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (12 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (5)
2 more
👀 Suggested reviewers @2233admin 📊 See the full report for this PR |
📝 WalkthroughSummary by CodeRabbit
WalkthroughCaptcha failures now receive a dedicated classification. The pipeline pauses affected sources, marks them for review, reports failed results, and excludes reviewed sources from enabled scheduling. Unit and database-backed tests cover the new behavior. ChangesCaptcha governance flow
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant CollectionPipeline
participant CaptchaActuator
participant SourceDatabase
participant Scheduler
CollectionPipeline->>CaptchaActuator: pause source for configured TTL
CaptchaActuator->>SourceDatabase: disable source and set review_required
SourceDatabase-->>CollectionPipeline: return pause details
CollectionPipeline-->>CollectionPipeline: return failed result with captcha_paused
Scheduler->>SourceDatabase: query enabled schedules
SourceDatabase-->>Scheduler: exclude review-required sources
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/pipeline/pipeline.py`:
- Around line 270-299: In the captcha-handling flow, introduce a local
captcha_paused flag initialized to False and set it to True only after
pause_source_for_captcha and the subsequent session.commit() complete
successfully. Return this flag in PipelineResult.metadata so missing sources or
any pause/commit exception report captcha_paused as False.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c518f9f4-7b2d-4c5b-a72e-24df7d0ba48f
📒 Files selected for processing (10)
backend/control/actuator.pybackend/control/error_kinds.pybackend/pipeline/error_taxonomy.pybackend/pipeline/pipeline.pybackend/scheduler.pytests/unit/control/test_actuator.pytests/unit/control/test_error_kinds.pytests/unit/pipeline/test_error_taxonomy.pytests/unit/pipeline/test_pipeline_errors.pytests/unit/test_scheduler.py
Comment on lines
+270
to
+299
| ttl = get_settings().control_pause_ttl_seconds | ||
| async with AsyncSessionLocal() as session: | ||
| src = await session.get(DataSource, source.id) | ||
| if src is not None: | ||
| await pause_source_for_captcha( | ||
| session, | ||
| source=src, | ||
| now=datetime.now(timezone.utc), | ||
| ttl_seconds=ttl, | ||
| ) | ||
| await session.commit() | ||
| logger.warning( | ||
| "[task:%s] captcha wall | paused source=%s (ttl=%ss, review_required)", | ||
| task_id, source.id, ttl, | ||
| ) | ||
| if run_id: | ||
| await events.emit( | ||
| run_id, "collect", | ||
| "验证码拦截：数据源已暂停，等待人工处理", | ||
| level="warning", | ||
| detail={"captcha_paused": True, "pause_ttl_seconds": ttl}, | ||
| ) | ||
| except Exception: | ||
| logger.exception("[task:%s] failed to pause source on captcha", task_id) | ||
| return PipelineResult( | ||
| success=False, | ||
| source_id=source.id, | ||
| error=channel_result.error, | ||
| metadata={"captcha_paused": True}, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Set captcha_paused only after the pause commits.
The result always returns {"captcha_paused": True}. This remains true if session.get() returns no source, pause_source_for_captcha() fails, or session.commit() fails. The caller can then report a pause that did not persist, while the scheduler can continue to dispatch the source.
Track a local captcha_paused = False. Set it to True only after await session.commit() succeeds.
Proposed fix
+ captcha_paused = False
try:
from backend.config import get_settings
from backend.control.actuator import pause_source_for_captcha
from backend.database import AsyncSessionLocal
ttl = get_settings().control_pause_ttl_seconds
async with AsyncSessionLocal() as session:
src = await session.get(DataSource, source.id)
if src is not None:
await pause_source_for_captcha(
session,
source=src,
now=datetime.now(timezone.utc),
ttl_seconds=ttl,
)
await session.commit()
+ captcha_paused = True
logger.warning(
"[task:%s] captcha wall | paused source=%s (ttl=%ss, review_required)",
task_id, source.id, ttl,
)
@@
return PipelineResult(
success=False,
source_id=source.id,
error=channel_result.error,
- metadata={"captcha_paused": True},
+ metadata={"captcha_paused": captcha_paused},
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ttl = get_settings().control_pause_ttl_seconds | |
| async with AsyncSessionLocal() as session: | |
| src = await session.get(DataSource, source.id) | |
| if src is not None: | |
| await pause_source_for_captcha( | |
| session, | |
| source=src, | |
| now=datetime.now(timezone.utc), | |
| ttl_seconds=ttl, | |
| ) | |
| await session.commit() | |
| logger.warning( | |
| "[task:%s] captcha wall | paused source=%s (ttl=%ss, review_required)", | |
| task_id, source.id, ttl, | |
| ) | |
| if run_id: | |
| await events.emit( | |
| run_id, "collect", | |
| "验证码拦截：数据源已暂停，等待人工处理", | |
| level="warning", | |
| detail={"captcha_paused": True, "pause_ttl_seconds": ttl}, | |
| ) | |
| except Exception: | |
| logger.exception("[task:%s] failed to pause source on captcha", task_id) | |
| return PipelineResult( | |
| success=False, | |
| source_id=source.id, | |
| error=channel_result.error, | |
| metadata={"captcha_paused": True}, | |
| ) | |
| captcha_paused = False | |
| try: | |
| from backend.config import get_settings | |
| from backend.control.actuator import pause_source_for_captcha | |
| from backend.database import AsyncSessionLocal | |
| ttl = get_settings().control_pause_ttl_seconds | |
| async with AsyncSessionLocal() as session: | |
| src = await session.get(DataSource, source.id) | |
| if src is not None: | |
| await pause_source_for_captcha( | |
| session, | |
| source=src, | |
| now=datetime.now(timezone.utc), | |
| ttl_seconds=ttl, | |
| ) | |
| await session.commit() | |
| captcha_paused = True | |
| logger.warning( | |
| "[task:%s] captcha wall | paused source=%s (ttl=%ss, review_required)", | |
| task_id, source.id, ttl, | |
| ) | |
| if run_id: | |
| await events.emit( | |
| run_id, "collect", | |
| "验证码拦截：数据源已暂停，等待人工处理", | |
| level="warning", | |
| detail={"captcha_paused": True, "pause_ttl_seconds": ttl}, | |
| ) | |
| except Exception: | |
| logger.exception("[task:%s] failed to pause source on captcha", task_id) | |
| return PipelineResult( | |
| success=False, | |
| source_id=source.id, | |
| error=channel_result.error, | |
| metadata={"captcha_paused": captcha_paused}, | |
| ) |
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 288-288: String contains ambiguous ： (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF001)
[warning] 288-288: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/pipeline/pipeline.py` around lines 270 - 299, In the captcha-handling
flow, introduce a local captcha_paused flag initialized to False and set it to
True only after pause_source_for_captcha and the subsequent session.commit()
complete successfully. Return this flag in PipelineResult.metadata so missing
sources or any pause/commit exception report captcha_paused as False.
This was referenced Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #63 added
error_type='captcha_challenge'classification but nothing consumed it:is_retryable()treated it as permanent (unknown types default to permanent), so a Doubao captcha wall permanently failed the task with no recovery path — while the control layer's pause/review machinery (actuator,paused_until,review_required) had no scheduler consumer at all.Changes
error_taxonomy: addCAPTCHA_CHALLENGE+is_captcha()— a third category (needs human), neither retryable nor permanenterror_kinds: mapcaptcha_challenge→ErrorKind.CAPTCHA(control vocabulary)actuator: addpause_source_for_captcha()— pause + require_review in one call; actuator stays the only mutator ofDataSourcepipeline: on captcha_challenge collect failure, pause the source (TTL fromcontrol_pause_ttl_seconds) + flag review instead of failing permanently; best-effort so actuator/DB errors never mask the original collect errorscheduler:_get_enabled_schedulesalso skips sources withreview_required=True— a paused/flagged source is never dispatched until a human clears itTest Plan
uv run pyteston affected suites: 96 passed.ruffon changed files: no new issues vs baseline (repo has pre-existing style debt).Closes the gap recorded in G3/G5 of the opencli-admin gap PRD.