Add Claude Code GitHub Workflow - #2
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces GitHub Actions workflows to enable Claude Code integration for automated code assistance and review. It adds two workflows: one for interactive Claude interactions via @mentions in comments, and another for automatic code reviews on pull requests.
- Adds
claude.ymlworkflow for triggering Claude via @mentions in PRs/issues - Adds
claude-code-review.ymlworkflow for automated PR reviews - Configures permissions and tool access for Claude operations
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| .github/workflows/claude.yml | Enables Claude Code interaction via @mentions in PR/issue comments with read permissions and optional CI result access |
| .github/workflows/claude-code-review.yml | Configures automated Claude code reviews on PR open/update events with specific bash tool allowances for GitHub CLI commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - Security concerns | ||
| - Test coverage | ||
|
|
||
| Use the repository's CLAUDE.md for guidance on style and conventions. Be constructive and helpful in your feedback. |
There was a problem hiding this comment.
The workflow references a CLAUDE.md file that doesn't appear to exist in the repository. Either this file should be created to provide style and convention guidance, or this line should be removed/updated to reference existing documentation.
Suggested change
| Use the repository's CLAUDE.md for guidance on style and conventions. Be constructive and helpful in your feedback. | |
| Be constructive and helpful in your feedback. If style or convention guidance is needed, refer to the repository's CONTRIBUTING.md or other relevant documentation. |
Comment on lines
+22
to
+24
| contents: read | ||
| pull-requests: read | ||
| issues: read |
There was a problem hiding this comment.
The workflow has read-only permissions for contents, pull-requests, and issues, which may prevent Claude from creating commits, branches, or PR comments as mentioned in the PR description. Consider adding contents: write, pull-requests: write, and issues: write if Claude needs to make changes and post comments.
Suggested change
| contents: read | |
| pull-requests: read | |
| issues: read | |
| contents: write | |
| pull-requests: write | |
| issues: write |
Comment on lines
+24
to
+25
| pull-requests: read | ||
| issues: read |
There was a problem hiding this comment.
The workflow only has read permissions, but line 52 instructs Claude to use gh pr comment to leave review comments. This will fail without pull-requests: write and issues: write permissions. Add write permissions to enable Claude to post comments.
Suggested change
| pull-requests: read | |
| issues: read | |
| pull-requests: write | |
| issues: write |
|
|
||
| # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md | ||
| # or https://docs.claude.com/en/docs/claude-code/cli-reference for available options | ||
| claude_args: '--allowed-tools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*)"' |
There was a problem hiding this comment.
The allowed-tools configuration uses wildcard patterns (e.g., gh pr comment:*), which could allow arbitrary arguments to these commands. This may introduce security risks if malicious input is provided. Consider being more explicit about which subcommands and arguments are allowed, or ensure input validation is handled by the action itself.
Suggested change
| claude_args: '--allowed-tools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*)"' | |
| claude_args: '--allowed-tools "Bash(gh issue view --json number,title,body,state,author),Bash(gh search issues --state all --limit 10),Bash(gh issue list --state all --limit 10),Bash(gh pr comment --body * --repo ${{ github.repository }} --number ${{ github.event.pull_request.number }}),Bash(gh pr diff --repo ${{ github.repository }} --number ${{ github.event.pull_request.number }}),Bash(gh pr view --json number,title,body,state,author --repo ${{ github.repository }} --number ${{ github.event.pull_request.number }}),Bash(gh pr list --state all --limit 10 --repo ${{ github.repository }})"' |
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 1 |
There was a problem hiding this comment.
Using fetch-depth: 1 may be insufficient for code review purposes, as Claude might need to access commit history or compare against base branches. Consider using fetch-depth: 0 to fetch full history, or at minimum a higher value to allow meaningful comparisons.
Suggested change
| fetch-depth: 1 | |
| fetch-depth: 0 |
Comment on lines
+39
to
+42
| # This is an optional setting that allows Claude to read CI results on PRs | ||
| additional_permissions: | | ||
| actions: read | ||
|
|
There was a problem hiding this comment.
The additional_permissions parameter (lines 40-41) duplicates the actions: read permission already granted at line 26. This redundancy may cause confusion. Consider removing the additional_permissions block since the permission is already granted at the job level.
Suggested change
| # This is an optional setting that allows Claude to read CI results on PRs | |
| additional_permissions: | | |
| actions: read |
This was referenced Dec 21, 2025
This was referenced Dec 30, 2025
This was referenced Jan 7, 2026
5 tasks
5 tasks
adnaan
added a commit
that referenced
this pull request
May 10, 2026
…t, fix terminology Fixes from Claude (4 reviews) and Copilot (5 inline) bot feedback: - Restore "Git-native and self-hosted" Why-bullet (3 Claude reviews flagged it as a meaningful differentiator that was dropped without replacement; matters for compliance/air-gap users). - Standardize "Postgres" → "PostgreSQL" across Why bullets, artifact table dashboard row, and triage row (Copilot, 3 occurrences). - Standardize "websocket"/"websockets" → "WebSocket" (Copilot, 2 places). - Soften "another Claude page aesthetic" → "generic LLM-output aesthetic" (Claude review #1: vendor-specific marketing claim ages poorly). - Clarify `lvt` → `lvt` code blocks in the interactive-explainer row (Claude reviews #1, #2: bare `lvt` was ambiguous vs the `lvt-*` attribute vocabulary used elsewhere). - Reword "Generated HTML drifts from the prompt" → "is opaque to hand-editing" (Claude review #4: "drifts" implies decay; the actual problem is opacity to manual edits). - Reword "frontmatter that's a YAML schema" → "frontmatter with a small set of well-defined fields" (Copilot: original implied formal schema validation that doesn't exist). - Add bridging sentence to AI-Assisted Development section (Claude review #1: "Describe what you want:" felt orphaned without a connector). Not addressed (deliberate): - "Why not just ask Claude for an HTML file?" heading kept Claude-specific. It's a deliberate positioning choice for the HN-derived audience; the section body is agent-agnostic. - Artifact table row length unchanged — minor stylistic suggestion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adnaan
added a commit
that referenced
this pull request
May 23, 2026
Round-5 review identified one blocker (#1) and added several nice-to-haves; addressing all of them in this commit. **Blocker — layout footgun guard (#1)** Resolve now detects when filepath.Dir(resolvedRoot) == resolvedRoot (true only when root is the filesystem root "/") and rejects site-rooted includes with a clear error rather than silently expanding the include surface to the whole filesystem. The doc comment already warned callers about non-standard layouts; this makes the genuinely broken case a loud failure. New test TestResolve_SiteRootedRejectsRootlessLayout pins the behavior: Resolve("/tmp", "/", "/etc/passwd") → "no parent" error. **Distinguishable error messages (#3)** The two confinement branches now emit specific errors: - Site-rooted escape → "include %q escapes the project root" - Page-relative escape → "include %q escapes the content root" Was a single shared "escapes the include root" — ambiguous when debugging. No tests asserted the old text (only the existence of err); safe to change. **Simplified TrimLeft/TrimPrefix chain (#2)** The path-normalisation arithmetic in the site-rooted branch dropped the intermediate `normalised := "/" + strings.TrimLeft(...)` variable. Now: `relative := strings.TrimLeft(includePath, "/")` fed directly into the "/"-empty check + filepath.Join. Fewer trims, same behavior. **Missing test — empty PagePathInRepo + site-rooted (#test-gap-1)** TestPreprocess_GitHubFooter_SiteRooted_NoPagePathInRepo confirms the site-rooted footer URL has no spurious double-slash when PagePathInRepo is unset (common for top-level pages). Locks in that the site-rooted branch doesn't fall through to the page-relative "PagePathInRepo + relToPage" join when PagePathInRepo == "". **Test robustness — explicit temp-dir invariant (#test-gap-3)** TestResolve_SiteRootedRejectsProjectEscape now skips (instead of silently passing or failing misleadingly) if the test framework ever stops placing two t.TempDir() calls under the same parent. The "/../<basename>/outside.go" escape path is only meaningful when project and outside are siblings. 13/13 tests pass. Full tinkerdown suite green. Docs validate clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adnaan
added a commit
that referenced
this pull request
May 23, 2026
* feat(include): site-rooted include="/..." paths Adds a new branch in include.Resolve: a leading "/" in the include attribute is interpreted as project-root-relative (one level above the discovery root) rather than page-relative. Files cited this way are confined to the project root, not the content root — pages can now reference source files that live in sibling top-level folders (e.g. a docs site referencing examples/<slug>/file.go) without resorting to "../../..." chains, which the content-root confinement would reject anyway. renderSourceFooter uses the include attribute verbatim as the repo-relative path for site-rooted includes, sidestepping the "include outside pageDir" guard that page-relative includes hit when they escape the page directory. Backward compatible: existing page-relative includes (`./_app/...`, `../foo.go`) continue to use the v1 confinement against the content root. Tests added: site-rooted resolution reaches sibling folder; escape above project root is rejected; footer URL uses include path verbatim (not PagePathInRepo). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(include): address PR review comments Addresses the four items from the Claude code review on #280: 1. **EvalSymlinks on projectRoot** — `filepath.Dir(resolvedRoot)` only resolves through one symlink level (the discovery root itself). If an ancestor directory is symlinked (e.g. the whole project lives under macOS's /tmp → /private/tmp tree), the later `filepath.Rel(confineRoot, resolvedCandidate)` check could false-positive as an escape. Re-running EvalSymlinks on the project root makes the confinement boundary airtight. 2. **t.Cleanup instead of defer** in TestResolve_SiteRootedRejectsProjectEscape — the test writes a file outside its own t.TempDir() so the symlink-escape scenario has somewhere to escape to. t.Cleanup runs even on panic; defer would leak the file if the test panicked before the deferred call. 3. **New test** TestResolve_SiteRootedCanTargetContentRoot pins down the (intentional) behavior that site-rooted paths landing inside the content root are allowed. The project root is a superset of the content root, so this falls out naturally; the test makes the intent explicit for future maintainers. 4. **Function-level doc on renderSourceFooter** now spells out that PagePathInRepo is IGNORED for site-rooted includes, and that incPath must be the raw attribute value (any caller that pre- normalizes the path would silently strip the leading "/" and produce a wrong footer URL with no error). go test ./internal/include/... — 9/9 tests pass (incl. new one). go test -tags=ci ./... — full tinkerdown suite green. tinkerdown validate against docs PR's content/ — 55/55 pages valid. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: untrack test.db + add *.db to gitignore test.db is a runtime artifact from an example using SQLite, not source. Pinning it in commit 644eec9 was unintended. Remove it from tracking and gitignore the pattern so future test runs don't re-stage it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(.gitignore): dedupe pre-existing .worktrees/ entries The file had `.worktrees/` listed four times and a stray bare `.worktrees/` mixed in with SQLite ignores. Consolidate into one section per concern. Functional behavior unchanged — git already ignored .worktrees/ regardless of how many times the pattern appeared. Picked up as the non-blocking nit from the review on the previous fix commit (f9286ae) which added *.db next to one of the duplicates. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(include): address follow-up review (#280 round 3) Round-3 review surfaced 4 items; 3 actionable, 1 false positive: **Actionable:** 1. Test hygiene — TestResolve_SiteRootedRejectsProjectEscape was writing outside.go to filepath.Dir(t.TempDir()), which on most runners is /tmp itself. Concurrent test instances could collide on the same path. Switched to a second t.TempDir() for "outside" and computed the escape path relative to its basename — both trees are now fully test-framework-managed. 2. Normalise multiple leading slashes in Resolve. filepath.Join handles "//foo" correctly on Linux but the TrimPrefix arithmetic was clearer with a single canonical leading "/". One-liner: includePath = "/" + strings.TrimLeft(includePath, "/"). 3. Reject "/" alone — resolves to the project root directory itself; downstream Slice would try to read a directory as a file and produce a confusing error. Early-return with a clearer "resolves to the project root, not a file" message. New test TestResolve_SiteRootedRejectsRootOnly covers "/", "//", "///". **False positive (no change):** 4. Reviewer suggested URL-encoding repoPath segments in renderSourceFooter. The href construction already passes repoPath through escapePath(), which URL-encodes each forward-slash-separated segment via url.PathEscape. Both branches go through the same href construction line, so both site-rooted and page-relative URLs are already correctly encoded. No code change needed. go test ./internal/include/... — 10/10 tests pass. go test -tags=ci ./... — full tinkerdown suite green. tinkerdown validate against docs PR's content/ — 55/55 pages valid. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(include): TrimLeft in renderSourceFooter + docs (#280 round 4) Round-4 review found a real bug: renderSourceFooter used TrimPrefix which only strips ONE leading slash. So while Resolve correctly normalises "//foo" → "/foo" before resolving (round-3 change), renderSourceFooter received the raw "//foo" and produced repoPath "/foo" — the leading slash leaked into the rendered GitHub URL as "/blob/main//examples/...". Fix: switch TrimPrefix to TrimLeft in renderSourceFooter, mirroring Resolve. New test TestPreprocess_GitHubFooter_SiteRooted_StripsExtraSlashes locks in the behavior with both a positive assertion ("/blob/main/examples/x/x.go" present) and an explicit anti-assertion ("/blob/main//" absent). Also addressed from the same review: - Documented the layout assumption in Resolve's doc comment: site-rooted paths use filepath.Dir(root) as the project root, hard-coding the convention that the discovery root sits one level inside the project. Callers with non-standard layouts where root IS the project root should use page-relative includes only. - Added inline comments to each branch in Resolve marking the confinement-root asymmetry (page-relative confines to discovery root; site-rooted confines to project root, one level up). This is the security model's crux — easy to miss without a marker. 11/11 tests pass. Full tinkerdown suite green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(include): guard non-standard layout + clearer errors (round 5) Round-5 review identified one blocker (#1) and added several nice-to-haves; addressing all of them in this commit. **Blocker — layout footgun guard (#1)** Resolve now detects when filepath.Dir(resolvedRoot) == resolvedRoot (true only when root is the filesystem root "/") and rejects site-rooted includes with a clear error rather than silently expanding the include surface to the whole filesystem. The doc comment already warned callers about non-standard layouts; this makes the genuinely broken case a loud failure. New test TestResolve_SiteRootedRejectsRootlessLayout pins the behavior: Resolve("/tmp", "/", "/etc/passwd") → "no parent" error. **Distinguishable error messages (#3)** The two confinement branches now emit specific errors: - Site-rooted escape → "include %q escapes the project root" - Page-relative escape → "include %q escapes the content root" Was a single shared "escapes the include root" — ambiguous when debugging. No tests asserted the old text (only the existence of err); safe to change. **Simplified TrimLeft/TrimPrefix chain (#2)** The path-normalisation arithmetic in the site-rooted branch dropped the intermediate `normalised := "/" + strings.TrimLeft(...)` variable. Now: `relative := strings.TrimLeft(includePath, "/")` fed directly into the "/"-empty check + filepath.Join. Fewer trims, same behavior. **Missing test — empty PagePathInRepo + site-rooted (#test-gap-1)** TestPreprocess_GitHubFooter_SiteRooted_NoPagePathInRepo confirms the site-rooted footer URL has no spurious double-slash when PagePathInRepo is unset (common for top-level pages). Locks in that the site-rooted branch doesn't fall through to the page-relative "PagePathInRepo + relToPage" join when PagePathInRepo == "". **Test robustness — explicit temp-dir invariant (#test-gap-3)** TestResolve_SiteRootedRejectsProjectEscape now skips (instead of silently passing or failing misleadingly) if the test framework ever stops placing two t.TempDir() calls under the same parent. The "/../<basename>/outside.go" escape path is only meaningful when project and outside are siblings. 13/13 tests pass. Full tinkerdown suite green. Docs validate clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adnaan
added a commit
that referenced
this pull request
Jun 20, 2026
…293) Three browser E2E tests flaked in the cross-repo job; two distinct, unrelated root causes (plus three more tests sharing cause #2): 1. TestActionButtons — SQLITE_BUSY. SQLite's busy_timeout defaults to 0, so a read that races a concurrent write fails immediately with "database is locked". The library now opens every SQLite connection with busy_timeout=5000 (internal/source: new sqliteDSN helper used by both SQLiteSource and the schema probe), and the test's own verification connection gets the same pragma — the read waits for the action's write to commit instead of failing. 2. Checkbox toggles never reflected the server's new state (TestAutoTasks_BasicToggle/_NoFullReload, TestLvtSourceMarkdownToggle/ ToggleBack). The client's morphdom pass preserves live <input> state over server-rendered values; for a server-authoritative lvt-on:click="Toggle" checkbox that's backwards, so the server's toggle never reflected in the DOM. Fixed upstream in @livetemplate/client v0.14.3 (livetemplate/client#135): an lvt-on:click checkbox/radio is now server-authoritative by default. Bump @livetemplate/client 0.11.9 → 0.14.3 and rebuild the embedded bundle; no per-template attribute is needed — the framework handles it generically for every server-driven toggle. Also hardened the auto-task E2E interactions: switch from chromedp.Click to JS .click() (CDP click is unreliable for delegated handlers in headless Docker Chrome, as action_buttons_e2e_test.go already documents) and replace fixed sleeps with waitForDOM polling for the actual expected state. Verified locally against Docker Chrome with @livetemplate/client@0.14.3: TestActionButtons, all six TestAutoTasks, both markdown toggle tests green on repeat runs. Claude-Session: https://claude.ai/code/session_01HyPY7btbvCZiLKkh9wSKJx Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
adnaan
added a commit
that referenced
this pull request
Jul 24, 2026
…ic checks) (#307) * M2 kickoff: expand deterministic-validation design (convention 9) Expands the outline-only M2 into full phase blocks, informed by M1's verified Learn feed-forwards and a direct read of ../livetemplate @ v0.20.1. Milestone shape (operator decision 2026-07-23): keep the upstream `Validate()` — cross-repo, upstream-first — over a tinkerdown-only lint. Three phases: 1. (upstream ../livetemplate) `livetemplate.Validate(templateText)` + tagged release. Earns its place because the structured, line-numbered parse/reactive-AST diagnostics live in `internal/parse` (unreachable downstream) and the public render path silently swallows them — a public API built on the internal primitives is the only way tinkerdown gets them. Render-determinism is an optional `WithSampleData` mode (needs typed state); data-free parse+AST checks are the core. 2. (tinkerdown) consume `Validate()` per lvt block for structured diagnostics; drop the /tmp temp-file dance at websocket.go:495. 3. (tinkerdown) the tinkerdown-owned semantic gaps M1 verified: bound-refs (a used lvt-source must resolve to a declared source — proven-open this session), action-param completeness, the state-ref diagnostic teaching the real lvt-source fix, lvt-persist→removed-with-migration. Design corrections vs the outline, recorded in-plan: - The attribute allowlist stays in tinkerdown: livetemplate-Go never references the client-only lvt-mod:/lvt-nav:/lvt-ignore namespaces, so an upstream KnownAttributes export would drift from the client; vocabulary.go + TestKnownAttributesAreReal (guarded vs the vendored bundle) is already the best source-of-truth. - Field-name/schema validation (Phase 5 feed-forward #2) re-sequenced to M3, where its introspection dependency already lives. Roadmap, gap-analysis row #1, and section headers updated in step; M3–M5 remain outline-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h * M2 Phase 1 record: Validate() shipped in livetemplate v0.21.0 Ticks the Phase 1 tracker and writes the Learn. livetemplate v0.21.0 ships Validate(templateText string, opts ...ValidateOption) ([]Diagnostic, error). Key drift from the plan, recorded in the Learn: - Built on the serve-faithful ParseFS/(*Template).Parse path, NOT raw internal/parse: .Parse runs the full serve pipeline and returns parse errors directly (unlike the swallowing Execute), and the ParseFS path clones the component-bearing template so components resolve (the .Parse method drops them). - Diagnostic shipped as {Line, Severity, Message}: html/template reports no column (verified) and no check had a hint, so Col/Hint were dropped pre-release after PR review. They can be appended back non-breakingly with the deferred sample-data mode. - WithSampleData deferred (advisor): no consumer this cycle, determinism already shipped engine-side in v0.20.0. SeverityWarning reserved as its home. - The justification sharpened to an empirical gap: tinkerdown validate passes an unclosed {{range}} in a block as clean (never runs livetemplate's parser on block content) — Validate closes exactly this, unreachable downstream. Feed-forward to Phase 2: consume via WithValidateComponents(getComponentTemplates()...), map {Line, Message} onto fileValidationError, pin v0.21.0 (provenance = No, ClientVersion still 0.20.0), drop the /tmp temp-file dance at websocket.go:495. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h * M2 Phase 2: tinkerdown validate consumes livetemplate.Validate() per block Pins livetemplate v0.21.0 and runs each lvt block's template through the new Validate(), so a template-syntax or composition error in a block — an unclosed {{range}}, an unknown function, an unresolved component — is reported with a line and message instead of surfacing only at serve as a silently-dropped block. - validate.go: validateBlockTemplates runs livetemplate.Validate(block.Content, WithValidateComponents(ComponentTemplates()...)) per interactive block, in sorted-ID order, and gates validFiles. Diagnostics render as "line N: message". - websocket.go: exported getComponentTemplates -> ComponentTemplates so validate and serve resolve components against one set; dropped the /tmp/lvt-*.tmpl temp-file dance for an in-memory WithParseFS (same parseSources path, disk-free, behavior-identical to serve's old WithParseFiles). The new check immediately found a real pre-existing bug: examples/markdown-data- bookmarks used {{split ...}}, registered nowhere, so that block had always failed to render at serve (invisible before this phase). Fixed by adding capability (operator decision): split is Tinkerdown's first base block helper (blockHelperFuncs), merged into ComponentTemplates' set funcs so it reaches both a block's parse (set funcs reach the main parse) and its tree generation (getComponentFuncs merges the set funcs). A test pins split in both funcMaps. Verified: examples corpus 55/55 green (datatable/table blocks resolve, no false positives); PII e2e passes through the disk-free path (29.9s); full suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h * M2 Phase 3: tinkerdown-owned semantic validate checks Closes four "passes validate, breaks at serve" gaps M1's Learn verified: - Bound refs (unresolvedSourceDiags): a source bound via lvt-source must resolve to a declared source (frontmatter ∪ the tinkerdown.yaml governing the file, resolved per-app via sourceConfigForFile so a multi-app `validate examples/` checks each app against its own config). A typo is reported with the declared list instead of erroring only at serve as "source not found". - Action-param completeness (unsuppliedActionParams): every :param a kind:sql action references must be supplied by a form field or data-* attribute (:operator is server-set, excluded). The supplied set is deliberately over-inclusive (every data-* key + form field anywhere), so it fires only when a param is supplied nowhere — biasing to a safe miss over a false positive. - State-ref diagnostic teaches the real fix (add lvt-source), not the undefined state="block-id". - lvt-persist reports a migration hint (removed from knownAttributes + added to suggestAttribute) instead of a confusing "no state reference". The :name scanner is extracted into a single scanParams shared by SubstituteParams (runtime SQL) and the new ReferencedParams (validate) — one source of truth, so what substitution consumes and what validate requires cannot drift. The SubstituteParams refactor is behavior-equivalent (its existing suites + the PII approve→ExecTx→audit e2e stay green). The bound-refs check caught two skill examples (02-dashboard, 10-status-page) that declared their source in a documentation yaml block rather than frontmatter, unlike the other 8 — moved to frontmatter so the few-shot corpus uniformly teaches "declare the sources you bind" (M2's generation-reliability goal). Verified: examples corpus 55/55, skill corpus 10/10, full suite green (incl. the PII e2e through the refactored SubstituteParams). Each new guard has a verified-to-fail test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
adnaan
added a commit
that referenced
this pull request
Jul 24, 2026
…oped) (#308) * M3 kickoff: expand runtime-policy + field-validation design (convention 9) Expands the outline-only M3 into full phase blocks, informed by a direct Explore of both repos (2026-07-24). Milestone shape re-scoped TINKERDOWN-ONLY (drift from the plan's "upstream" filing). The evidence is firmer than M2's: an upstream WithActionPolicy would be dead code, not merely redundant — - Tinkerdown routes actions entirely through runtime.GenericState.HandleAction → RunSQLAction, NEVER through livetemplate.DispatchWithState. A livetemplate action-policy hook would never fire for tinkerdown. - Tinkerdown state is map-based ([]map[string]interface{}), so livetemplate's reflection-based introspection has no typed struct to read. - Unlike M2 (tinkerdown consumed livetemplate.Validate), an upstream hook here has zero tinkerdown consumption. A livetemplate-side hook for its own typed-struct consumers is separable framework work, deferred. Two tinkerdown phases: 1. Runtime approved-surface enforcement — a shared policy gate at all three action paths (WS-custom, webhook-direct, WS-builtins; no shared chokepoint today) so a running app / bypassing caller may only invoke approved actions against approved sources, respecting readonly. Untangled from confirm: (a client hint that can't be server-verified) — M3 enforces the approved surface, not a dialog. Per-action, not per-user (no per-request identity). 2. Field-name validation — validate checks {{.Field}} against a source's schema (source.SchemaProvider, sqlite-first; snakeToPascal; skip no-schema sources as a safe miss), closing M2 Phase 5 feed-forward #2. Roadmap, gap-analysis rows 2/3, provenance table (M3 = N/A, no client bump), and forward-references updated in step; M4–M5 remain outline-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h * M3 Phase 1: runtime approved-surface enforcement Enforces the approved surface at runtime, not only at generation time. When a project declares a generation: block, a running app — or a webhook / crafted-message caller — may only invoke an approved action, and a builtin write may only touch an approved source. Closes the hole the generation-time lint + the inert (client-only) confirm: leave open. Opt-in: no generation block, no gate. The approved surface is two surfaces: - config.EnforceApprovedAction(name) gates a custom action on its NAME ONLY, not its source — an approved action legitimately targets a writable store kept out of generation.sources (PII's access_store is unbindable by design). - config.EnforceApprovedSource(name) gates a builtin write on the block's BOUND source, which a manifest app may only bind if approved. Three call sites (the paths share no chokepoint): GenericState.HandleAction's custom-action branch and its add/toggle/delete/update branch (config threaded in via SetPageConfig, now unconditional), and the webhook entry (→ 403). confirm: stays a client hint — a dialog can't be server-verified. Tests prove rejection on every path (config predicates; WS custom + builtin; webhook 403-and-not-executed; opt-in when no generation block) and the PII e2e passes with approved actions running through the gate. Full suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h * M3 Phase 2 de-scoped: field-name validation needs a schema source tinkerdown lacks Audit finding: field-name validation introspects a source's schema via SQLiteSource.Schema() (PRAGMA on the live DB), but tinkerdown's sqlite *.db is gitignored and built from seed.sql at serve — so at validate time (CI, fresh checkout, generate->validate->serve) no DB exists, Schema() returns nil, and the check skips the entire corpus. That makes it the self-certifying-guard anti-pattern this project kept killing: it cannot fire where it is verified (the corpus 55/55 false-positive gate is vacuous) yet WOULD fire unvetted on a dev machine's stray .db — negative expected value, not harmless best-effort. So M3 ships as Phase 1 (the runtime approved-surface gate — the higher-value half). The M2 Phase 5 feed-forward #2 it was meant to close is re-filed to § Risks (deferred), not dropped: the real prerequisite is a validate-time schema source (parse CREATE TABLE from a seed, or a declared columns: schema on the source config) — a distinct introspection capability, backlog/M4. Walks back the plan's now-overclaiming M3 statements (Roadmap line, gap-row-2, deliverables, milestone-shape) to match; collapses the Phase 2 block to a de-scope note with the reasoning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h --------- Co-authored-by: Claude Opus 4.8 (1M context) <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.
🤖 Installing Claude Code GitHub App
This PR adds a GitHub Actions workflow that enables Claude Code integration in our repository.
What is Claude Code?
Claude Code is an AI coding agent that can help with:
How it works
Once this PR is merged, we'll be able to interact with Claude by mentioning @claude in a pull request or issue comment.
Once the workflow is triggered, Claude will analyze the comment and surrounding context, and execute on the request in a GitHub action.
Important Notes
Security
There's more information in the Claude Code action repo.
After merging this PR, let's try mentioning @claude in a comment on any PR to get started!