Serve robots.txt and a sitemap that lists every tool - #175
Merged
Conversation
Frappe answers /robots.txt from a Website Settings field. That field is empty, so the site served a 200 with no body. The value is site data rather than repository data, so anything typed into the desk would have to be typed again on the next deployment, and nothing in the repository would record what it said. Frappe answers /sitemap.xml from its own page list and from DocTypes with a web view. Toolbox tools are website route rules, so none of them appeared. The sitemap offered exactly two URLs, Frappe's stock /about and /contact, and hid all fifteen tools. Both files now come from the app, built from toolbox/routes.py. The override holds because TemplatePage searches installed apps in reverse order and Toolbox is installed after Frappe. The sitemap lists the routes seo.py marks indexable, which is the same flag that writes the robots meta tag, so a page cannot be told to stay out of the index and be advertised for crawling in the same breath. That is why /settings is absent. There is no lastmod: Frappe's sitemap stamps every entry with today's date, every day, and a date that is always wrong teaches a crawler to discount the real ones later. robots.txt carries no "Allow: /" line. Anything not disallowed is already allowed, and the line would not be harmless. Google resolves a conflict by the longest matching path, but several parsers take the first match, including Python's own robotparser; under those a leading blanket allow wins every comparison and voids every Disallow below it. Measured before the line was removed: /api/method/... read as crawlable. /settings is not disallowed either. It carries noindex and is linked from the navigation, and a crawler that is blocked from fetching a page never reads the noindex on it, so the URL can still surface with no description at all. backend 169, vitest 610, e2e 195 passed and 6 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Closes #173. Follows #174.
Both files exist on the deployed site today. Both are useless.
/robots.txtreturns200withContent-Length: 0. Frappe reads it from a Website Settings field, which is empty. That field is site data, so a value typed into the desk would have to be typed again on the next deployment, and nothing in the repository would record what it said./sitemap.xmlreturns Frappe's stock pages and not one tool:Frappe builds it from its own page list and from DocTypes with a web view. Toolbox tools are website route rules, so none of them qualify. All fifteen were invisible to anything reading the sitemap.
After
Both are generated from
toolbox/routes.py. The override holds becauseTemplatePagesearches installed apps in reverse order and Toolbox is installed after Frappe — an e2e test asserts the served result rather than the mechanism, so the day that stops being true, it fails.Two decisions that look like omissions
No
Allow: /. Anything not disallowed is already allowed, and the line is not harmless. Google resolves a conflict by longest matching path, but several parsers take the first match — including Python's ownrobotparser. Under those, a leading blanket allow wins every comparison and voids everyDisallowbeneath it. This was measured, not assumed: with the line present,/api/method/toolbox.weather.forecastparsed as crawlable.test_it_declares_no_blanket_allowis the guard against putting it back./settingsis not disallowed. It carriesnoindexfrom #174 and it is linked from the navigation. A crawler blocked from fetching a page never reads thenoindexon it, so the URL can still surface in results with no description. Blocking and hiding are different instructions, and only one of them belongs on a page we link to.No
lastmod. Frappe's sitemap stamps every entry with today's date, every day. A date that is always wrong is worse than none: a crawler that stops trusting it discounts the real ones later. The sitemap protocol makes the field optional.Consistency with #174
The sitemap lists the routes
seo.pymarksindexable, which is the same flag that writes the robots meta tag. A page cannot be told to stay out of the index and be advertised for crawling in the same breath, andtest_the_count_matches_what_seo_declares_indexableholds the two together.Verified
Live on
toolbox.localhost:8100:/sitemap.xmlis valid XML underElementTreewith 16 URLs and correctapplication/xml;/robots.txtistext/plain, parses underRobotFileParser, declares the sitemap, and returns the intended verdict for each path — tools crawlable,/settingscrawlable,/api/and/app/blocked.backend 169 (was 158) · vitest 610 · e2e 195 passed / 6 skipped (was 189/6) · build clean.
ruffandpre-commitare not installed in this bench, so the Python was checked against the project's conventions by hand rather than by running the hooks.