Skip to content

feat: pr-body-spec-backlink#73

Merged
satyaborg merged 1 commit into
mainfrom
feat/pr-body-spec-backlink
Jun 19, 2026
Merged

feat: pr-body-spec-backlink#73
satyaborg merged 1 commit into
mainfrom
feat/pr-body-spec-backlink

Conversation

@satyaborg

Copy link
Copy Markdown
Owner

Polish the generated PR description footer and link back to the spec

Generated by devloop --create-pr.

Problem

The draft PR body that devloop --create-pr writes (draft_pull_request_body, devloop:3709) reads like an internal tool dump rather than a finished description. It opens with Generated by \devloop --create-pr`. (devloop:3724) — the raw invocation, not a brand — and ends with Latest commit: $commit (devloop:3744`), a noisy line that duplicates what GitHub already shows in the PR's commit list. It also never points a reader back to the spec that drove the change, so anyone reviewing the PR has no in-description path to the intent it implements. The moment it hurts: opening a freshly created draft PR, seeing a bare command string at the top and a redundant SHA at the bottom, and having to leave the PR to find the spec that explains why the change exists.

Outcome

The generated PR body ends with the literal footer Generated by [devloop.sh](https://devloop.sh) as its last block (below the ---), carries no Latest commit: line, and no longer contains the Generated by \devloop --create-pr`.text. When (and only when) the source spec resolves to a path inside the repo and is committed at the PR head commit, aSpec:line sits just above the footer linking to that spec via an absolutehttps://github.com///blob//` URL. Otherwise no spec link appears, and the body still leaks no absolute local filesystem path.

Acceptance Criteria

AC1: The generated PR body's final content block (below the ---) is the literal line Generated by [devloop.sh](https://devloop.sh), and the string devloop --create-pr does not appear anywhere in the body.
AC2: The generated PR body contains no Latest commit: line and no bare commit-hash line sourced from it.
AC3: When the source spec is committed inside the repo at the PR head commit, the body contains a Spec: line whose link target is an absolute https://github.com/<owner>/<repo>/blob/<commit>/<relpath> URL using the spec's repo-relative path, positioned above the Generated by footer.
AC4: When the source spec is untracked/uncommitted, the body contains no Spec: line and no blob/ URL.
AC5: When the source spec resolves to a path outside the repo, the body contains no Spec: line.
AC6: The generated PR body contains no absolute local filesystem path (the existing /Users/ no-leak guard at scripts/devloop_test.sh:1788 still passes).
AC7: bash scripts/devloop_test.sh passes, including the updated e2e PR-body assertions and a new direct unit test covering the committed, uncommitted, and outside-repo backlink cases.

Review Trail

Review rounds and the final report are posted as PR comments below.


Latest commit: faebad2

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 19, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
devloop faebad2 Commit Preview URL

Branch Preview URL
Jun 19 2026, 04:08 AM

@satyaborg

Copy link
Copy Markdown
Owner Author

Devloop Review Round 1

  • Verdict: ACCEPT

Review 1

Verdict: ACCEPT

Acceptance matrix

Criterion Status Implementation evidence Test evidence
AC1 PASS Hardcoded Generated by \devloop --create-pr`.line deleted from heredoc; footer printed last as literalGenerated by devloop.sh (devloop:3747); whole body piped through sed 's/devloop --create-pr/devloop/g' (devloop:3748`) so the string cannot appear even when spec prose mentions it. e2e contains "$pr_body" "Generated by [devloop.sh]...", not_contains "$pr_body" "devloop --create-pr", and equals "$pr_body_footer" "Generated by [devloop.sh]..." (footer-is-last via awk); unit spec backlink body raw command asserts command_spec prose mention is scrubbed. Full suite green.
AC2 PASS Latest commit: $commit line removed from heredoc (was devloop:3744); commit="none" display fallback also dropped. e2e not_contains "$pr_body" "Latest commit:" plus grep -Eq '^[0-9a-f]{7,40}$' bare-hash guard → fail if present.
AC3 PASS pr_spec_backlink (devloop:3751-3770) computes repo-relative $rel, confirms git cat-file -e "$commit:$rel", derives nameWithOwner via gh repo view, and prints Spec: [$rel](https://github.com/$owner/blob/$commit/$rel); emitted between --- and footer (devloop:3746). unit spec backlink committed asserts exact https://github.com/satyaborg/devloop/blob/<sha>/specs/committed.md; spec backlink body + grep-line-number ordering check proves Spec: is above the footer.
AC4 PASS Uncommitted/untracked spec → git cat-file -e "$commit:$rel" fails → return 0 (empty) (devloop:3765). unit spec backlink uncommitted expects ""; e2e (fixture spec is uncommitted) not_contains "$pr_body" "Spec:" and not_contains "$pr_body" "blob/".
AC5 PASS Spec outside repo → prefix case "$repo_abs"/* fails → *) return 0 (devloop:3760-3763). unit spec backlink outside repo (spec under $work, repo $backlink_repo) expects "".
AC6 PASS Body uses prefix-stripped $rel (devloop:3761), never $spec_abs; backlink URL carries only the repo-relative path. e2e grep -q '/Users/' no-leak guard still present and passes.
AC7 PASS New pr_spec_backlink direct unit test (committed/uncommitted/outside-repo) plus updated e2e PR-body assertions. I ran bash scripts/devloop_test.sh: 52 ok, 0 not ok, including ok - spec backlink and ok - 100% project function coverage. Red check confirmed: against main's devloop the suite aborts at not ok - spec backlink committed ... got [].

Engineering quality matrix

Area Status Evidence
Correctness PASS Path canonicalization is consistent on both sides of the prefix match — repo_abs uses pwd -P (devloop:3758) and absolute_existing_file also uses pwd -P (devloop:2662) — so the case "$spec_abs" in "$repo_abs"/* test is symlink-safe (verified: the test's $work-rooted fixtures match correctly). All failure paths return 0 (empty), keeping the body well-formed. $FINAL_COMMIT is reachable from $SOURCE_REPO because the worktree shares the object store. Dropping commit="none" is safe since the commit is no longer rendered.
Test quality PASS Direct unit test verifies exact outcomes (full URL, empty cases) and an integration test asserts real devloop --create-pr body content, footer-last position, Spec/footer ordering, and absence of the dropped lines. Red/green verified by reverting devloop to main and rerunning.
Maintainability PASS Single small named helper with early-return guards; explicit-argument threading mirrors the existing commit parameter convention through all four functions.
Architecture boundaries PASS Backlink logic lives in its own pr_spec_backlink helper and SOURCE_REPO is threaded as an explicit source_repo arg rather than read as a global in the body builder, keeping the helper unit-testable in isolation.
Simplicity PASS Direct guard chain, no wrappers or new dependencies. The name_with_owner post-gh sed/head cleanup is mildly belt-and-suspenders but harmless.
Security PASS git/gh invoked with quoted args, no eval; $rel/$commit/$owner are interpolated only into a printed URL string, never executed. No new attack surface.
Operational safety PASS Best-effort and non-fatal: every failure mode (empty inputs, non-dir repo, outside-repo, uncommitted, invalid commit, gh failure) yields an empty backlink and never aborts PR creation or alters run status.

Review flags

  • Silent decision: absent - Both notable choices are recorded in the track "Design tradeoffs": the body-stream devloop --create-pr scrub and threading SOURCE_REPO explicitly instead of reading the global.
  • Scope drift: absent - Committed diff touches only devloop (target functions, new helper, call site) and scripts/devloop_test.sh. No README.md change required (it has only a usage-table row, not a PR-body example). The .devloop/tracks/ file is untracked runtime, correctly not committed.
  • Missing test: absent - AC7's three required cases (committed/uncommitted/outside-repo) are covered and function coverage is 100%. The defensive gh repo view-failure and empty-input branches are not directly asserted, but they fall outside AC7's stated test scope (see Notes).

Findings

None.

Missing tests

  • None.

Fix instructions

None.

Notes

  • The blanket | sed 's/devloop --create-pr/devloop/g' (devloop:3748) rewrites any occurrence of the command string in spec-sourced prose (title/Problem/Outcome/criteria), not just the removed generated line. This is a documented (track) and tested (command_spec) tradeoff, and it is required to satisfy AC1's literal "does not appear anywhere" wording. Blast radius is narrow — only specs that themselves describe devloop's --create-pr command would see prose silently altered (this very spec's Problem section is such a case). Acceptable as scoped, but worth keeping in mind if PR bodies ever need to quote that command verbatim.
  • Untested defensive branches: the gh repo view failure path and the empty-source_repo/spec/commit guards in pr_spec_backlink rely on return 0 and are not directly asserted. The gh-success, uncommitted (cat-file miss), and outside-repo (case miss) branches are exercised; this stays within AC7's explicit three-case requirement and the function-level coverage gate.
  • Red/green confirmed independently: reverting only devloop to main while keeping the new tests reproduces not ok - spec backlink committed ... got []; HEAD is fully green (52 ok / 0 not ok, 100% function coverage).

@satyaborg

Copy link
Copy Markdown
Owner Author

Devloop Final Report

Field Value
Final status accepted
Pass count 1 / 5
Final verdict ACCEPT
PR URL #73
Branch feat/pr-body-spec-backlink

Acceptance Matrix Summary

Acceptance matrix

Criterion Status Implementation evidence Test evidence
AC1 PASS Hardcoded Generated by \devloop --create-pr`.line deleted from heredoc; footer printed last as literalGenerated by devloop.sh (devloop:3747); whole body piped through sed 's/devloop --create-pr/devloop/g' (devloop:3748`) so the string cannot appear even when spec prose mentions it. e2e contains "$pr_body" "Generated by [devloop.sh]...", not_contains "$pr_body" "devloop --create-pr", and equals "$pr_body_footer" "Generated by [devloop.sh]..." (footer-is-last via awk); unit spec backlink body raw command asserts command_spec prose mention is scrubbed. Full suite green.
AC2 PASS Latest commit: $commit line removed from heredoc (was devloop:3744); commit="none" display fallback also dropped. e2e not_contains "$pr_body" "Latest commit:" plus grep -Eq '^[0-9a-f]{7,40}$' bare-hash guard → fail if present.
AC3 PASS pr_spec_backlink (devloop:3751-3770) computes repo-relative $rel, confirms git cat-file -e "$commit:$rel", derives nameWithOwner via gh repo view, and prints Spec: [$rel](https://github.com/$owner/blob/$commit/$rel); emitted between --- and footer (devloop:3746). unit spec backlink committed asserts exact https://github.com/satyaborg/devloop/blob/<sha>/specs/committed.md; spec backlink body + grep-line-number ordering check proves Spec: is above the footer.
AC4 PASS Uncommitted/untracked spec → git cat-file -e "$commit:$rel" fails → return 0 (empty) (devloop:3765). unit spec backlink uncommitted expects ""; e2e (fixture spec is uncommitted) not_contains "$pr_body" "Spec:" and not_contains "$pr_body" "blob/".
AC5 PASS Spec outside repo → prefix case "$repo_abs"/* fails → *) return 0 (devloop:3760-3763). unit spec backlink outside repo (spec under $work, repo $backlink_repo) expects "".
AC6 PASS Body uses prefix-stripped $rel (devloop:3761), never $spec_abs; backlink URL carries only the repo-relative path. e2e grep -q '/Users/' no-leak guard still present and passes.
AC7 PASS New pr_spec_backlink direct unit test (committed/uncommitted/outside-repo) plus updated e2e PR-body assertions. I ran bash scripts/devloop_test.sh: 52 ok, 0 not ok, including ok - spec backlink and ok - 100% project function coverage. Red check confirmed: against main's devloop the suite aborts at not ok - spec backlink committed ... got [].

Engineering Quality Summary

Engineering quality matrix

Area Status Evidence
Correctness PASS Path canonicalization is consistent on both sides of the prefix match — repo_abs uses pwd -P (devloop:3758) and absolute_existing_file also uses pwd -P (devloop:2662) — so the case "$spec_abs" in "$repo_abs"/* test is symlink-safe (verified: the test's $work-rooted fixtures match correctly). All failure paths return 0 (empty), keeping the body well-formed. $FINAL_COMMIT is reachable from $SOURCE_REPO because the worktree shares the object store. Dropping commit="none" is safe since the commit is no longer rendered.
Test quality PASS Direct unit test verifies exact outcomes (full URL, empty cases) and an integration test asserts real devloop --create-pr body content, footer-last position, Spec/footer ordering, and absence of the dropped lines. Red/green verified by reverting devloop to main and rerunning.
Maintainability PASS Single small named helper with early-return guards; explicit-argument threading mirrors the existing commit parameter convention through all four functions.
Architecture boundaries PASS Backlink logic lives in its own pr_spec_backlink helper and SOURCE_REPO is threaded as an explicit source_repo arg rather than read as a global in the body builder, keeping the helper unit-testable in isolation.
Simplicity PASS Direct guard chain, no wrappers or new dependencies. The name_with_owner post-gh sed/head cleanup is mildly belt-and-suspenders but harmless.
Security PASS git/gh invoked with quoted args, no eval; $rel/$commit/$owner are interpolated only into a printed URL string, never executed. No new attack surface.
Operational safety PASS Best-effort and non-fatal: every failure mode (empty inputs, non-dir repo, outside-repo, uncommitted, invalid commit, gh failure) yields an empty backlink and never aborts PR creation or alters run status.

Implementation Summary

  • Final branch: feat/pr-body-spec-backlink
  • Final commit: faebad2
  • Commit message: feat: pr-body-spec-backlink

Commit References

  • pass 1 faebad2 feat: pr-body-spec-backlink (devloop, scripts/devloop_test.sh)

Tests Run

  • Verification hook log: not configured
  • Review test evidence: see the acceptance matrix summary above.

Residual Risk

  • No blocking residual risk was recorded by the final review.

@satyaborg satyaborg marked this pull request as ready for review June 19, 2026 04:35
@satyaborg satyaborg merged commit a801b4e into main Jun 19, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant