Init agent skills for Tirith - #361
Open
Akshat0694 wants to merge 13 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
- Updated CSS for verification lines to improve layout and readability. - Introduced a new DocBreadcrumbsWrapper component to integrate CopyPageMenu with breadcrumbs. - Added styles for the new DocBreadcrumbsWrapper component. - Refactored PaginatorNavLink to use a reusable Arrow component for navigation arrows. - Updated installation instructions in multiple documentation files to specify versioning for Tirith. - Added a new agent skills documentation file detailing the installation and usage of the Tirith skill pack. - Enhanced CI integration documentation to clarify usage and improve instructions. - Created a skill.sh script for installing the Tirith policy skill, ensuring a clean and safe installation process.
The skill's core instruction is "run the policy against a plan that should fail it", but the pack pointed at src/tirith/tui/examples/, which does not exist in a project that installed via skill.sh. examples/required-tags/ now ships with the pack: the SKILL.md policy, a plan that fails it (exit 3) and one that passes (exit 0). skill.sh downloads it alongside the markdown; all four exits in its README were run. Exit 2 was described three ways, all stale: "declared in status.py but never returned" (SKILL.md, Cursor rule), "platform check timeout" (verdicts.md, platform.md). status.py no longer declares it, and a platform timeout raises CheckError, which exits 1. The pack now says 2 is argparse's usage error and Tirith has no timeout code. The `tirith lint` warning was explained in full in SKILL.md, schema.md and validate.md, each with its own roadmap speculation. validate.md is now the one place; the others are a sentence and a pointer. The "When lint ships" section is gone: a skill should describe the present, and three copies of a forecast go wrong together. SKILL.md is restructured around lists rather than paragraphs, with the same facts.
…rith A separate skill rather than a reference inside tirith-policies, because the shape is the same for every source language (inventory, classify, translate, verify, report) and only the mapping tables differ. Sentinel is the first; Checkov and Rego slot in as further reference files. The mapping is measured, not guessed. All 110 policies in hashicorp/terraform-sentinel-policies and the CIS AWS policy set were classified: 41 translate exactly, 40 approximately, 29 not at all. The full table ships as reference/sentinel-corpus.md so an agent can look a public policy up instead of re-deriving it. The two dominant losses are per-block conjunction (issue #316) and instance-level pairing across resources; tfconfig-only policies account for most of the cloud-agnostic set. Five worked examples, one per fidelity story, each verified against the engine. Approximate translations ship diverges.json, a plan where Sentinel and Tirith disagree, because a fidelity note nobody can run is a fidelity note nobody reads. Three engine behaviours the translation tables depend on were verified rather than assumed: Contains on a map tests keys; a present-but-null attribute is evaluated, not skipped; and the action operation emits one result per action, so a destroy guard does not fire on a replacement. skill.sh installs both skills unconditionally. All downloads still complete before anything is copied into place, so a failed fetch of either pack installs neither.
… skill test A second Claude session installed both skills over the network, wrote three policies from plain-English requirements and reported what it found. Each finding was reproduced against the engine before being fixed. - terraform-plan.md's action example used "destroy"; plan JSON says "delete". Copied literally, the guard exits 0 on a real delete. The section is rewritten around the fact that `action` emits one result per action element: `NotEquals "delete"` blocks deletes and replacements, `ContainedIn ["delete"]` with `!` blocks only a pure delete, and the order of a replacement's two actions cannot be tested. - The migrate example prevent-database-destroy moves from approximate to exact using the universal form; its divergence plan becomes a second failing plan. sentinel.md and two corpus rows are corrected the same way; one row had put a list into NotEquals, which never equals an action string and passes everything. - A type-scoped policy on a plan with none of that type exits 3 by default and 1 with error_tolerance 1; there is no "nothing in scope, pass". Documented in verdicts.md and as a trap in SKILL.md. verdicts.md's exit table no longer claims 0 for that case, and debug-ci.md no longer sends the reader there. - The docs page said "no restart"; a running session did not see the newly installed skills until restarted. Qualified.
The live test's third phase migrated a four-policy Sentinel set and found that the skill's own restrict-ssh-ingress example returns no verdict when the plan also holds a security group with no ingress blocks: the tolerated skip erases the bastion's failure (issue #293). Both skills now say that the error_tolerance-2 "where the attribute exists" idiom is unsafe on mixed plans until #293 is fixed, and the plan reference recommends one evaluator per type over a wildcard with tolerance. Also from that phase: an unset optional list renders as null in plan JSON and Contains or NotContains on null is a hard unsupported-type failure that no tolerance forgives, so the `x else default` idiom row is split; scope differs even for exact rows because Sentinel's find_resources excludes no-op and deleted resources; an approximation that drops the test a param fed must name the orphaned param rather than ship an unread variables.json; and the mock-to-fixture recipe now says to read the test .hcl for the expected verdict first.
…elper table A fresh session auto-discovered both skills, ran fourteen fixtures and probes with every exit code as predicted, and checked the text against the engine. Three claims were wrong or half right: - An unknown condition.type does exit 3 with `errors` empty, but its result message names the evaluator. The skill said it was indistinguishable from a real violation, which sent the reader to the plan instead of to the message. Six passages corrected. - Exit 2 is never returned. The CLI catches argparse's SystemExit and exits 1, so a bad argument or `tirith lint` is 1, not 2. The morning's correction had replaced one wrong story about exit 2 with another. - `tirith --version` is mentioned in install.md but not where the install command is given. The Sentinel helper table now says its fidelity column rates the test only: scope and a computed attribute can still make a translation approximate. The smoke test's migration of an allowed-regions policy on aws_s3_bucket was exactly that case, since `region` is usually inherited from the provider and absent from change.after.
…tation - Introduced Terraform plan provider with lessons covering resource attributes, actions, and counts. - Added Kubernetes provider with lessons on manifest handling and attribute paths. - Updated the lessons structure to support multiple providers and their respective lessons. - Enhanced the learn page to display lessons grouped by provider, improving navigation. - Updated styles for track headers to maintain consistency across the documentation. - Expanded provider overview documentation to include guidance on creating new providers and examples of potential use cases.
PR #365 changed the roll-up so a tolerated skip no longer erases a sibling's verdict. Both skills now state the fixed behaviour where they used to warn about the bug, and the SSH example's notes say a skipped group leaves the bastion's failure standing. Re-verified: the probe plan that masked the bastion before now exits 3, and a compliant bucket next to a destroyed one exits 0.
|
❌ The last analysis has failed. |
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
2 participants
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.
Description
What changes are being made?
/tirith/skills/). It was excluded from the build by apages.excludeentry indocusaurus.config.js, with the navbar item commented outbeside its original. Both are restored, and
Skillsis added to the shared colophonbetween Learn and Docs.
was written before several accuracy passes landed on the rest of the site, so it was
the last surface still asserting things that are not true:
tirith lint, read the report" and "tirith lintreads the engine's ownregistries" —
tirith lintis not in the released package.src/tirith/cli.pydispatches
platformanduiand nothing else. Now describes the trap classes andnotes that
tirith uivalidates against the live registries today.ExitStatus.ERROR_TIMEOUT = 2is declared instatus.pyand returned nowhere, including on the platform path, which maps atimeout to
1.both need files that are not in this repository and both drive
tirith lint. Thatentry now points at the pipeline reference, which does ship, and says the editor loop
is in development.
applied to the rest of the site, so it never got the pass. The shipped page now has
none.
skillCopyevent. Copyingthe skill pack is this page's primary action and it was the only untracked one.
pages.excludeblock, and the note in
Colophon.jsexplaining why Skills was absent from the footer.Why are these changes necessary?
repository, so its
curlcommands and every file link returned 404. That was fixedwhen
.claude/skills/tirith-policies/was ported in Docs/promote landing pages #288, so the reason for hiding itis gone. All ten
reference/*.mdfiles it links to now resolve.corrected. A page telling a reader to run a command that does not exist is worse than a
missing page: it fails after they have committed to trying.
Which issues or tickets does this PR close or relate to?
depends on.
Type of Change
Checklist
npm run buildpasses.onBrokenLinks: 'throw'is what proves every internal linkresolves. No Python changed, so
black --check .is unaffected.documentation/README.md: the/tirith/skills/row is un-struck and the paragraphexplaining how to restore the hidden page is removed.
No test changes. This is documentation-site copy and configuration; nothing touches
src/tirith/. Correctness is enforced by the build itself, which throws on a brokeninternal link.
None. One route is added; nothing is removed or renamed.
Screenshots or Recordings (if applicable)
Additional Information
Build warning, pre-existing and benign. The build reports a broken anchor,
/tirith/skills → /tirith/learn/#playground. It is a false positive:id="playground"ispresent in the rendered
/learn/HTML, but Docusaurus only indexes anchors it can extractfrom Markdown headings, not
idattributes on JSX elements. The warning disappeared whilethis page was excluded and returns with it. It does not fail the build.
Verified rather than assumed:
reference/*.mdfiles the page links to exist under.claude/skills/tirith-policies/./skills/index.htmlcontains none of the four corrected claims, and zeroem dashes.
One thing a reviewer should look at. The page still describes the skill pack as the
source of truth for policy authoring. That is true, and the pack was itself corrected in a
recent pass, but the two now have to be kept in step: if
tirith lintships, this page,reference/validate.mdandreference/pipelines.mdall change together.