diff --git a/src/job_group.cpp b/src/job_group.cpp
index b1a145dcd..73099298b 100644
--- a/src/job_group.cpp
+++ b/src/job_group.cpp
@@ -47,8 +47,10 @@ void job_group_t::set_pgid(pid_t pgid) {
 
 maybe_t<pid_t> job_group_t::get_pgid() const { return pgid_; }
 
-void job_group_t::populate_group_for_job(job_t *job, const job_group_ref_t &proposed) {
-    assert(!job->group && "Job already has a group");
+// static
+job_group_ref_t job_group_t::resolve_group_for_job(const job_t &job,
+                                                   const job_group_ref_t &proposed) {
+    assert(!job.group && "Job already has a group");
     // Note there's three cases to consider:
     //  nullptr         -> this is a root job, there is no inherited job group
     //  internal        -> the parent is running as part of a simple function execution
@@ -56,10 +58,10 @@ void job_group_t::populate_group_for_job(job_t *job, const job_group_ref_t &prop
     //  non-internal    -> we are running as part of a real pipeline
     // Decide if this job can use an internal group.
     // This is true if it's a simple foreground execution of an internal proc.
-    bool initial_bg = job->is_initially_background();
-    bool first_proc_internal = job->processes.front()->is_internal();
+    bool initial_bg = job.is_initially_background();
+    bool first_proc_internal = job.processes.front()->is_internal();
     bool can_use_internal =
-        !initial_bg && job->processes.size() == 1 && job->processes.front()->is_internal();
+        !initial_bg && job.processes.size() == 1 && job.processes.front()->is_internal();
 
     bool needs_new_group = false;
     if (!proposed) {
@@ -73,27 +75,25 @@ void job_group_t::populate_group_for_job(job_t *job, const job_group_ref_t &prop
         needs_new_group = true;
     }
 
-    job->mut_flags().is_group_root = needs_new_group;
+    if (!needs_new_group) return proposed;
 
-    if (!needs_new_group) {
-        job->group = proposed;
-    } else {
-        properties_t props{};
-        props.job_control = job->wants_job_control();
-        props.wants_terminal = job->wants_job_control() && !job->from_event_handler();
-        props.is_internal = can_use_internal;
-        props.job_id = can_use_internal ? -1 : acquire_job_id();
-        job->group.reset(new job_group_t(props, job->command()));
+    // We will need to create a new group.
+    properties_t props{};
+    props.job_control = job.wants_job_control();
+    props.wants_terminal = job.wants_job_control() && !job.from_event_handler();
+    props.is_internal = can_use_internal;
+    props.job_id = can_use_internal ? -1 : acquire_job_id();
+    job_group_ref_t result{new job_group_t(props, job.command())};
 
-        // Mark if it's foreground.
-        job->group->set_is_foreground(!initial_bg);
+    // Mark if it's foreground.
+    result->set_is_foreground(!initial_bg);
 
-        // Perhaps this job should immediately live in fish's pgroup.
-        // There's two reasons why it may be so:
-        //  1. The job doesn't need job control.
-        //  2. The first process in the job is internal to fish; this needs to own the tty.
-        if (!can_use_internal && (!props.job_control || first_proc_internal)) {
-            job->group->set_pgid(getpgrp());
-        }
+    // Perhaps this job should immediately live in fish's pgroup.
+    // There's two reasons why it may be so:
+    //  1. The job doesn't need job control.
+    //  2. The first process in the job is internal to fish; this needs to own the tty.
+    if (!can_use_internal && (!props.job_control || first_proc_internal)) {
+        result->set_pgid(getpgrp());
     }
+    return result;
 }
diff --git a/src/job_group.h b/src/job_group.h
index f845fb31d..90da659bd 100644
--- a/src/job_group.h
+++ b/src/job_group.h
@@ -74,9 +74,11 @@ class job_group_t {
     void mark_root_constructed() { root_constructed_ = true; };
     bool is_root_constructed() const { return root_constructed_; }
 
-    /// Given a job and a proposed job group (possibly null), populate the job's group field.
+    /// Given a job and a proposed job group (possibly null), return a group for the job.
     /// The proposed group is the group from the parent job, or null if this is a root.
-    static void populate_group_for_job(job_t *job, const job_group_ref_t &proposed_tree);
+    /// This never returns null.
+    static job_group_ref_t resolve_group_for_job(const job_t &job,
+                                                 const job_group_ref_t &proposed_group);
 
     ~job_group_t();
 
diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp
index 6d17c152d..eb1864e83 100644
--- a/src/parse_execution.cpp
+++ b/src/parse_execution.cpp
@@ -1342,9 +1342,10 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
 
     // Clean up the job on failure or cancellation.
     if (pop_result == end_execution_reason_t::ok) {
-        // Set the pgroup assignment mode and job group, now that the job is populated.
-        job_group_t::populate_group_for_job(job.get(), ctx.job_group);
-        assert(job->group && "Should have a job group");
+        // Resolve the job's group and mark if this job is the first to get it.
+        job->group = job_group_t::resolve_group_for_job(*job, ctx.job_group);
+        assert(job->group && "Should not have a null group");
+        job->mut_flags().is_group_root = (job->group != ctx.job_group);
 
         // Success. Give the job to the parser - it will clean it up.
         parser->job_add(job);