Skip to content

Comments

fix(prompts): "Shared Prompts" filter ignores direct shares (USER/GROUP ACL), only returns PUBLIC#11882

Open
NalinNair wants to merge 3 commits intodanny-avila:mainfrom
NalinNair:fix/shared-prompts-filter
Open

fix(prompts): "Shared Prompts" filter ignores direct shares (USER/GROUP ACL), only returns PUBLIC#11882
NalinNair wants to merge 3 commits intodanny-avila:mainfrom
NalinNair:fix/shared-prompts-filter

Conversation

@NalinNair
Copy link
Contributor

Problem

"Shared Prompts" filter only shows publicly-shared prompts. Prompts shared directly via the people picker (USER ACL) do not appear.

Secondary: direct URL navigation to a shared prompt shows the group name but empty text.

Root Cause

filterAccessibleIdsBySharedLogic intersects publicPromptGroupIds with accessibleIds when searchSharedOnly=true. Prompts with only USER ACL entries are excluded.

getPromptGroup uses findOne() without $lookup, so productionPrompt is never populated for direct URL navigation.

Fix

Bug AfilterAccessibleIdsBySharedLogic now accepts ownedPromptGroupIds:

  • MY_PROMPTS: accessible intersect owned
  • SHARED_PROMPTS: (accessible union public) minus owned
  • ALL: accessible union public (deduplicated)

Legacy fallback preserved when ownedPromptGroupIds is omitted.

Bug BgetPromptGroup uses aggregation with $lookup to populate productionPrompt.

Files Changed

File Change
packages/api/src/prompts/format.ts New ownedPromptGroupIds param for author-based filtering
api/server/routes/prompts.js Fetch owned IDs in parallel, pass to filter (2 locations)
api/models/Prompt.js Add getOwnedPromptGroupIds(), change getPromptGroup to $lookup aggregation

Testing

Tested with two users (author + recipient) across all filter modes:

  • Direct-shared prompt appears in "Shared Prompts" for recipient
  • Public prompt appears in "Shared Prompts" for non-author
  • Own prompts excluded from "Shared Prompts"
  • Own prompts appear in "My Prompts"
  • "All" shows everything accessible
  • Direct URL navigation to shared prompt shows text (Bug B)
  • Pagination unaffected

…d public prompts

The "Shared Prompts" filter only showed publicly-shared prompts, ignoring
prompts shared directly via the people picker (USER ACL). Fix by querying
the user's owned prompt group IDs and using author-based filtering instead
of public-only filtering. Also populate productionPrompt via $lookup in
the single-group endpoint so direct URL navigation shows prompt text.

Bug A: filterAccessibleIdsBySharedLogic now uses (accessible ∪ public) − owned
Bug B: getPromptGroup uses aggregation with $lookup for productionPrompt

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes prompt-group filtering so “Shared Prompts” includes direct shares (USER/GROUP ACL), and fixes direct URL navigation so shared prompt groups correctly populate productionPrompt.

Changes:

  • Update shared/my/all prompt-group ID filtering to incorporate owned prompt-group IDs (and deduplicate ID unions).
  • Fetch owned prompt-group IDs in the prompts routes and pass them into the shared-logic filter.
  • Update getPromptGroup to use an aggregation $lookup to populate productionPrompt.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/api/src/prompts/format.ts Extends shared filtering to distinguish owned vs shared prompt groups; dedupes unions; keeps legacy fallback.
api/server/routes/prompts.js Fetches owned/public IDs in parallel and supplies owned IDs to filtering in /all and /groups.
api/models/Prompt.js Adds getOwnedPromptGroupIds; changes getPromptGroup to aggregate with $lookup for productionPrompt.
Comments suppressed due to low confidence (3)

packages/api/src/prompts/format.ts:174

  • New owned/shared filtering logic is introduced here, but there are no unit tests covering MY_PROMPTS vs SHARED_PROMPTS vs ALL behavior (especially the owned/public/accessible set operations and legacy fallback). Adding focused tests would help prevent regressions in prompt group filtering.
export async function filterAccessibleIdsBySharedLogic({
  accessibleIds,
  searchShared,
  searchSharedOnly,
  publicPromptGroupIds,
  ownedPromptGroupIds,
}: {
  accessibleIds: Types.ObjectId[];
  searchShared: boolean;
  searchSharedOnly: boolean;
  publicPromptGroupIds?: Types.ObjectId[];
  ownedPromptGroupIds?: Types.ObjectId[];
}): Promise<Types.ObjectId[]> {
  const ownedIdStrings = new Set((ownedPromptGroupIds || []).map((id) => id.toString()));

  if (!searchShared) {
    // MY_PROMPTS — only prompt groups the user authored
    if (ownedPromptGroupIds != null) {
      return accessibleIds.filter((id) => ownedIdStrings.has(id.toString()));
    }
    // Legacy fallback: exclude public IDs (imprecise but backwards-compatible)
    const publicIdStrings = new Set((publicPromptGroupIds || []).map((id) => id.toString()));
    return accessibleIds.filter((id) => !publicIdStrings.has(id.toString()));
  }

  if (searchSharedOnly) {
    // SHARED_PROMPTS — all prompts the user can access that they did NOT author
    // Combine accessible + public, deduplicate, then exclude owned
    const allAccessible = [...accessibleIds, ...(publicPromptGroupIds || [])];
    const uniqueMap = new Map(allAccessible.map((id) => [id.toString(), id]));

    if (ownedPromptGroupIds != null) {
      return [...uniqueMap.values()].filter((id) => !ownedIdStrings.has(id.toString()));
    }
    // Legacy fallback
    if (!publicPromptGroupIds?.length) {
      return [];
    }
    const accessibleIdStrings = new Set(accessibleIds.map((id) => id.toString()));
    return publicPromptGroupIds.filter((id) => accessibleIdStrings.has(id.toString()));
  }

  // ALL — return everything accessible + public (deduplicated)
  const allAccessible = [...accessibleIds, ...(publicPromptGroupIds || [])];
  const uniqueMap = new Map(allAccessible.map((id) => [id.toString(), id]));
  return [...uniqueMap.values()];

api/models/Prompt.js:567

  • getPromptGroup changed from findOne().lean() to an aggregation with $lookup/$unwind, but there are no tests asserting that productionPrompt is populated (and that it stays populated for direct URL navigation). Consider adding a model-level test that creates a group + production prompt and verifies the returned shape.
  getPromptGroup: async (filter) => {
    try {
      // Cast string _id to ObjectId for aggregation (findOne auto-casts, aggregate does not)
      const matchFilter = { ...filter };
      if (typeof matchFilter._id === 'string') {
        matchFilter._id = new ObjectId(matchFilter._id);
      }
      const result = await PromptGroup.aggregate([
        { $match: matchFilter },
        {
          $lookup: {
            from: 'prompts',
            localField: 'productionId',
            foreignField: '_id',
            as: 'productionPrompt',
          },
        },
        { $unwind: { path: '$productionPrompt', preserveNullAndEmptyArrays: true } },
      ]);
      const group = result[0] || null;
      if (group?.author) {
        group.author = group.author.toString();
      }
      return group;

api/models/Prompt.js:384

  • getOwnedPromptGroupIds queries PromptGroup.find({ author }) using the raw author string. Elsewhere in this file the author filter is consistently cast with new ObjectId(userId), and a non-castable value would currently be swallowed by the catch and treated as “no owned groups”, breaking MY_PROMPTS/SHARED_PROMPTS filtering. Consider validating/casting author to ObjectId (e.g., ObjectId.isValid + new ObjectId) and returning/throwing on invalid input instead of silently returning an empty list.
const getOwnedPromptGroupIds = async (author) => {
  try {
    const groups = await PromptGroup.find({ author }, { _id: 1 }).lean();
    return groups.map((g) => g._id);
  } catch (error) {
    logger.error('Error getting owned prompt group IDs', error);
    return [];
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return group;
} catch (error) {
logger.error('Error getting prompt group', error);
return { message: 'Error getting prompt group' };
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getPromptGroup returns { message: 'Error getting prompt group' } from the catch block. Callers like canAccessPromptGroupResource treat any truthy resolver result as “found” and may pass this object through as resourceId to checkPermission, potentially turning a bad/invalid groupId into a 500 or an unexpected permission check path. Prefer returning null (not found/invalid) or rethrowing so the middleware can respond deterministically.

Suggested change
return { message: 'Error getting prompt group' };
return null;

Copilot uses AI. Check for mistakes.
NalinNair added a commit to NalinNair/LibreChat that referenced this pull request Feb 20, 2026
…ng, null error return, unit tests

- getOwnedPromptGroupIds: validate and cast author to ObjectId
  (consistent with rest of Prompt.js, prevents silent empty-list on
  invalid input)
- getPromptGroup catch: return null instead of { message } object so
  callers (canAccessPromptGroupResource) don't treat errors as found
- Add format.spec.ts with 8 tests covering MY_PROMPTS, SHARED_PROMPTS,
  ALL filtering logic including deduplication and legacy fallback paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
NalinNair and others added 2 commits February 20, 2026 15:45
…ng, null error return, unit tests

- getOwnedPromptGroupIds: validate and cast author to ObjectId
  (consistent with rest of Prompt.js, prevents silent empty-list on
  invalid input)
- getPromptGroup catch: return null instead of { message } object so
  callers (canAccessPromptGroupResource) don't treat errors as found
- Add format.spec.ts with 8 tests covering MY_PROMPTS, SHARED_PROMPTS,
  ALL filtering logic including deduplication and legacy fallback paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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