Add URL format validation to SEO diagnostics job queue #106

Closed
opened 2026-07-18 14:11:39 +02:00 by retoor · 42 comments
Owner

Summary

The SEO Diagnostics service accepts and queues URLs with obvious formatting errors, such as "ahttps://devplace.net/sitem", wasting processing resources on jobs that are guaranteed to fail.

Steps to Reproduce

  1. Submit a URL to the SEO Diagnostics service that starts with a non-standard scheme (e.g., "ahttps://devplace.net/sitem").
  2. Observe that the job is accepted and queued for the background worker.
  3. Check the job status; it will eventually fail.

Expected Behaviour

URLs that do not start with http:// or https:// or that contain malformed schemes should be rejected at the input validation stage before being enqueued.

Actual Behaviour

Malformed URLs such as "ahttps://devplace.net/sitem" and relative paths like "/feed" are accepted and queued, leading to failed jobs. 5 out of 10 total SEO jobs have failed, with some failures attributable to invalid URL formats.

Environment

Not provided.


Reported by retoor via DevPlace.

## Summary The SEO Diagnostics service accepts and queues URLs with obvious formatting errors, such as "ahttps://devplace.net/sitem", wasting processing resources on jobs that are guaranteed to fail. ## Steps to Reproduce 1. Submit a URL to the SEO Diagnostics service that starts with a non-standard scheme (e.g., "ahttps://devplace.net/sitem"). 2. Observe that the job is accepted and queued for the background worker. 3. Check the job status; it will eventually fail. ## Expected Behaviour URLs that do not start with `http://` or `https://` or that contain malformed schemes should be rejected at the input validation stage before being enqueued. ## Actual Behaviour Malformed URLs such as "ahttps://devplace.net/sitem" and relative paths like "/feed" are accepted and queued, leading to failed jobs. 5 out of 10 total SEO jobs have failed, with some failures attributable to invalid URL formats. ## Environment Not provided. --- *Reported by **retoor** via DevPlace.*
Collaborator

I am picking this ticket up now.

First I will bring up an isolated workspace with a fresh checkout and investigate whether the ticket matches the current state of the code before planning any change. I will announce every step here before I take it.

I am picking this ticket up now. First I will bring up an isolated workspace with a fresh checkout and investigate whether the ticket matches the current state of the code before planning any change. I will announce every step here before I take it.
Collaborator

Starting research round 1 of at most 5.

I am now reading the relevant source, tests, and prior work in the live checkout to verify the ticket and gather the context needed for a concrete plan. This can take a while; I will post periodic progress updates until it completes.

Starting research round 1 of at most 5. I am now reading the relevant source, tests, and prior work in the live checkout to verify the ticket and gather the context needed for a concrete plan. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Investigation confirms the ticket’s claim: SeoRunForm accepts malformed URLs such as "ahttps://..." and "/feed" because its validation only checks for blank or whitespace input. The scheme is not validated. Downstream code does reject these URLs eventually, but only after a job has been queued, wasting resources as described.

The fix is straightforward: add scheme validation to SeoRunForm.url_scheme using the pattern from IsslopRunForm, restricted to http:// and https://. No further research is needed; the ticket is accurate and specific enough to proceed. The next step is drafting a concrete implementation plan.

Investigation confirms the ticket’s claim: `SeoRunForm` accepts malformed URLs such as `"ahttps://..."` and `"/feed"` because its validation only checks for blank or whitespace input. The scheme is not validated. Downstream code does reject these URLs eventually, but only after a job has been queued, wasting resources as described. The fix is straightforward: add scheme validation to `SeoRunForm.url_scheme` using the pattern from `IsslopRunForm`, restricted to `http://` and `https://`. No further research is needed; the ticket is accurate and specific enough to proceed. The next step is drafting a concrete implementation plan.
Collaborator

I am drafting the implementation plan now.

The research confirmed the ticket; I am turning the findings into a concrete, verifiable plan.

I am drafting the implementation plan now. The research confirmed the ticket; I am turning the findings into a concrete, verifiable plan.
Collaborator

An implementation plan has been drafted from the investigation findings. The plan addresses the requirement for URL scheme validation in SeoRunForm by replacing the existing minimal validator with one that rejects inputs not starting with http:// or https://. Corresponding test coverage is added in tests/api/tools/seo.py, and all commands will be executed inside a virtual environment to avoid system package conflicts.

The next step is handing the plan to the execution agent for implementation.

An implementation plan has been drafted from the investigation findings. The plan addresses the requirement for URL scheme validation in `SeoRunForm` by replacing the existing minimal validator with one that rejects inputs not starting with `http://` or `https://`. Corresponding test coverage is added in `tests/api/tools/seo.py`, and all commands will be executed inside a virtual environment to avoid system package conflicts. The next step is handing the plan to the execution agent for implementation.
Collaborator

The full, verbatim implementation plan is attached: plan-ticket-106.md

The full, verbatim implementation plan is attached: [plan-ticket-106.md](https://retoor.molodetz.nl/attachments/b47a28b4-304c-48a6-beb1-52f47575f2fc)
Collaborator

Starting implementation attempt 1 of at most 5.

I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.

Starting implementation attempt 1 of at most 5. I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Attempt 1 completed but was inconclusive. The execution agent's output did not clearly indicate success or failure. Another round of targeted research is in progress before retrying.

Attempt 1 completed but was inconclusive. The execution agent's output did not clearly indicate success or failure. Another round of targeted research is in progress before retrying.
Collaborator

Starting research round 2 of at most 5.

I am now reading the relevant source, tests, and prior work in the live checkout to verify the ticket and gather the context needed for a concrete plan. This can take a while; I will post periodic progress updates until it completes.

Starting research round 2 of at most 5. I am now reading the relevant source, tests, and prior work in the live checkout to verify the ticket and gather the context needed for a concrete plan. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Compared the ticket requirements against the current source and test suite, and cross-referenced external documentation and general knowledge to verify feasibility. The research confirms that the exact validation described—rejecting URLs that do not start with http:// or https://—is already implemented in SeoRunForm via the SEO_URL_SCHEME_PATTERN regex and field_validator, with corresponding test coverage. The ticket appears fully satisfied by the existing code on this branch.

This does not match the ticket’s stated need for further action. Could you confirm whether the ticket should be closed, or is there a requirement that is not captured by the current implementation?

Compared the ticket requirements against the current source and test suite, and cross-referenced external documentation and general knowledge to verify feasibility. The research confirms that the exact validation described—rejecting URLs that do not start with `http://` or `https://`—is already implemented in `SeoRunForm` via the `SEO_URL_SCHEME_PATTERN` regex and `field_validator`, with corresponding test coverage. The ticket appears fully satisfied by the existing code on this branch. This does not match the ticket’s stated need for further action. Could you confirm whether the ticket should be closed, or is there a requirement that is not captured by the current implementation?
Collaborator

Resuming this ticket. Root-cause analysis found two bugs in my own pipeline that caused this specific escalation, not anything about your report:

  1. The execution-outcome classifier used to make a single, non-retried call to check whether my implementation attempt succeeded, and silently treated any transient hiccup as "inconclusive" — discarding a real, already-committed code change and sending the ticket back into research.
  2. When that happened, my re-investigation then found the change I had just made already present in the code and concluded the ticket must be stale, asking you to confirm/close it instead of recognizing it as my own unfinished work.

Separately, the workspace container's Python/pip setup could never satisfy this project's own test/lint commands (PEP 668 + a Python version mismatch), so even a correct change could never pass verification and reach a pull request. All three are fixed now; this ticket's workspace has been rebuilt on the corrected image and its state reset to resume investigation.

Resuming this ticket. Root-cause analysis found two bugs in my own pipeline that caused this specific escalation, not anything about your report: 1. The execution-outcome classifier used to make a single, non-retried call to check whether my implementation attempt succeeded, and silently treated any transient hiccup as "inconclusive" — discarding a real, already-committed code change and sending the ticket back into research. 2. When that happened, my re-investigation then found the change I had just made already present in the code and concluded the ticket must be stale, asking you to confirm/close it instead of recognizing it as my own unfinished work. Separately, the workspace container's Python/pip setup could never satisfy this project's own test/lint commands (PEP 668 + a Python version mismatch), so even a correct change could never pass verification and reach a pull request. All three are fixed now; this ticket's workspace has been rebuilt on the corrected image and its state reset to resume investigation.
Collaborator

The ticket has been cancelled and closed at your request. The reply explicitly asked to close, ignore, or drop this ticket, so no further action was taken.

If any follow-up is needed, please confirm whether you would like to proceed with a different course of action.

The ticket has been cancelled and closed at your request. The reply explicitly asked to close, ignore, or drop this ticket, so no further action was taken. If any follow-up is needed, please confirm whether you would like to proceed with a different course of action.
Collaborator

Correction: this ticket was closed by a bug in my own comment-handling pipeline, not by anything said here. My prior resume comment on this ticket was, due to that bug, misread as a human reply and its own explanatory text (mentioning "close it" while describing why the earlier escalation happened) was misclassified as a request to cancel. That bug is now fixed in three independent ways: my own comments are never again treated as a human reply regardless of content, leaving an escalation state now always clears the pending-question flag, and the cancel-detection safety net's negation check now covers the whole sentence, not just the part before the trigger phrase. Reopening this issue and resuming the ticket.

Correction: this ticket was closed by a bug in my own comment-handling pipeline, not by anything said here. My prior resume comment on this ticket was, due to that bug, misread as a human reply and its own explanatory text (mentioning "close it" while describing why the earlier escalation happened) was misclassified as a request to cancel. That bug is now fixed in three independent ways: my own comments are never again treated as a human reply regardless of content, leaving an escalation state now always clears the pending-question flag, and the cancel-detection safety net's negation check now covers the whole sentence, not just the part before the trigger phrase. Reopening this issue and resuming the ticket.
Collaborator

Starting research round 3 of at most 5.

I am now reading the relevant source, tests, and prior work in the live checkout to verify the ticket and gather the context needed for a concrete plan. This can take a while; I will post periodic progress updates until it completes.

Starting research round 3 of at most 5. I am now reading the relevant source, tests, and prior work in the live checkout to verify the ticket and gather the context needed for a concrete plan. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Compared the ticket requirements against the current source and test suite. The research confirms that the exact validation demanded—rejecting URLs not starting with http:// or https://—has already been implemented, tested, and merged on this ticket's own branch in a prior round. No clarification from the reporter contradicts this finding, and the change is not an unrelated fix; it is the same work the ticket describes.

The next step is to verify the existing change against the project's own checks to confirm it meets project standards and is ready for merge. No further development is required unless these checks reveal a gap.

Compared the ticket requirements against the current source and test suite. The research confirms that the exact validation demanded—rejecting URLs not starting with `http://` or `https://`—has already been implemented, tested, and merged on this ticket's own branch in a prior round. No clarification from the reporter contradicts this finding, and the change is not an unrelated fix; it is the same work the ticket describes. The next step is to verify the existing change against the project's own checks to confirm it meets project standards and is ready for merge. No further development is required unless these checks reveal a gap.
Collaborator

Starting implementation attempt 2 of at most 5.

I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.

Starting implementation attempt 2 of at most 5. I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Implementation attempt 2 finished; I am verifying it now.

Running the project's own build, test, and lint commands against the change.

Implementation attempt 2 finished; I am verifying it now. Running the project's own build, test, and lint commands against the change.
Collaborator

Attempt 2 did not succeed. Both test_command and lint_command checks failed.

The change either failed to apply cleanly to the current branch state or did not pass the project’s own validation suite.

A third attempt will be made using the recorded failure details and lessons from the previous run.

Attempt 2 did not succeed. Both `test_command` and `lint_command` checks failed. The change either failed to apply cleanly to the current branch state or did not pass the project’s own validation suite. A third attempt will be made using the recorded failure details and lessons from the previous run.
Collaborator

Starting implementation attempt 3 of at most 5.

I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.

Starting implementation attempt 3 of at most 5. I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Implementation attempt 3 finished; I am verifying it now.

Running the project's own build, test, and lint commands against the change.

Implementation attempt 3 finished; I am verifying it now. Running the project's own build, test, and lint commands against the change.
Collaborator

Attempt 3 failed due to failing checks in test_command. The change either did not apply cleanly or did not pass the project's own validation. The next attempt will incorporate the recorded failure details and lessons learned from this run.

Attempt 3 failed due to failing checks in test_command. The change either did not apply cleanly or did not pass the project's own validation. The next attempt will incorporate the recorded failure details and lessons learned from this run.
Collaborator

Starting implementation attempt 4 of at most 5.

I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.

Starting implementation attempt 4 of at most 5. I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Implementation attempt 4 finished; I am verifying it now.

Running the project's own build, test, and lint commands against the change.

Implementation attempt 4 finished; I am verifying it now. Running the project's own build, test, and lint commands against the change.
Collaborator

Attempt 4 did not succeed. The checks failed on test_command, indicating the change either did not apply cleanly or did not satisfy the project's own validation criteria.

The next attempt will incorporate the recorded failure details and lessons learned from this iteration.

Attempt 4 did not succeed. The checks failed on `test_command`, indicating the change either did not apply cleanly or did not satisfy the project's own validation criteria. The next attempt will incorporate the recorded failure details and lessons learned from this iteration.
Collaborator

Starting implementation attempt 5 of at most 5.

I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.

Starting implementation attempt 5 of at most 5. I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Implementation attempt 5 finished; I am verifying it now.

Running the project's own build, test, and lint commands against the change.

Implementation attempt 5 finished; I am verifying it now. Running the project's own build, test, and lint commands against the change.
Collaborator

We implemented five iterations, each building on lessons from the prior failure, but the latest attempt still fails the test_command check. The root cause is still unclear from the test output alone.

Could you provide more guidance or a narrower scope to help resolve the test_command failure?

We implemented five iterations, each building on lessons from the prior failure, but the latest attempt still fails the test_command check. The root cause is still unclear from the test output alone. Could you provide more guidance or a narrower scope to help resolve the test_command failure?
Collaborator

I am picking this ticket up now.

First I will bring up an isolated workspace with a fresh checkout and investigate whether the ticket matches the current state of the code before planning any change. I will announce every step here before I take it.

I am picking this ticket up now. First I will bring up an isolated workspace with a fresh checkout and investigate whether the ticket matches the current state of the code before planning any change. I will announce every step here before I take it.
Collaborator

I need input to continue.

What I tried: Retried this step 3 times.
What I found: Each attempt failed for internal reasons unrelated to this ticket. The details are in the operator's logs; this is not a judgment about the ticket.

The system needs operator attention before I can continue. Reply here once things are healthy and I will resume from where I left off.

I need input to continue. What I tried: Retried this step 3 times. What I found: Each attempt failed for internal reasons unrelated to this ticket. The details are in the operator's logs; this is not a judgment about the ticket. The system needs operator attention before I can continue. Reply here once things are healthy and I will resume from where I left off.
Collaborator

I am picking this ticket up now.

First I will bring up an isolated workspace with a fresh checkout and investigate whether the ticket matches the current state of the code before planning any change. I will announce every step here before I take it.

I am picking this ticket up now. First I will bring up an isolated workspace with a fresh checkout and investigate whether the ticket matches the current state of the code before planning any change. I will announce every step here before I take it.
Collaborator

Starting research round 1 of at most 5.

I am now reading the relevant source, tests, and prior work in the live checkout to verify the ticket and gather the context needed for a concrete plan. This can take a while; I will post periodic progress updates until it completes.

Starting research round 1 of at most 5. I am now reading the relevant source, tests, and prior work in the live checkout to verify the ticket and gather the context needed for a concrete plan. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Investigation confirms the ticket's diagnosis. SeoRunForm lacks URL scheme validation, allowing malformed URLs that pass the model and enqueue only to fail at guard_public_url; the reference implementation in IsslopRunForm validates against non-http/https schemes at input. No further investigation is required.

Decision is to proceed. Next step is drafting a concrete implementation plan to add the same validation to SeoRunForm.

Investigation confirms the ticket's diagnosis. `SeoRunForm` lacks URL scheme validation, allowing malformed URLs that pass the model and enqueue only to fail at `guard_public_url`; the reference implementation in `IsslopRunForm` validates against non-`http`/`https` schemes at input. No further investigation is required. Decision is to proceed. Next step is drafting a concrete implementation plan to add the same validation to `SeoRunForm`.
Collaborator

I am drafting the implementation plan now.

The research confirmed the ticket; I am turning the findings into a concrete, verifiable plan.

I am drafting the implementation plan now. The research confirmed the ticket; I am turning the findings into a concrete, verifiable plan.
Collaborator

The investigation findings were used to draft an implementation plan for URL format validation in the SEO diagnostics job queue. The plan addresses the root cause: the SeoRunForm model lacks scheme validation, allowing invalid URLs to pass through to the background queue before being rejected by downstream guards. This wastes job resources and generates unnecessary failures.

The implementation covers two files: app/core/models.py (adding a Pydantic field validator and a URL pattern constant) and tests/api/tools/seo.py (adding four targeted test cases). The plan includes acceptance criteria for passing tests and lint checks, and explicitly avoids repeating past integration issues.

The plan is now ready for execution. Handover to the execution agent.

The investigation findings were used to draft an implementation plan for URL format validation in the SEO diagnostics job queue. The plan addresses the root cause: the `SeoRunForm` model lacks scheme validation, allowing invalid URLs to pass through to the background queue before being rejected by downstream guards. This wastes job resources and generates unnecessary failures. The implementation covers two files: `app/core/models.py` (adding a Pydantic field validator and a URL pattern constant) and `tests/api/tools/seo.py` (adding four targeted test cases). The plan includes acceptance criteria for passing tests and lint checks, and explicitly avoids repeating past integration issues. The plan is now ready for execution. Handover to the execution agent.
Collaborator

The full, verbatim implementation plan is attached: plan-ticket-106.md

The full, verbatim implementation plan is attached: [plan-ticket-106.md](https://retoor.molodetz.nl/attachments/195e796d-326b-4cea-b0ee-752d1df3a7cd)
Collaborator

Starting implementation attempt 1 of at most 5.

I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.

Starting implementation attempt 1 of at most 5. I am executing the plan inside this ticket's isolated workspace. This can take a while; I will post periodic progress updates until it completes.
Collaborator

Implementation attempt 1 finished; I am verifying it now.

Running the project's own build, test, and lint commands against the change.

Implementation attempt 1 finished; I am verifying it now. Running the project's own build, test, and lint commands against the change.
Collaborator

Implemented the change on the first attempt. The project's build, test, and lint commands all completed successfully. Next steps are to commit the change, push a branch, and open a pull request.

Implemented the change on the first attempt. The project's build, test, and lint commands all completed successfully. Next steps are to commit the change, push a branch, and open a pull request.
Collaborator

I am delivering the change now.

Committing the verified change, pushing branch typosaurus/ticket-106, and opening a pull request.

I am delivering the change now. Committing the verified change, pushing branch `typosaurus/ticket-106`, and opening a pull request.
typosaurus referenced this issue from a commit 2026-07-23 04:38:02 +02:00
Collaborator

The change has been committed and the branch typosaurus/ticket-106 pushed. A corresponding pull request has been opened at #125.

This step was taken because the implementation passed the project's internal checks.

The pull request is now being monitored for review, merge, or any further feedback.

The change has been committed and the branch `typosaurus/ticket-106` pushed. A corresponding pull request has been opened at https://retoor.molodetz.nl/retoor/devplacepy/pulls/125. This step was taken because the implementation passed the project's internal checks. The pull request is now being monitored for review, merge, or any further feedback.
Collaborator

The pull request for this change has been merged. The fix was validated through the standard review and testing process.

No further action is required. This ticket is resolved.

The pull request for this change has been merged. The fix was validated through the standard review and testing process. No further action is required. This ticket is resolved.
Collaborator

This ticket has reached its outcome. The pull request is merged and the branch is no longer needed.

Would you like me to clean up the branch and workspace for this ticket? Reply with /typosaurus cancel-and-cleanup to confirm, or leave it as is.

This ticket has reached its outcome. The pull request is merged and the branch is no longer needed. Would you like me to clean up the branch and workspace for this ticket? Reply with `/typosaurus cancel-and-cleanup` to confirm, or leave it as is.
Sign in to join this conversation.
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: retoor/devplacepy#106
No description provided.