Skip to content

Conversation

krystophny
Copy link
Contributor

@krystophny krystophny commented Oct 2, 2025

Summary

  • detect PR metadata via env so GH CLI is optional and refs like fix-pr-ref-link still link to the active pull request
  • parse existing branches.json to retain prior branch entries and use restored artifacts for diff calculations
  • update CI to restore the published site for PR builds, pass head ref + PR metadata env vars, and keep deploying to the standard Pages site
  • extend integration coverage for multiple branches, PR refs, and workflow env handling

Testing

  • make templates
  • fpm test

Copy link

qodo-merge-pro bot commented Oct 2, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Insecure temp file

Description: Using a fixed predictable temp file path '/tmp/testboard_pr.json' for command output can
allow race conditions or symlink attacks leading to data leakage or command output
interception.
gh_api.f90 [135-170]

Referred Code
character(len=1024) :: cmd, temp_file
integer :: unit, stat, ios

temp_file = '/tmp/testboard_pr.json'
write (cmd, '(A)') trim(command)//' > "'//trim(temp_file)// &
   '" 2>/dev/null'

call execute_command_line(trim(cmd), exitstat=stat)
if (stat /= 0) then
   success = .false.
   call cleanup_temp_file(temp_file)
   return
end if

open (newunit=unit, file=temp_file, status='old', action='read', iostat=ios)
if (ios /= 0) then
   success = .false.
   call cleanup_temp_file(temp_file)
   return
end if



 ... (clipped 15 lines)
Insecure temp file

Description: Using a fixed predictable temp file path '/tmp/testboard_pr_state.json' can be exploited
via symlink or TOCTOU attacks to overwrite or read unintended files.
gh_api.f90 [245-285]

Referred Code
temp_file = '/tmp/testboard_pr_state.json'

! Call gh CLI to get PR state
write (cmd, '(A,I0,A)') 'gh pr view ', pr_number, ' --repo "'//trim(repo)// &
   '" --json state --jq ".state" > "'//trim(temp_file)//'" 2>/dev/null'

call execute_command_line(trim(cmd), exitstat=stat)

if (stat /= 0) return

! Read state from file
open (newunit=unit, file=temp_file, status='old', action='read', iostat=ios)
if (ios /= 0) return

read (unit, '(A)', iostat=ios) line
if (ios == 0) then
   ! Remove quotes and newlines
   line = adjustl(trim(line))
   if (line(1:1) == '"') then
      pos1 = 2
      pos2 = index(line(2:), '"')


 ... (clipped 20 lines)
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link

qodo-merge-pro bot commented Oct 2, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid race conditions with unique filenames

To prevent race conditions, generate a unique temporary filename in
run_pr_command instead of using the hardcoded /tmp/testboard_pr.json. A unique
name can be created by incorporating the process ID (PID).

src/gh_api.f90 [138-140]

-          temp_file = '/tmp/testboard_pr.json'
+      integer :: pid
+      character(len=24) :: pid_str
+      ! Assumes getpid() is available, e.g. from a C binding
+      pid = getpid()
+      write(pid_str, '(I0)') pid
+      temp_file = '/tmp/testboard_pr_' // trim(pid_str) // '.json'
       write (cmd, '(A)') trim(command)//' > "'//trim(temp_file)// &
          '" 2>/dev/null'
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a race condition due to a hardcoded temporary filename, which can cause incorrect behavior in concurrent executions. This is a significant correctness issue.

Medium
High-level
Refactor logic to avoid redundant parsing

In get_pr_info, the PR number is parsed from the ref string twice. This
redundant parsing should be removed by parsing only once at the beginning and
reusing the result.

Examples:

src/gh_api.f90 [27-68]
      pr_number = parse_pr_number_from_ref(normalized_branch)
      if (pr_number > 0) then
         metadata%pr_number = pr_number
         if (len_trim(repo) > 0) then
            metadata%pr_url = 'https://github.com/'//trim(repo)//'/pull/'// &
                              trim(str(pr_number))
         end if
      end if

      link_parsed = (metadata%pr_number > 0 .and. len_trim(metadata%pr_url) > 0)

 ... (clipped 32 lines)

Solution Walkthrough:

Before:

subroutine get_pr_info(branch, ...)
  ...
  pr_number = parse_pr_number_from_ref(normalized_branch)
  ...
  if (pr_number > 0) then
     command_ok = run_pr_command(...)
  else
     command_ok = run_pr_command(...)

     if (.not. command_ok .and. starts_with(normalized_branch, 'refs/pull/')) then
        ! Redundant parsing of the same string
        pr_number = parse_pr_number_from_ref(normalized_branch)
        if (pr_number > 0) then
           command_ok = run_pr_command(...)
        end if
     end if
  end if
  ...
end subroutine

After:

subroutine get_pr_info(branch, ...)
  ...
  pr_number = parse_pr_number_from_ref(normalized_branch)
  ...
  if (pr_number > 0) then
     command_ok = run_pr_command('gh pr view ' // pr_number, ...)
  else
     command_ok = run_pr_command('gh pr list --head ' // branch_query, ...)
  end if

  ! If the first command failed but we have a PR number, try the other command.
  if (.not. command_ok .and. pr_number > 0) then
    command_ok = run_pr_command('gh pr list --head ' // branch_query, ...)
  end if
  ...
end subroutine
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies redundant code where parse_pr_number_from_ref is called twice on the same variable, improving code clarity and removing unnecessary operations.

Low
General
Simplify digit check using intrinsic function

Simplify the is_all_digits function by using the intrinsic verify function
instead of a manual character-by-character loop. This is more concise and
idiomatic Fortran.

src/gh_api.f90 [203-218]

-       logical function is_all_digits(text) result(all_digits)
+   logical function is_all_digits(text) result(all_digits)
     !! Check whether the provided text is composed only of digits
       character(len=*), intent(in) :: text
-      integer :: i, code
+      character(len=10), parameter :: digits = '0123456789'
+      character(len=256) :: trimmed_text
 
-      all_digits = (len_trim(text) > 0)
-      if (.not. all_digits) return
+      trimmed_text = trim(text)
+      if (len(trimmed_text) == 0) then
+         all_digits = .false.
+         return
+      end if
 
-      do i = 1, len_trim(text)
-         code = iachar(text(i:i))
-         if (code < iachar('0') .or. code > iachar('9')) then
-            all_digits = .false.
-            return
-         end if
-      end do
+      all_digits = (verify(trimmed_text, digits) == 0)
    end function is_all_digits
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion offers a correct and more idiomatic way to implement the is_all_digits function using the intrinsic verify, which improves code conciseness and readability.

Low
  • Update

@krystophny krystophny merged commit 66b4383 into main Oct 3, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant